Repository navigation
Conversation
mykaul
force-pushed
the
perf/cache-named-tuple-factory
branch
from
March 13, 2026 10:03
a945645 to
9ea1ed1
Compare
Introduce the data layer for Private Link client routes support: - ClientRoutesChangeType enum for CLIENT_ROUTES_CHANGE event types - ClientRouteProxy dataclass and ClientRoutesConfig for user-facing configuration - _Route frozen dataclass for immutable route records - _RouteStore for thread-safe route storage with atomic update/merge and preferred route selection that avoids unnecessary connection_id migration when multiple routes exist for the same host
Add _ClientRoutesHandler which manages the full lifecycle of dynamic address translation via system.client_routes: - initialize(): loads all routes at startup and on control connection reconnect - handle_client_routes_change(): processes CLIENT_ROUTES_CHANGE events with targeted merge or full refresh depending on event data - _query_all_routes_for_connections(): complete refresh query using connection_id IN (...) - _query_routes_for_change_event(): targeted query grouping by connection_id with host_id IN (...) per group - _execute_routes_query(): common query execution and result parsing with proxy address override support - resolve_host(): host_id to (address, port) resolution with DNS lookup
- ClientRoutesEndPointFactory: creates endpoints from system.peers rows by extracting host_id, deferring address translation and DNS resolution until connection time - ClientRoutesEndPoint: endpoint that resolves via _ClientRoutesHandler on each connection attempt, ensuring immediate reaction to route changes and CLIENT_ROUTES_CHANGE events
Cluster: - Add client_routes_config parameter with mutual exclusivity check against endpoint_factory - Create _ClientRoutesHandler and ClientRoutesEndPointFactory when client_routes_config is provided ControlConnection: - Register CLIENT_ROUTES_CHANGE event watcher when handler is present - Forward events to handler via _handle_client_routes_change - Trigger full route re-read on control connection reconnection
Cover ClientRouteEntry/ClientRoutesConfig validation, _RouteStore get/merge operations, _ClientRoutesHandler initialization, ClientRoutesEndPoint resolution with and without route mappings, and SSL check_hostname rejection with client_routes_config.
Add comprehensive integration tests covering: - TCP proxy and NLB emulator infrastructure for simulating private link connectivity - query_routes filtering with different connection/host ID combinations - Full private-link connectivity verifying all driver connections go exclusively through the NLB proxy - Dynamic route updates via REST API with driver reconnection through new proxy ports
Recently scylladb started to rely on the options "--auth-superuser-name" and "--auth-superuser-salted-password" to ensure that a cassandra/cassandra user exists for tests - without those options a default superuser no longer exists.
…ames Skip the regex scanner and stack-based parser in parse_casstype_args() when the type string has no parentheses. For simple types like 'AsciiType' or 'org.apache.cassandra.db.marshal.FloatType', go directly to lookup_casstype_simple() which is just a prefix strip + dict lookup. This avoids re.Scanner, re.split on ':' / '=>', int() try/except, and list-of-lists stack manipulation for the common case of non-parameterized types. Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
The time.sleep(10) in setup_keyspace() is redundant because callers already ensure the cluster is fully ready before calling it: - use_cluster() calls start_cluster_wait_for_up() which uses wait_for_binary_proto=True + wait_other_notice=True, then wait_for_node_socket() per node - External cluster path (wait=False) had no sleep anyway Remove the wait parameter entirely and its associated sleep, saving 10s per cluster startup.
Replace fixed sleeps with condition-based polling to speed up tests: - simulacron/utils.py: replace 5s sleep with HTTP endpoint polling (max 15s timeout, typically <1s) - test_authentication.py: replace 10s sleep with auth readiness poll that tries connecting with default credentials - upgrade/__init__.py: replace 10s auth sleep with same polling pattern - upgrade/test_upgrade.py: replace 3x 20s sleeps (60s total) with control connection readiness polling Total potential saving: ~95s of unconditional waiting per test run.
Replace fixed sleeps with condition-based polling in four test files: - test_shard_aware.py: replace 25s of sleeps (5+10+5+5) with wait_until_not_raised polling for reconnection after shard connection close and iptables blocking - test_metrics.py: replace 15s of sleeps (5+5+5) with polling for cluster recovery and node-down detection - test_tablets.py: replace 13s of sleeps (3+10) with polling for metadata refresh and decommission completion - simulacron/test_connection.py: replace 20s of sleeps (10+10) with polling for quiescent pool state Total potential saving: ~73s of unconditional waiting.
… for invalidation The tablet tests were intermittently failing because: 1. get_query_trace() used the default 2s max_wait, which is too short under resource pressure (--smp 2). Increased to 10s. 2. test_tablets_invalidation_decommission_non_cc_node used a fixed time.sleep(2) hoping tablet metadata invalidation would complete. Replaced with wait_until polling for the tablet record to be purged (0.5s delay, 20 attempts = 10s budget).
- test_cluster.py: replace sleep(1) x10 iterations with connect(wait_for_all_pools=True) for deterministic pool readiness - test_query.py: replace sleep(5) with wait_until polling for 'Preparing all known prepared statements' log message - test_connection.py: replace sleep(2) with wait_until polling for host_down listener notification
…superuser config Use set_configuration_options() (the Python API behind `ccm updateconf`) to set auth_superuser_name and auth_superuser_salted_password directly in the YAML config instead of passing them via the SCYLLA_EXT_OPTS environment variable.
…ema_agreement min(self._timeout, total_timeout - elapsed) raises TypeError when control_connection_timeout is set to None, which is explicitly documented as a supported value (meaning no timeout). Guard the min() call so that when self._timeout is None, we use only the remaining schema agreement wait time.
Patch reactor.running to False in setUp() so that maybe_start() always enters the branch that spawns the reactor thread. Without this, leaked global reactor state from prior tests can leave reactor.running as True, causing maybe_start() to skip thread creation and the reactor.run mock to never be called — making the assertion in test_connection_initialization fail intermittently. Observed in CI on PyPy 3.11 + macOS x86 (Rosetta 2), where timing differences make the reactor state leak more likely.
The column kind filter at line 2744 used 'clustering_key' but system_schema.columns uses 'clustering' as the kind value. This caused clustering columns to not be excluded from the 'other columns' loop, resulting in them being processed twice (once as clustering key, once as regular column). The correct value 'clustering' was already used 6 lines above in the clustering key extraction loop.
ScyllaDB doesn't support triggers, so skip the triggers query when connected to ScyllaDB. This is detected by checking if the connection has shard awareness (using the existing _is_not_scylla() method). Changes to both SchemaParserV3 and SchemaParserV4: - Modified _query_all() to conditionally append triggers query only for non-ScyllaDB - Modified _query_all() response unpacking to use array slicing for cleaner code - Modified get_table() in V3 to conditionally query triggers This eliminates unnecessary failed queries to system_schema.triggers on ScyllaDB. Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
- Fix spelling: 'tring' → 'string' in docstring - Remove extra 't' at end of comment - Refactor complex list comprehension for clarity - Use 'is None' instead of '== None' for None comparison Co-authored-by: mykaul <4655593+mykaul@users.noreply.github.com>
Co-authored-by: mykaul <4655593+mykaul@users.noreply.github.com>
mykaul
force-pushed
the
perf/cache-named-tuple-factory
branch
2 times, most recently
from
April 3, 2026 18:15
8398d42 to
9a76016
Compare
The pool module emits a DEBUG log message when selecting a connection for a query. Emitting a log message for every query is too noisy. Since Python logging lacks a TRACE level, just remove the log.
mykaul
force-pushed
the
perf/cache-named-tuple-factory
branch
from
April 7, 2026 11:04
9a76016 to
a662281
Compare
TLS clients can skip the expensive part of a handshake by replaying a session established earlier with the same peer (RFC 5077 tickets for TLS 1.2, RFC 8446 PSKs for TLS 1.3), but OpenSSL never does this on its own: the client has to hold on to the session and offer it explicitly on the next connection. Add the storage half of that: a bounded, thread-safe LRU of TLS sessions keyed by TLS peer identity, plus an EndPoint.tls_session_cache_key property that produces the key. The cache is a module of its own, cassandra.ssl_session_cache, because keeping sessions is all it does: what may be offered to whom, and for how long, belongs with the connections that work it out. A cached session is not consumed by being used -- one session can be replayed by any number of concurrent connections -- so get() leaves the entry in place and each successful handshake stores back over it whatever the peer handed over. Entries carry the lifetime the caller gives them and are dropped once it runs out, so a session is never offered past the point the peer said it would honour it; what that lifetime should be is for the caller to work out, since it depends on how the session resumes. Whether a store moves an existing deadline is decided here rather than asked of the caller: a session the peer handed back unchanged keeps the deadline it had, since its lifetime runs from when the peer issued it and not from when it was last replayed, and re-stamping a full lifetime on every reuse would let one ticket be offered for as long as connections keep being opened. Below TLS 1.3 an abbreviated handshake hands back the offered session itself, so that is the ordinary case; TLS 1.3 normally issues a fresh one, which is entitled to a lifetime of its own. The two are told apart by session id, compared under the same lock that stores the result, so no concurrent store can land in between. An entry recognised this way keeps the object holding it as well as its deadline. SSLSocket.session builds a new wrapper on every access, so what a resumed connection stores back is another handle on the credential already there; swapping one for the other would change nothing but the identity a caller compares against when it asks for its own session to be dropped. Keeping it means an entry changes identity only when it changes credential. A caller can also say which session it offered on the handshake it is storing the result of, and a store of that same session is skipped where the entry no longer holds it: another connection has stored a session the peer reissued to it, or the deadline passed and a lookup dropped the entry. In neither case has this caller anything to add, while storing it would put a deadline running from now on a session the peer issued at some earlier point -- and would replace a fresher session with an older one. Neither comparison can say anything about a ticket whose session id is empty, which RFC 5077 section 3.4 lets a server send and for which SSLSession offers no ticket to compare instead. Two of those are reported as the same session, the conservative reading: the deadline then stays where it is, where calling them different would re-stamp a full lifetime on what may well be the ticket already held. What that costs is resumption rather than correctness -- a reissued ticket inherits its predecessor's deadline, and one arriving where the entry has since gone is not stored, which the next connection puts right by offering nothing and storing afresh. SNI endpoints add the server name to their key, since they all share a proxy address and port but are distinct TLS peers. Client-routes endpoints key on the node's host_id rather than the proxy address they happen to resolve to at the moment. Nothing uses the cache yet. Refs DRIVER-165
Offer the cached session for the endpoint before the handshake, and store the negotiated session once the connection is up, so that the next connection to the same node -- in particular the burst of per-shard connections a pool opens at once -- can skip the certificate exchange and signature of a full handshake. The session is stored from the ReadyMessage / AuthSuccessMessage handlers rather than right after the handshake. A TLS 1.3 server sends its NewSessionTicket as a post-handshake message, so a session read straight after connect() carries no ticket and would not resume; by the time the CQL handshake has completed the ticket has been read off the socket. Storing is idempotent, so nothing needs to track whether it already happened, and every failure in this path is logged and dropped: resumption is an optimisation, and both call sites are wrapped in @defunct_on_error, where a raised exception would kill a healthy connection. How long a session may be offered is worked out here, because it depends on how the session resumes: a ticket's lifetime is the one the server announced, while SSLSession.timeout is only the local context's default and says nothing about what the peer will still accept, so it is used solely for a session that resumes by id. RFC 8446 section 4.6.1 also caps the client at seven days however long the server asked for. A zero lifetime is read against the negotiated version, since the two RFCs disagree on it: TLS 1.3 says discard the ticket immediately, while RFC 5077 section 3.3 reserves zero for "lifetime unspecified" and leaves retention to local policy, so a TLS 1.2 ticket is kept and timed by the local timeout. The version also rules out caching a TLS 1.3 session that carries only an id: resumption there is the ticket's pre-shared key, while the id a TLS 1.3 handshake carries is the legacy_session_id_echo a server sends back for the middlebox compatibility mode of RFC 8446 appendix D.4, which resumes nothing. OpenSSL reports no id at all until a NewSessionTicket has been read, so that session is not one it produces; stating the rule against the negotiated version rather than against what one library exposes is what makes it hold regardless. OpenSSL does not apply either limit on the client's behalf -- it will offer an expired ticket and let the server refuse it. What the entry gets is what is left of the announced lifetime: the peer issued the ticket during the handshake, while the store happens a startup exchange and perhaps an authentication later, and that time is the peer's to spend rather than the entry's. The age is measured from a monotonic mark taken as the handshake began, not from SSLSession.time, which is a wall-clock stamp: subtracting that would let a clock step landing in between decide the answer -- far enough forward and nothing is cached at all, backward and the age disappears. The deadline the cache keeps is monotonic too, so nothing after the store can skew it either. A pool reaches a shard-aware node on a second port, which would otherwise key those connections separately from the one the control connection established, leaving the whole per-shard burst to handshake in full. The endpoint alias that _get_shard_aware_endpoint already builds for that port therefore carries the node's cache key, so both listeners share one session and nothing has to be threaded through the connection factory. The port stays part of the key by default, so two unrelated TLS servers on one address still cannot share a session; only an endpoint that names another node is exempt. The SSLContext is part of the key because a session cannot be replayed onto a different one and a cache may be shared by several clusters. It is held strongly there: a cached session already keeps its context alive on its own -- CPython's SSLSession holds a reference to the context it was established with -- so holding it weakly here would buy nothing. The key also carries the name wrap_socket() is given, which is the name the peer certificate is verified against. A resumed handshake sends no Certificate, so that name is never checked again; offering a session to a connection expecting a different name would silently skip hostname verification for it. Both the key and wrap_socket() take the name from one accessor so the two cannot drift apart. A session offered on a connection whose handshake then failed is dropped from the cache. Both RFCs have a server fall back to a full handshake rather than fail when it will not resume, so this should not happen; but nothing stores a fresh session for a connection that never came up, so an entry that did provoke a failure would otherwise be offered again by every later connection until its lifetime ran out. The handshake need not fail where it is started: ssl_options may carry do_handshake_on_connect=False, which leaves it to the first read or write and so to the reactor, and a failure there arrives at defunct() instead -- which retracts for the same reason and under the same rule. Only a TLS error counts: a refused or reset connection says nothing about the session. And only the session this connection offered goes: connections to one node are opened together, so another may have stored a session the peer issued in its place, and that one failed nothing. A sibling storing the same session back is not that, and leaves the entry retractable, because the cache keeps the object it already holds for a session it recognises. Nor does an endpoint that borrowed its key retract anything: the shard-aware listener resumes the node's session and stores back to it, which is the point of sharing the key, but the entry belongs to the endpoint that owns it. Dropping it there would cost the node's other pools and its control connection a handshake apiece, and a pool filling against that listener would do it again on every retry, while a session that genuinely cannot resume fails on the owner too and goes from there. What was offered is kept until the store, which hands it to the cache and then lets it go. Telling a session the peer reissued from the one that came back unchanged needs both sides of the exchange, and where connections to a node are opened together only the connection that offered one knows the second. Letting it go there is what bounds the retraction above: reaching the store means the CQL handshake completed, so the TLS one stood, and a TLS error long afterwards has nothing of this connection's to withdraw. Three accessors are the whole of what a reactor whose TLS does not go through the stdlib ssl module has to reimplement to take part: the policy around them asks one for the session to store, one for the negotiated version and one to restore a session onto a socket, and reads nothing off a socket itself. Connections whose SSLContext is derived from ssl_options do not participate, because a session cannot be replayed onto a different context and each of those connections builds its own. The asyncio reactor opts out entirely: its handshake happens inside loop.create_connection(), with no point at which a session could be restored. Refs DRIVER-165
Create an SSLSessionCache per Cluster whenever TLS is configured through ssl_context, and hand it to every connection the cluster opens, so that resumption is on by default with no configuration. Pass ssl_session_cache=None to turn it off, or an instance of your own to size it or share it between clusters. No cache is created where resumption cannot work: the deprecated ssl_options-only path, whose per-connection SSLContexts a session cannot be replayed onto, and reactors that report they cannot restore a session before the handshake, which today means asyncio -- which is also what the default connection class resolves to on Python 3.12 and newer with no libev extension installed, asyncore having left the standard library there. connection_class is not required to derive from Connection, so one that does not report the capability at all is treated as lacking it rather than raising. Nothing is settled in advance. ssl_session_cache is a property, answering against whatever ssl_context and connection_class are in force when it is read and making a cache on first use where one is wanted -- once, under a lock of its own, however many threads ask at the same moment, since for a cluster given TLS after connect() the first to ask are whichever pools are opening connections. Both of those are public attributes, so a decision taken at construction would leave resumption off on a reactor that does support it, or hand the keyword to a connection class that does not take it -- and one retaken later needs the first to be remembered, which is how an explicit None came to be overwritten. Behind the property is what the caller set and nothing else: assigning None turns resumption off and leaves it off, and assigning a cache asks for one as much as passing it to the constructor does, whenever it is done. A cache the caller asked for that cannot be used reads back as None, and connect() says why. Asking for resumption and silently getting none is worse than not having it: a cache answered back here would be handed to every connection -- which a connection class that does not take the keyword cannot even accept -- and would sit reachable and empty for anyone reading it back, which is also what a server that issues no tickets looks like. Only what the caller set is worth interrupting anyone over, since a cache made for a cluster is only ever made where it can be used; turning TLS off afterwards is not something to complain about. Where nothing was asked for there is still something worth finding -- resumption is on by default wherever it works, so a cluster that configured TLS and will not get it is a surprise -- and that is a line at debug rather than a warning. A cluster with no TLS at all is told nothing, having nothing to resume. connect() is the one place that says any of it, so it is said once without anything having to record that it was; a cache assigned after that reads back as None just the same, without a second word about it. Saying it from the setter instead would speak too early, while the caller is still configuring -- assigning the cache before the context is an ordering that owes nobody a warning. Whose the cache is settles what becomes of it, and nothing has to track that either: the field behind the property holds what the caller set, and one made for the cluster is kept apart from it. One created here is reachable only through the attribute, so it and the sessions in it go when the cluster does and shutdown has nothing to do. One the caller supplied stays the caller's: shutdown leaves its entries alone, which is what lets clusters share sessions -- at the same time, or one after another, so that a cluster replacing an earlier one resumes rather than handshaking in full -- and keeps the driver from deleting rows in an object it does not own. Its entries hold the SSLContext their session was established with, bounded by the cache's max_size, and clear() is there for a caller who wants them gone sooner. Refs DRIVER-165
Stand up a TLS server on loopback and connect to it with the driver's own socket setup, so the restore-before-handshake and store-after-startup paths run for real and the result is read back the way OpenSSL reports it, through SSLSocket.session_reused. Covers TLS 1.2 and TLS 1.3, the latter skipped where the local OpenSSL does not offer it -- skipping the subclass rather than the base, since a skipped base would take its subclasses with it. The certificates come from tests/tls_certificates.py rather than from here, so that the integration suite generates its own the same way -- the addresses to name are the caller's argument, which is what that suite needs and this one takes the loopback default for. cryptography is optional, so that module reports whether it found it and both suites skip on that. One waits for the monotonic clock to move before opening its second connection. What a store works out as the deadline is the mark taken when the handshake began plus the lifetime announced -- the two readings of the clock cancel -- so two handshakes it cannot tell apart are given the same deadline, which is right but leaves a comparison of the two nothing to see. Windows resolves that clock to about ten milliseconds, and two loopback handshakes fit inside it. Two of these pin down behaviour that is easy to regress: that four connections opened at once all resume from the single cached session -- the per-shard burst DRIVER-165 is about -- and that on TLS 1.3 nothing is cached until the server's NewSessionTicket has actually been read off the socket. Refs DRIVER-165
Restart the cluster with client encryption on, warm a session cache with one cluster, then hand it to a second one and require every connection it opens to have resumed -- which is the question only a real server can answer: whether it accepts one session offered concurrently by the whole batch of per-shard connections. The cluster is given a shard-aware TLS port, since that is the port those per-shard connections use and therefore where resumption has to pay off; Scylla leaves it unset by default. The certificate names every node rather than only the contact point, or the driver could not build pools to the rest of the cluster and the test would quietly examine a single host. Each Session is held for the duration of a test: Cluster.sessions is a WeakSet, so a dropped Session takes its pools -- everything worth inspecting -- with it and leaves only the control connection behind. The number of connections collected is asserted before their resumption flags, so the test cannot pass by examining almost nothing. A connection that has lost its socket is left out of the reading rather than counted as not having resumed -- SSLSocket.session_reused answers None once the socket is closed, which would read as a resumption failure for a connection that is simply gone -- and it is that count which keeps leaving one out from quietly shrinking the sample. The key and certificate outlive the run when KEEP_TEST_CLUSTER does: remove_cluster() is a no-op there, and what it leaves behind still names those files. Whether there is anything here to test at all depends on the reactor, so the skip asks the connection class Cluster will instantiate rather than reading EVENT_LOOP_MANAGER: with no selector set and no libev to import, that class is the asyncio reactor, which cannot restore a session before the handshake. Follows the reconfigure-and-remove pattern the other modules here use for cluster-level options, and generates the server certificate with cryptography, through the generator the loopback suite also uses, so the test does not depend on an openssl binary. Refs DRIVER-165
Resumption is on by default and has prerequisites on both sides, which until now were described only in the ssl_session_cache docstring -- read by someone already looking at the attribute, rather than by someone setting up TLS. Give it a section in the security guide, next to the SSL configuration it belongs with, covering what the server and the reactor each have to provide -- and what resumption costs, which is the part a security guide owes a reader most. Scylla only issues session tickets when enable_session_tickets is set in client_encryption_options. That defaults to true from Scylla 2026.3 and to false in the releases before it, so on an earlier one a cluster has to be told to issue them; without them the cache stays empty and every connection performs a full handshake, with no indication of why. What it costs is verification. A resumed handshake carries no Certificate message, so nothing about the server's certificate is checked again while a cached session is offered -- not its expiry, and not a revocation list the context carries -- and one that expires or is revoked goes on being accepted until the entry does, bounded by the lifetime the server announced and the driver's seven-day cap. The hostname is the exception, and only because the cache is keyed by the name verified when the session was established, so a session is never offered where a different name is expected. Someone who cannot accept that window is told how to turn resumption off. The reactor and server paragraphs move off the class rather than being copied, leaving the docstring to the semantics of the attribute -- what it holds, whose it is, how to turn it off or size it -- and a pointer to the guide for what has to be true for any of it to happen. Refs DRIVER-165
Bind application use of the shared control connection to one keyspace so fallback queries from different sessions cannot leak USE state into one another. Reject explicit USE and select the bound keyspace before sending application requests. Reclaim the binding only after its owners are gone and its fallback requests have drained. Track that traffic independently from control traffic, unwind a new claim when no request is sent, and keep protocol encoding outside the binding lock. Fixes scylladb#1013.
Release logical fallback accounting when a request times out while retaining an orphan barrier until its late response can no longer change the physical keyspace. Prevent active self-rebinding and validate reclaimed bindings against the connection's actual keyspace. Limit CQL USE detection to CQL statements so valid graph queries are not rejected.
Keep provisional session claims until all concurrent initial sends resolve, so a failed first send cannot release ownership claimed by another request. Cover the overlap where one send fails while another is pending, then verify different keyspaces remain rejected and the original session can continue.
Record pooled request IDs before sending so a speculative timeout can detach the callback for the stream actually in flight. Track active control-connection fallback callbacks and verify their identity before orphaning a stream. This keeps recycled control request IDs and fallback accounting untouched after the original response has completed.
Authentication failures and exhausted retry schedules left their handlers installed, so every later DOWN event treated a stopped handler as active and permanently skipped the host. Release terminal handlers without clearing a concurrent replacement, let later DOWN events start a fresh retry cycle, and serialize pending lifecycle transitions so stale restart work cannot replace newer recovery state. Fixes scylladb#1026
Clear the pending host addition atomically with marking the host up, so listener callbacks observe a completed lifecycle. This prevents concurrent DOWN handling from starting an add-mode reconnector and ensures listener exceptions cannot leave the flag stuck.\n\nCover both cases by sending DOWN and raising from an on_add listener.
…#1024) * control-connection: reconnect when DOWN handling is skipped ControlConnection._signal_error() treated a conviction-policy decision as proof that Cluster.on_down() had dispatched its asynchronous callbacks. DOWN handling can instead be suppressed by an open session pool, existing host state, disabled pool creation, shutdown, or a rejected executor submission, leaving the defunct control connection without recovery work. Report whether DOWN handling was actually queued and reconnect directly when it was not. Keep callback-driven recovery for accepted transitions so the normal host-down path does not submit a duplicate attempt, and cover TCP, Unix-socket, alternate-route, and stable host-ID connections. Fixes scylladb#847 * control-connection: preserve reconnect ownership The direct recovery path can overlap queued attempts, heartbeat returns, topology-driven replacements, and finite retry schedules. Without a single owner for pending and delayed work, later triggers can reset backoff, stale handlers can replace healthy connections, and exhausted handlers can suppress an independent failure. Serialize reconnect submissions, retain triggers until an active attempt or handler owns them, and release handler slots with identity checks. Preserve failures that arrive during proactive retries, honor empty and exhausted schedules, and keep a successfully restored control connection open while still closing host-probe connections. Make connection installation and shutdown races explicit so an overlapping failed attempt cannot park retries over a newer connection and an orphaned connection is closed. * changelog: document control-connection recovery * control-connection: preserve DOWN recovery callbacks Isolate session and listener failures so asynchronous DOWN handling still starts host reconnection after reporting successful dispatch. Keep heartbeat submission outside the reconnection lock and use the control-handler type invariant directly.\n\nDocument policy-driven retry cadence and finite-schedule exhaustion. * control-connection: preserve overlapping retry state A zero-delay retry can start on another executor worker before the previous handler invocation reaches its finally block. Prevent the stale invocation from clearing the newer run's active marker so heartbeat triggers during a final failed attempt remain eligible for recovery. * control-connection: clarify finite retry exhaustion A reconnect trigger received while the final scheduled attempt is running starts a fresh schedule if that attempt fails. Document this timing-dependent exception while retaining the distinction for heartbeat returns after exhaustion.
…out-7.x Update actions/checkout action to v7
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Bumps [anyio](https://github.com/agronholm/anyio) from 4.13.0 to 4.14.2. - [Release notes](https://github.com/agronholm/anyio/releases) - [Commits](agronholm/anyio@4.13.0...4.14.2) --- updated-dependencies: - dependency-name: anyio dependency-version: 4.14.2 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* build(deps): remove vulnerable Python 3.9 resolutions Align package metadata with the existing Python 3.9 support removal so uv no longer retains unsupported compatibility branches containing vulnerable dependency versions. Require the patched pytest 9.1 line using a compatible-release specifier. This avoids a literal "<10", which cibuildwheel passes through cmd.exe as input redirection when installing test dependencies on Windows. * tests: fix timestamp comparison under pytest 9 The vector round-trip helper calls comparison callbacks with observed and expected values. Passing both to a partial pytest.approx call made expected the relative-tolerance argument and discarded the returned matcher, so the timestamp path performed no assertion under pytest 8. Pytest 9 rejects relative tolerance for datetimes and exposes that misuse. Compare observed with the matcher explicitly and disable relative tolerance so timestamps retain their intended one-second absolute tolerance.
* cluster: stop schedulers before executor teardown ThreadPoolExecutor disables new submissions before ordinary atexit callbacks run. A cluster scheduler can otherwise wake during interpreter shutdown and fail while submitting work to its executor. Register scheduler cleanup through the threading shutdown hook, which runs before executor teardown, while keeping full cluster cleanup in regular atexit so application threads finish before connections close. Coordinate cleanup with cluster registration so a non-daemon thread cannot register a scheduler after the cleanup snapshot. Preserve warm imports after threading shutdown starts and allow remaining shutdown callbacks to run when scheduler cleanup fails. Add isolated subprocess coverage for shutdown ordering, late registration, cleanup failures, and warm late imports. * cluster: reject connects after late import * cluster: fail retries when scheduler stops * cluster: serialize retry abort completion Finalize scheduler-aborted retries under the ResponseFuture callback lock so a concurrent successful response cannot invoke both callback paths or replace the shutdown outcome. Preserve the existing behavior for non-shutdown completions, where a late successful response may supersede an earlier error. Add coverage for both completion orders. * control-connection: fail fallback USE retry on shutdown Route the internal keyspace-selection retry through the shutdown-aware scheduler so session or executor teardown cannot strand a response future. Add coverage for retries arriving after session shutdown and scheduler rejection. * cluster: harden scheduler shutdown completion Make scheduler shutdown wait for an in-flight owner and drain queued callbacks only after its thread exits. This keeps concurrent cluster cleanup ordered without deadlocking callbacks that re-enter shutdown. Scope retry-abort finalization to the result page that scheduled it. Preserve ordinary response completion semantics, reject paging after terminal scheduler shutdown, and ignore stale abort callbacks from completed pages. * control-connection: exclude internal USE from retry metrics Keep the fallback keyspace-selection retry on the shutdown-aware scheduler without reporting it as a RetryPolicy decision. Preserve metric accounting for ordinary policy retries and cover both paths. * cluster: make scheduler rejection explicit Return late shutdown state from cluster registration so connect owns the cleanup and error at the call site without weakening the atomic registry check. Propagate schedule_unique acceptance consistently with other scheduler entry points. Limit the sentinel test time mock to the cluster module so unrelated background threads cannot make it fail on Windows. * cluster: release scheduler lock before submission Claim and remove due tasks while holding the scheduler lock, then submit them after releasing it. Scheduler shutdown still joins the thread before executor teardown, preserving ordering while allowing event-loop threads to schedule without blocking behind executor submission. Cover the lock boundary with a blocking executor submission and a concurrent schedule call. * cluster: harden scheduler shutdown edge cases Clear inherited cluster registrations and replace their lock in fork children so child thread teardown cannot run parent callbacks or deadlock on locks held at fork. Allow child-created clusters to register normally. Treat executor submission rejection as terminal scheduler shutdown. Requeue the claimed task for ordered draining so concurrent shutdown owners and reentrant callbacks complete without stranding retries. Cover both paths with fork and concurrency regression tests. * cluster: reject late connects after scheduler cleanup Check the global scheduler-shutdown state on every Cluster.connect call so an initialized cluster cannot create a new session after its scheduler has stopped. Preserve the existing first-connect reactor initialization order. Cover the repeated-connect path with an isolated shutdown subprocess. * cluster: avoid executor joins on late connects Reject repeated connects after scheduler cleanup without closing an initialized cluster before application threads finish. Keep its registered ordinary atexit cleanup. For fresh clusters, claim shutdown under the cluster lock and clean up after releasing it. Once scheduler cleanup starts, avoid waiting on executor teardown so callbacks can re-enter shutdown without deadlocking or joining themselves. Cover own-executor late connects and reentrant shutdown. * cluster: narrow scheduler shutdown handling Keep interpreter lifecycle ordering and queued statement retry completion in this change. Restore the existing control-connection fallback handoff, executor-rejection behavior, and schedule_unique contract so their separate failure semantics can be handled independently. Defer internal handoff completion to scylladb#1044 and executor-rejection recovery to scylladb#1045.
Refresh the docs lock with patched anyio, soupsieve, tornado, and aiohttp releases. This clears the docs Dependabot alerts and removes related aiohttp vulnerabilities from the docs environment.
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…repeated exec() calls Cache the Row namedtuple class keyed on tuple(colnames) so Python's namedtuple() (which internally calls exec()) is only invoked once per unique column schema. For prepared statements the column names never change, eliminating redundant class creation on every result set. This applies to both the pure-Python and Cython builds: row decoding is done in Cython, but the row factory is still called per result. The cache is a functools.lru_cache (maxsize=10000) on a helper keyed on the raw column names, so differing case, order or contents produce distinct classes. lru_cache is thread-safe, including on free-threaded Python, and keeps memory bounded for applications running many ad hoc queries with varying schemas. named_tuple_factory itself stays a plain function, since ResultSet.was_applied compares against it by identity. The positional-name fallback for invalid identifiers is cached too, so its warning is logged once per schema. The SyntaxError fallback to pseudo_namedtuple_factory is removed: it only covered the 255-field namedtuple limit of Python 3.0-3.6, and requires-python is >=3.10. Cache correctness tests live in tests/unit/test_row_factories.py (run in CI); benchmarks/ contains only the timing benchmarks, and pytest-benchmark is added to the dev dependency group to run them. Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com>
mykaul
force-pushed
the
perf/cache-named-tuple-factory
branch
from
September 28, 2026 11:37
ef0bb90 to
afe4203
Compare
- replace the lru_cache with a bounded FIFO cache guarded by an RLock: concurrent misses of a cold schema now all receive the same Row class, while the hit path stays lock-free (dkropachev review). - lower the cache bound to 1024 classes and cover eviction in a unit test (dkropachev/CodeRabbit review). - cover the invalid-identifier sanitization path and that its warning is logged once per schema (dkropachev review). - correct the pseudo_namedtuple_factory docs: it is no longer an automatic fallback of named_tuple_factory (dkropachev review). Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com>
mykaul
force-pushed
the
perf/cache-named-tuple-factory
branch
from
October 1, 2026 11:40
00c62af to
003c6b1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rownamedtuple class innamed_tuple_factory()keyed ontuple(colnames), so Python'snamedtuple()(which internally callsexec()) is only invoked once per unique column schemaMotivation
named_tuple_factoryis the defaultrow_factoryin the driver. Every call tonamedtuple('Row', columns)internally callsexec()to generate a new class — this is surprisingly expensive. For prepared statements executing the same query repeatedly, the column names never change, yet we pay thenamedtuple()+exec()cost on every result set.Benchmark results
Benchmarks compare the original code (Before) against the new cached implementation (After).
10 columns, 1 row (isolates class creation overhead):
5 columns, 100 rows:
10 columns, 100 rows:
Design notes
dictkeyed ontuple(colnames)(raw column names before cleaning)SyntaxError,Exception) preserved unchangedTests
All existing unit tests pass (46 passed).