Skip to content

Commit 0f7a280

Browse files
committed
Remove timeout config options introduced in 4.4.5
* `update_routing_table_timeout` * `session_connection_timeout` Server-side keep-alives communicated through configuration hints together with `connection_acquisition_timeout` (which, starting with 4.4.5, includes the full handshake, i.e., with `HELLO` message) are sufficient to avoid the driver getting stuck.
1 parent 799720d commit 0f7a280

File tree

19 files changed

+123
-225
lines changed

19 files changed

+123
-225
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,11 @@
123123
`protocol_version` and `init_size`.
124124
- Introduced `neo4j.exceptions.SessionError` that is raised when trying to
125125
execute work on a closed or otherwise terminated session.
126+
- Removed deprecated config options `update_routing_table_timeout` and
127+
`session_connection_timeout`.
128+
Server-side keep-alives communicated through configuration hints together with
129+
the config option `connection_acquisition_timeout` are sufficient to avoid the
130+
driver getting stuck.
126131

127132

128133
## Version 4.4

neo4j/_async/driver.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ def driver(
8080
max_connection_lifetime: float = ...,
8181
max_connection_pool_size: int = ...,
8282
connection_timeout: float = ...,
83-
update_routing_table_timeout: float = ...,
8483
trust: t.Union[
8584
te.Literal["TRUST_ALL_CERTIFICATES"],
8685
te.Literal["TRUST_SYSTEM_CA_SIGNED_CERTIFICATES"]
@@ -97,7 +96,6 @@ def driver(
9796

9897
# undocumented/unsupported options
9998
# they may be change or removed any time without prior notice
100-
session_connection_timeout: float = ...,
10199
connection_acquisition_timeout: float = ...,
102100
max_transaction_retry_time: float = ...,
103101
initial_retry_delay: float = ...,
@@ -339,7 +337,6 @@ def encrypted(self) -> bool:
339337

340338
def session(
341339
self,
342-
session_connection_timeout: float = ...,
343340
connection_acquisition_timeout: float = ...,
344341
max_transaction_retry_time: float = ...,
345342
database: t.Optional[str] = ...,

neo4j/_async/io/_pool.py

Lines changed: 32 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -33,16 +33,13 @@
3333
AsyncRLock,
3434
)
3535
from ..._async_compat.network import AsyncNetworkUtil
36-
from ..._async_compat.util import AsyncUtil
3736
from ..._conf import (
3837
PoolConfig,
3938
WorkspaceConfig,
4039
)
4140
from ..._deadline import (
4241
connection_deadline,
4342
Deadline,
44-
merge_deadlines,
45-
merge_deadlines_and_timeouts,
4643
)
4744
from ..._exceptions import BoltError
4845
from ..._routing import RoutingTable
@@ -222,18 +219,18 @@ async def health_check(connection_, deadline_):
222219

223220
@abc.abstractmethod
224221
async def acquire(
225-
self, access_mode, timeout, acquisition_timeout,
226-
database, bookmarks, liveness_check_timeout
222+
self, access_mode, timeout, database, bookmarks, liveness_check_timeout
227223
):
228224
""" Acquire a connection to a server that can satisfy a set of parameters.
229225
230226
:param access_mode:
231-
:param timeout: total timeout (including potential preparation)
232-
:param acquisition_timeout: timeout for actually acquiring a connection
227+
:param timeout: timeout for the core acquisition
228+
(excluding potential preparation like fetching routing tables).
233229
:param database:
234230
:param bookmarks:
235231
:param liveness_check_timeout:
236232
"""
233+
...
237234

238235
def kill_and_release(self, *connections):
239236
""" Release connections back into the pool after closing them.
@@ -397,12 +394,11 @@ def __repr__(self):
397394
self.address)
398395

399396
async def acquire(
400-
self, access_mode, timeout, acquisition_timeout,
401-
database, bookmarks, liveness_check_timeout
397+
self, access_mode, timeout, database, bookmarks, liveness_check_timeout
402398
):
403399
# The access_mode and database is not needed for a direct connection,
404400
# it's just there for consistency.
405-
deadline = merge_deadlines_and_timeouts(timeout, acquisition_timeout)
401+
deadline = Deadline.from_timeout_or_deadline(timeout)
406402
return await self._acquire(
407403
self.address, deadline, liveness_check_timeout
408404
)
@@ -464,22 +460,6 @@ def __repr__(self):
464460
"""
465461
return "<{} addresses={!r}>".format(self.__class__.__name__, self.get_default_database_initial_router_addresses())
466462

467-
@asynccontextmanager
468-
async def _refresh_lock_deadline(self, deadline):
469-
timeout = deadline.to_timeout()
470-
if timeout == float("inf"):
471-
timeout = -1
472-
if not await self.refresh_lock.acquire(timeout=timeout):
473-
raise ClientError(
474-
"pool failed to update routing table within {!r}s (timeout)"
475-
.format(deadline.original_timeout)
476-
)
477-
478-
try:
479-
yield
480-
finally:
481-
self.refresh_lock.release()
482-
483463
@property
484464
def first_initial_routing_address(self):
485465
return self.get_default_database_initial_router_addresses()[0]
@@ -513,7 +493,7 @@ async def get_or_create_routing_table(self, database):
513493
return self.routing_tables[database]
514494

515495
async def fetch_routing_info(
516-
self, address, database, imp_user, bookmarks, deadline
496+
self, address, database, imp_user, bookmarks, timeout
517497
):
518498
""" Fetch raw routing info from a given router address.
519499
@@ -524,32 +504,32 @@ async def fetch_routing_info(
524504
:type imp_user: str or None
525505
:param bookmarks: iterable of bookmark values after which the routing
526506
info should be fetched
527-
:param deadline: connection acquisition deadline
507+
:param timeout: connection acquisition timeout
528508
529509
:return: list of routing records, or None if no connection
530510
could be established or if no readers or writers are present
531511
:raise ServiceUnavailable: if the server does not support
532512
routing, or if routing support is broken or outdated
533513
"""
514+
deadline = Deadline.from_timeout_or_deadline(timeout)
534515
cx = await self._acquire(address, deadline, None)
535516
try:
536-
with connection_deadline(cx, deadline):
537-
routing_table = await cx.route(
538-
database or self.workspace_config.database,
539-
imp_user or self.workspace_config.impersonated_user,
540-
bookmarks
541-
)
517+
routing_table = await cx.route(
518+
database or self.workspace_config.database,
519+
imp_user or self.workspace_config.impersonated_user,
520+
bookmarks
521+
)
542522
finally:
543523
await self.release(cx)
544524
return routing_table
545525

546526
async def fetch_routing_table(
547-
self, *, address, deadline, database, imp_user, bookmarks
527+
self, *, address, timeout, database, imp_user, bookmarks
548528
):
549529
""" Fetch a routing table from a given router address.
550530
551531
:param address: router address
552-
:param deadline: deadline
532+
:param timeout: connection acquisition timeout
553533
:param database: the database name
554534
:type: str
555535
:param imp_user: the user to impersonate while fetching the routing
@@ -563,7 +543,7 @@ async def fetch_routing_table(
563543
new_routing_info = None
564544
try:
565545
new_routing_info = await self.fetch_routing_info(
566-
address, database, imp_user, bookmarks, deadline
546+
address, database, imp_user, bookmarks, timeout
567547
)
568548
except Neo4jError as e:
569549
# checks if the code is an error that is caused by the client. In
@@ -606,7 +586,7 @@ async def fetch_routing_table(
606586
return new_routing_table
607587

608588
async def _update_routing_table_from(
609-
self, *routers, database, imp_user, bookmarks, deadline,
589+
self, *routers, database, imp_user, bookmarks, timeout,
610590
database_callback
611591
):
612592
""" Try to update routing tables with the given routers.
@@ -621,12 +601,9 @@ async def _update_routing_table_from(
621601
async for address in AsyncNetworkUtil.resolve_address(
622602
router, resolver=self.pool_config.resolver
623603
):
624-
if deadline.expired():
625-
return False
626604
new_routing_table = await self.fetch_routing_table(
627-
address=address,
628-
deadline=deadline,
629-
database=database, imp_user=imp_user, bookmarks=bookmarks
605+
address=address, timeout=timeout, database=database,
606+
imp_user=imp_user, bookmarks=bookmarks
630607
)
631608
if new_routing_table is not None:
632609
new_database = new_routing_table.database
@@ -656,7 +633,7 @@ async def update_routing_table(
656633
table
657634
:type imp_user: str or None
658635
:param bookmarks: bookmarks used when fetching routing table
659-
:param timeout: timeout in seconds for how long to try updating
636+
:param timeout: connection acquisition timeout
660637
:param database_callback: A callback function that will be called with
661638
the database name as only argument when a new routing table has been
662639
acquired. This database name might different from `database` if that
@@ -665,10 +642,7 @@ async def update_routing_table(
665642
666643
:raise neo4j.exceptions.ServiceUnavailable:
667644
"""
668-
deadline = merge_deadlines_and_timeouts(
669-
timeout, self.pool_config.update_routing_table_timeout
670-
)
671-
async with self._refresh_lock_deadline(deadline):
645+
async with self.refresh_lock:
672646
routing_table = await self.get_or_create_routing_table(database)
673647
# copied because it can be modified
674648
existing_routers = set(routing_table.routers)
@@ -681,23 +655,22 @@ async def update_routing_table(
681655
if await self._update_routing_table_from(
682656
self.first_initial_routing_address, database=database,
683657
imp_user=imp_user, bookmarks=bookmarks,
684-
deadline=deadline, database_callback=database_callback
658+
timeout=timeout, database_callback=database_callback
685659
):
686660
# Why is only the first initial routing address used?
687661
return
688662
if await self._update_routing_table_from(
689663
*(existing_routers - {self.first_initial_routing_address}),
690664
database=database, imp_user=imp_user, bookmarks=bookmarks,
691-
deadline=deadline, database_callback=database_callback
665+
timeout=timeout, database_callback=database_callback
692666
):
693667
return
694668

695669
if not prefer_initial_routing_address:
696670
if await self._update_routing_table_from(
697671
self.first_initial_routing_address, database=database,
698672
imp_user=imp_user, bookmarks=bookmarks,
699-
deadline=deadline,
700-
database_callback=database_callback
673+
timeout=timeout, database_callback=database_callback
701674
):
702675
# Why is only the first initial routing address used?
703676
return
@@ -714,7 +687,7 @@ async def update_connection_pool(self, *, database):
714687
await super(AsyncNeo4jPool, self).deactivate(address)
715688

716689
async def ensure_routing_table_is_fresh(
717-
self, *, access_mode, database, imp_user, bookmarks, deadline=None,
690+
self, *, access_mode, database, imp_user, bookmarks, timeout=None,
718691
database_callback=None
719692
):
720693
""" Update the routing table if stale.
@@ -730,15 +703,15 @@ async def ensure_routing_table_is_fresh(
730703
:return: `True` if an update was required, `False` otherwise.
731704
"""
732705
from neo4j.api import READ_ACCESS
733-
async with self._refresh_lock_deadline(deadline):
706+
async with self.refresh_lock:
734707
routing_table = await self.get_or_create_routing_table(database)
735708
if routing_table.is_fresh(readonly=(access_mode == READ_ACCESS)):
736709
# Readers are fresh.
737710
return False
738711

739712
await self.update_routing_table(
740713
database=database, imp_user=imp_user, bookmarks=bookmarks,
741-
timeout=deadline, database_callback=database_callback
714+
timeout=timeout, database_callback=database_callback
742715
)
743716
await self.update_connection_pool(database=database)
744717

@@ -778,34 +751,24 @@ async def _select_address(self, *, access_mode, database):
778751
return choice(addresses_by_usage[min(addresses_by_usage)])
779752

780753
async def acquire(
781-
self, access_mode, timeout, acquisition_timeout,
782-
database, bookmarks, liveness_check_timeout
754+
self, access_mode, timeout, database, bookmarks, liveness_check_timeout
783755
):
784756
if access_mode not in (WRITE_ACCESS, READ_ACCESS):
785757
raise ClientError("Non valid 'access_mode'; {}".format(access_mode))
786758
if not timeout:
787759
raise ClientError("'timeout' must be a float larger than 0; {}"
788760
.format(timeout))
789-
if not acquisition_timeout:
790-
raise ClientError("'acquisition_timeout' must be a float larger "
791-
"than 0; {}".format(acquisition_timeout))
792-
deadline = Deadline.from_timeout_or_deadline(timeout)
793761

794762
from neo4j.api import check_access_mode
795763
access_mode = check_access_mode(access_mode)
796-
async with self._refresh_lock_deadline(deadline):
764+
async with self.refresh_lock:
797765
log.debug("[#0000] C: <ROUTING TABLE ENSURE FRESH> %r",
798766
self.routing_tables)
799767
await self.ensure_routing_table_is_fresh(
800768
access_mode=access_mode, database=database, imp_user=None,
801-
bookmarks=bookmarks, deadline=deadline
769+
bookmarks=bookmarks, timeout=timeout
802770
)
803771

804-
# Making sure the routing table is fresh is not considered part of the
805-
# connection acquisition. Hence, the acquisition_timeout starts now!
806-
deadline = merge_deadlines(
807-
deadline, Deadline.from_timeout_or_deadline(acquisition_timeout)
808-
)
809772
while True:
810773
try:
811774
# Get an address for a connection that have the fewest in-use
@@ -817,6 +780,7 @@ async def acquire(
817780
raise SessionExpired("Failed to obtain connection towards '%s' server." % access_mode) from err
818781
try:
819782
log.debug("[#0000] C: <ACQUIRE ADDRESS> database=%r address=%r", database, address)
783+
deadline = Deadline.from_timeout_or_deadline(timeout)
820784
# should always be a resolved address
821785
connection = await self._acquire(
822786
address, deadline, liveness_check_timeout

neo4j/_async/work/workspace.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ def _set_cached_database(self, database):
7878
self._config.database = database
7979

8080
async def _connect(self, access_mode, **acquire_kwargs):
81-
timeout = Deadline(self._config.session_connection_timeout)
8281
if self._connection:
8382
# TODO: Investigate this
8483
# log.warning("FIXME: should always disconnect before connect")
@@ -100,13 +99,12 @@ async def _connect(self, access_mode, **acquire_kwargs):
10099
database=self._config.database,
101100
imp_user=self._config.impersonated_user,
102101
bookmarks=self._bookmarks,
103-
timeout=timeout,
102+
timeout=self._config.connection_acquisition_timeout,
104103
database_callback=self._set_cached_database
105104
)
106105
acquire_kwargs_ = {
107106
"access_mode": access_mode,
108-
"timeout": timeout,
109-
"acquisition_timeout": self._config.connection_acquisition_timeout,
107+
"timeout": self._config.connection_acquisition_timeout,
110108
"database": self._config.database,
111109
"bookmarks": self._bookmarks,
112110
"liveness_check_timeout": None,

neo4j/_conf.py

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -295,13 +295,6 @@ class PoolConfig(Config):
295295
connection_timeout = 30.0 # seconds
296296
# The maximum amount of time to wait for a TCP connection to be established.
297297

298-
#: Update Routing Table Timout
299-
update_routing_table_timeout = 90.0 # seconds
300-
# The maximum amount of time to wait for updating the routing table.
301-
# This includes everything necessary for this to happen.
302-
# Including opening sockets, requesting and receiving the routing table,
303-
# etc.
304-
305298
#: Trust
306299
trust = DeprecatedAlternative(
307300
"trusted_certificates", _trust_to_trusted_certificates
@@ -392,12 +385,6 @@ class WorkspaceConfig(Config):
392385
""" WorkSpace configuration.
393386
"""
394387

395-
#: Session Connection Timeout
396-
session_connection_timeout = 120.0 # seconds
397-
# The maximum amount of time to wait for a session to obtain a usable
398-
# read/write connection. This includes everything necessary for this to
399-
# happen. Including fetching routing tables, opening sockets, etc.
400-
401388
#: Connection Acquisition Timeout
402389
connection_acquisition_timeout = 60.0 # seconds
403390
# The maximum amount of time a session will wait when requesting a connection from the connection pool.

neo4j/_sync/driver.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ def driver(
8080
max_connection_lifetime: float = ...,
8181
max_connection_pool_size: int = ...,
8282
connection_timeout: float = ...,
83-
update_routing_table_timeout: float = ...,
8483
trust: t.Union[
8584
te.Literal["TRUST_ALL_CERTIFICATES"],
8685
te.Literal["TRUST_SYSTEM_CA_SIGNED_CERTIFICATES"]
@@ -97,7 +96,6 @@ def driver(
9796

9897
# undocumented/unsupported options
9998
# they may be change or removed any time without prior notice
100-
session_connection_timeout: float = ...,
10199
connection_acquisition_timeout: float = ...,
102100
max_transaction_retry_time: float = ...,
103101
initial_retry_delay: float = ...,
@@ -339,7 +337,6 @@ def encrypted(self) -> bool:
339337

340338
def session(
341339
self,
342-
session_connection_timeout: float = ...,
343340
connection_acquisition_timeout: float = ...,
344341
max_transaction_retry_time: float = ...,
345342
database: t.Optional[str] = ...,

0 commit comments

Comments
 (0)