Repository navigation
Conversation
mykaul
force-pushed
the
perf/cache-apply-parameters
branch
from
March 13, 2026 12:03
6057c76 to
6556867
Compare
mykaul
marked this pull request as draft
March 13, 2026 12:15
There was a problem hiding this comment.
Pull request overview
This PR improves performance in the CQL type system by caching _CassandraType.apply_parameters() results so repeated construction of parameterized CQL type classes reuses stable singleton class objects instead of repeatedly calling type().
Changes:
- Add a class-level cache to
_CassandraType.apply_parameters()keyed by(cls, subtypes, names)to reuse generated parameterized type classes. - Add a new pytest-based benchmark module to measure cached vs uncached
apply_parameters()performance and include basic correctness checks.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cassandra/cqltypes.py | Introduces _apply_parameters_cache and uses it in _CassandraType.apply_parameters() to avoid repeated dynamic class creation. |
| benchmarks/test_apply_parameters_benchmark.py | Adds performance benchmarks (and some correctness assertions) for cached vs uncached apply_parameters(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+375
to
+377
| subtypes = tuple(subtypes) | ||
| cache_key = (cls, subtypes, tuple(names) if names else names) | ||
| cached = cls._apply_parameters_cache.get(cache_key) |
Comment on lines
+375
to
+383
| subtypes = tuple(subtypes) | ||
| cache_key = (cls, subtypes, tuple(names) if names else names) | ||
| cached = cls._apply_parameters_cache.get(cache_key) | ||
| if cached is not None: | ||
| return cached | ||
| newname = cls.cass_parameterized_type_with(subtypes) | ||
| return type(newname, (cls,), {'subtypes': subtypes, 'cassname': cls.cassname, 'fieldnames': names}) | ||
| result = type(newname, (cls,), {'subtypes': subtypes, 'cassname': cls.cassname, 'fieldnames': names}) | ||
| cls._apply_parameters_cache[cache_key] = result | ||
| return result |
| """ | ||
|
|
||
| import pytest | ||
|
|
Comment on lines
+203
to
+250
| # --------------------------------------------------------------------------- | ||
| # Correctness tests | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def test_cached_returns_same_object(): | ||
| """Cached apply_parameters returns the exact same type object.""" | ||
| a = cqltypes.ListType.apply_parameters((cqltypes.UTF8Type,)) | ||
| b = cqltypes.ListType.apply_parameters((cqltypes.UTF8Type,)) | ||
| assert a is b | ||
|
|
||
|
|
||
| def test_different_params_different_types(): | ||
| """Different subtypes produce different cached types.""" | ||
| a = cqltypes.ListType.apply_parameters((cqltypes.UTF8Type,)) | ||
| b = cqltypes.ListType.apply_parameters((cqltypes.Int32Type,)) | ||
| assert a is not b | ||
| assert a.subtypes == (cqltypes.UTF8Type,) | ||
| assert b.subtypes == (cqltypes.Int32Type,) | ||
|
|
||
|
|
||
| def test_cached_type_attributes(): | ||
| """Cached type has correct attributes.""" | ||
| t = cqltypes.MapType.apply_parameters((cqltypes.UTF8Type, cqltypes.LongType)) | ||
| assert t.subtypes == (cqltypes.UTF8Type, cqltypes.LongType) | ||
| assert issubclass(t, cqltypes.MapType) | ||
| assert t.cassname == cqltypes.MapType.cassname | ||
|
|
||
|
|
||
| def test_cached_matches_uncached(): | ||
| """Cached version produces equivalent types to uncached.""" | ||
| for cls, subtypes in PARAM_TYPE_SPECS: | ||
| cached = cls.apply_parameters(subtypes) | ||
| uncached = apply_parameters_uncached(cls, subtypes) | ||
|
|
||
| assert cached.subtypes == uncached.subtypes | ||
| assert cached.cassname == uncached.cassname | ||
| assert issubclass(cached, cls) | ||
| assert issubclass(uncached, cls) | ||
|
|
||
|
|
||
| def test_nested_parameterized_types(): | ||
| """Nested parameterized types (e.g. List<Map<text,int>>) are cached.""" | ||
| inner = cqltypes.MapType.apply_parameters((cqltypes.UTF8Type, cqltypes.Int32Type)) | ||
| outer1 = cqltypes.ListType.apply_parameters((inner,)) | ||
| outer2 = cqltypes.ListType.apply_parameters((inner,)) | ||
| assert outer1 is outer2 | ||
| assert outer1.subtypes == (inner,) |
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>
) For tablet tables, TokenAwarePolicy built the replica list by filtering the child policy's plan, which is round-robin rotated on every call. LWT/serial queries skip shuffling so that every request for a key goes to the same Paxos coordinator, but the rotation defeated that: successive LWTs on one key started at different replicas. Use tablet.replicas order for LWT/serial queries, matching the vnode path, which already uses get_replicas() order. Distance bucketing is unchanged. Fixes: scylladb#781 Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com>
…anent failures (scylladb#632) * Initial plan Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com> * Fix TruncateError retry behavior - stop retrying on truncate errors Co-authored-by: mykaul <4655593+mykaul@users.noreply.github.com> Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com> * Add unit tests for TruncateError message handling Co-authored-by: mykaul <4655593+mykaul@users.noreply.github.com> Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com> * Address code review feedback - simplify to_exception call and clarify test comment Co-authored-by: mykaul <4655593+mykaul@users.noreply.github.com> Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com> * Refactor TruncateError handling for better readability Keep TruncateError in the error tuple but check for it specifically within the block to avoid retry logic. This makes it clearer that TruncateError is similar to other request errors but requires different handling. Co-authored-by: mykaul <4655593+mykaul@users.noreply.github.com> Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com> --------- Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mykaul <4655593+mykaul@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* cluster: collect host pool futures before callbacks Schedule pool creation for every session before registering completion callbacks during ADD and UP handling. Future.add_done_callback() can run inline for an already completed future, so registering callbacks inside the scheduling loop could mark a host up and notify listeners before later sessions had even started their pools. * cluster: fence asynchronous work after host removal Mark removed Host instances terminal while holding their lifecycle lock, and pair that state with pool publication and reconnector installation. Discard work that loses the race with removal so delayed tasks cannot recreate pools or retry handlers for metadata that no longer exists. * control-connection: recover same-endpoint host replacements Treat an endpoint that reports a different host ID as a node replacement and let the replacement addition own pool and control-connection reconciliation. Aggregate every session result, remove partial pools, and restore other newly eligible pools so a failed replacement cannot race duplicate work or strand the endpoint. Carry replacement-aware callbacks through reconnector handoffs, refresh delayed retries, and invalidate stale DOWN work. This preserves recovery semantics until the replacement succeeds or is removed, including authentication failures and temporary IGNORED policy distances. * host: make host_id the canonical identity Require a non-nil UUID host ID, install it before constructing the conviction policy, and use the read-only ID for Host equality, hashing, and ordering. This separates immutable node identity from mutable connection endpoints and keeps Host-keyed pools reachable when routes change. Derive current hosts from pool values where equal-key dictionary updates can retain an older Host object, and update metrics, topology helpers, tests, and documentation for the new identity contract. * tests: distinguish control and data timeout cleanup Model a data pool appearing while a fallback request is in flight and explicitly identify ordinary timeout connections as non-control. Verify timeout cleanup keeps each stream on its owning path and never returns the shared control connection through a newly visible data pool. * cluster: keep removed additions terminal Abort add finalization when lifecycle removal has already won the host lock. A replacement retry can refresh topology and remove its Host before reaching the IGNORED fast path; without this guard it marks the stale object up and emits on_add after on_remove. Document the visible same-endpoint replacement lifecycle and update control-connection tests added on master to use the rebased UUID fixtures.
…ladb#1055) * auth: preserve OVERLOADED during connection setup Keep OVERLOADED responses from legacy credentials and SASL handshakes as server errors so contact-point failover and host reconnection treat them as transient. Continue converting other authentication failures to AuthenticationFailed. Add unit coverage for overload and bad credentials on both handshake paths. Fixes scylladb#1054 * auth: test host reconnection after overload Cover OverloadedErrorMessage in the reconnection handler retry case so authentication overloads keep their active retry slot.
…lladb#802) _Frame is instantiated for every response frame received from the server. Adding __slots__ eliminates the per-instance __dict__ allocation (~104 bytes on CPython), reducing memory pressure on high-throughput workloads. _Frame only has 6 fixed attributes (version, flags, stream, opcode, body_offset, end_pos) and is never monkey-patched or dynamically extended. Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
* ci: use cibuildwheel with PyPy 8 support cibuildwheel 4.2.1 provisions PyPy 7.3.23 on Windows, but cryptography 50.0.2 publishes its Windows PyPy wheel for the pp80 ABI. Dependency installation otherwise falls back to a source build without OpenSSL. Use the merged cibuildwheel PyPy 8 update on Windows until it is released. Keep non-Windows jobs on 4.2.1 and preserve the current Windows build targets. * ci: fix Windows CIBW_BUILD export
…ylladb#1063) The CQL integer types are signed, but the test generated random values up to the *unsigned* max: random.randint(0, 2**31) for int, 2**63 for bigint/varint and 2**15 for smallint. Whenever the random draw hit the exact upper bound, value serialization raised: struct.error: 'h' format requires -32768 <= number <= 32767 since the bound equals signed_max + 1. This made the test flaky (it passed on the unlucky-draw-free runs, e.g. it failed on one 3.12 integration job). Use the signed maxima: 2**31 - 1, 2**63 - 1 and 2**15 - 1 (tinyint already used 2**7 - 1).
* Installation doc formatting fix (cherry picked from commit 434b1f5) * Update README.rst to add python version badge (apache#1237) Adapt the badge to the scylla-driver package. (cherry picked from commit d7f4541) * Removed 2.7 Cpython defines (apache#1252) (cherry picked from commit dd2343d) * PYTHON-1419 Distribute SNI endpoints across proxy addresses (apache#1243) Preserve Scylla node-domain and TLS session-cache behavior while assigning each endpoint a different initial proxy address. (cherry picked from commit 1fd9c82) * Add execute_concurrent_async and expose concurrent APIs on Session (apache#1229) Make execute_concurrent_async genuinely nonblocking and preserve input order. Handle empty inputs, invalid concurrency, fail-fast races, iterator failures, synchronous errbacks, and late callbacks without hanging or completing the aggregate future twice. (cherry picked from commit 84ece0b) * Fix spelling issues in Python comments and messages (apache#1231) Adapt wording to the current Scylla code and keep corrections grammatically accurate. (cherry picked from commit ef176a5) * docs: replace stale DataStax driver links (apache#1258) Point runtime guidance and object-mapper examples at the Scylla driver documentation. (cherry picked from commit a9d21fc) * Document applicable upstream 3.29.3 changes Record the public concurrent-execution API and SNI connection fix without downgrading the Scylla driver version or replacing its release history. Adapted from upstream commit 27a1770. * concurrent: honor execution profile request timeout Leave child request timeouts unset so Session.execute_async applies the selected execution profile's request_timeout. This prevents a stalled child from keeping execute_concurrent_async pending indefinitely. * concurrent: forward fatal pump failures Capture BaseException failures from background pump work so the aggregate future completes instead of remaining pending when input iteration or request submission aborts. * concurrent: harden aggregate completion paths Complete aggregate futures outside the cluster executor so callbacks can shut down their cluster without joining the current worker. If the completion thread cannot start, complete inline so the Future never stays pending. Read input one item ahead and consume it without holding the executor lock. Exhaustion is then known at full concurrency, so a Session shutdown after the final response still reports a completed batch as successful, and response callbacks never wait behind user iterator code or request submission. Use None as the stored-exception sentinel so falsey exceptions are raised instead of returning partial results. Validate concurrency the same way for the sync and async entry points, and document the timeout, fail-fast, input consumption and cancellation semantics of the Session concurrent APIs. * concurrent: fix fail-fast and exhaustion races Raise failures recorded during initial fill before waiting for earlier in-flight requests, preserving fail-fast behavior when a child has no timeout. Treat StopIteration as terminal across async pump turns so resumed iterators cannot submit late statements. Clarify legacy and execution-profile timeout selection in public docs. --------- Co-authored-by: absurdfarce <bret.mcguire@datastax.com> Co-authored-by: Brad Schoening <5796692+bschoening@users.noreply.github.com> Co-authored-by: Lukasz Antoniak <lukasz.antoniak@gmail.com> Co-authored-by: Jonathan Ellis <jbellis@gmail.com> Co-authored-by: Bret McGuire <absurdfarce@users.noreply.github.com>
…ror (scylladb#1068) test_no_connection_refused_on_timeout formatted failures with result.message, which only some exceptions have (NoHostAvailable does, InvalidRequest does not). An unexpected exception therefore surfaced as AttributeError and hid the real server error. Use the exception itself. The xfail (scylladb/scylladb#18068, tablets, Scylla < 2025.4) used raises=AttributeError and only matched through that accident: LWT gives InvalidRequest there. pytest.fail raises Failed, so match on pytest.fail.Exception to keep the xfail working. Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Regenerate the root development lock with the patched urllib3 release. This resolves CVE-2026-97687, CVE-2026-97688, and CVE-2026-97689 without changing shipped driver dependencies or the docs lock.
* protocol: load NumPy handler lazily Keep ordinary Cluster imports from initializing optional NumPy and its parser. Resolve and cache the availability probe and protocol handler only when callers explicitly request them, while preserving the existing class-or-None API. Add isolated subprocess coverage for compiled, missing-NumPy, and missing-Cython paths. Fixes scylladb#1067 * protocol: harden lazy NumPy handler initialization Publish the handler through the module dictionary without holding a lock across optional imports. This avoids inheriting a locked state after fork. Read module state through sys.modules so Cython versions before 3 do not re-enter the module hooks through globals(). Share isolated subprocess setup and make the positive NumPy test independent of the availability probe under test.
* docs: add connectivity and client routes guide Document direct, VPC-peered, and per-node proxy connectivity, including network reachability, shard-aware ports, datacenter-aware routing, TLS, topology checks, route provisioning, and dynamic Client Routes. Add API reference coverage for the public configuration types and align proxy collection annotations with runtime behavior. Fixes scylladb#892 * docs: clarify connectivity behavior Document Client Routes selection, TLS validation, shard-aware forwarding, and direct fallback semantics. Correct ScyllaDB Cloud and direct TLS guidance.
* CASSPYTHON-10 Update cassandra.util.Version to better support server version strings Preserve tolerant parsing for legacy Cassandra and Scylla version forms while adding canonical prerelease, build metadata, ordering, and hashing behavior. Use parsed major versions (major.minor for DSE) for schema selection so Cassandra 3.x/4.x and DSE prereleases select their matching schema parsers. (cherry picked from commit 95e2517) * Document applicable upstream 3.30.0 changes Record the version parsing, comparison, and hashing fixes without changing the Scylla driver version or replacing its release history. Adapted from upstream commit 65b26fe. * Address review findings in Version parsing and schema parser selection Treat Scylla package release suffixes (-<n>.<yyyymmdd>.<hash>) as build metadata so a release build sorts after its RCs, accept dotted prereleases after a numeric build component, keep the trailing text of a numeric component as the prerelease in the tolerant parser, and always warn with the assumed version when that parser is used. Add __slots__, label build and prerelease in repr, and document that prerelease tags compare case-sensitively as in SemVer. Read release_version from system.local when the known version is empty and include the query error in the fallback warning. * CASSPYTHON-10 Require numeric major and minor components Share the release-component grammar and extraction between ordinary and dotted prerelease forms so the parser paths cannot drift. Reject bare-major and nonnumeric-minor inputs while preserving tolerant parsing after a valid major.minor prefix. Use 0.0 as the missing schema version sentinel and cover both rejected and retained forms. * CASSPYTHON-10 Refine version and schema parser semantics Treat ambiguous -0.DATE.HASH suffixes vendor-neutrally as prereleases, preserving the complete tag and the driver's natural ordering semantics. Warn about a missing release_version only when parser selection actually falls back to V22, allowing valid DSE versions to select their release-line parser without a spurious warning. Document the intentional ordering and schema selection behavior and add focused coverage. * CASSPYTHON-10 Accept hyphens in version build metadata Share the metadata suffix grammar between ordinary and dotted prerelease forms and accept hyphens without falling back to legacy parsing.\n\nKeep metadata excluded from comparison and hashing, and cover canonical and dotted forms. * CASSPYTHON-10 Address review feedback Consolidate Version release notes, describe schema fallback in outcome terms, and clarify dotted prerelease placement. Remove an assertion already implied by strict ordering. --------- Co-authored-by: absurdfarce <bret.mcguire@ibm.com>
…1079) * cluster: prevent competing pool replacement Fence same-host pool creation, failure handling, and reconciliation with generation and intent ownership. Preserve a live competing winner, discard stale candidates, and join superseding lifecycle work without closing connections carrying in-flight requests. Serialize pool removal with publication while keeping connection setup, waits, and shutdown outside the Session lock. Add deterministic coverage for creation, removal, failure, distance, keyspace, host-identity, and shutdown races. Fixes: scylladb#317 * cluster: explain cancelled completion handling
Add micro-benchmarks measuring query plan generation throughput for DCAwareRoundRobinPolicy, RackAwareRoundRobinPolicy, TokenAwarePolicy, DefaultLoadBalancingPolicy, and HostFilterPolicy. Uses pytest-benchmark for accurate timing and statistical reporting with a simulated 45-node cluster topology (5 DCs x 3 racks x 3 nodes) and 100,000 deterministic queries. The mock cluster's token-range/replica map (vnode_cluster) and the generated query load (benchmark_queries) are independent, module-scoped fixtures sized via separate TOKEN_RANGE_COUNT / QUERY_COUNT constants, so either can be tuned without touching the other. Also rename tests/integration/standard/column_encryption/test_policies.py to test_encrypted_policies.py to avoid module name conflicts when running the full test suite. Run with: pytest -m benchmark tests/performance/ Benchmark results comparing master vs PR scylladb#651 optimizations (Python 3.14.3, pytest-benchmark 5.2.3, GC disabled, median): Policy | master (Kops/s) | PR#651 (Kops/s) | Speedup --------------------------|-----------------|-----------------|-------- DCAware | 833 | 1898 | 2.3x RackAware | 542 | 1589 | 2.9x TokenAware(DCAware) | 135 | 572 | 4.2x TokenAware(RackAware) | 123 | 539 | 4.4x Default(DCAware) | 674 | 1257 | 1.9x HostFilter(DCAware) | 394 | 579 | 1.5x
…cylladb#750) In cqltype_to_python(), double-quoted UDT names containing backslashes or single quotes were passed to ast.literal_eval without proper escaping, producing SyntaxWarning on Python 3.12+ or SyntaxError for single quotes. Escape both backslashes and single quotes before wrapping the token for ast.literal_eval. The reverse operation in python_to_cqltype() is updated to unescape both sequences as well.
…ladb#779) * tests: fix test_roundrobin decommission with dead node Scylla's Raft topology coordinator rejects decommission when there are dead nodes in the cluster. Restart node 3 before decommissioning node 1. * tests: fix test_roundrobin_two_dcs bootstrap with dead node Scylla's Raft topology coordinator rejects bootstrap when there are dead nodes in the cluster. Bootstrap node 5 before force-stopping node 1. * tests: fix test_roundrobin_two_dcs_2 bootstrap with dead node Scylla's Raft topology coordinator rejects bootstrap when there are dead nodes in the cluster. Bootstrap node 5 before force-stopping node 1. * tests: fix test_token_aware_with_rf_2 for Scylla shard-aware routing Scylla's shard-aware routing may distribute TokenAwarePolicy queries across both replicas instead of always picking the first one. Assert that the total query count across both replicas equals 12. * tests: fix test_dc_aware_roundrobin_one_remote_host decommission order Scylla's Raft topology coordinator can fail when decommissioning with only 2 voters. Reorder decommissions so node 1 is decommissioned while 3 Raft voters remain (nodes 1, 2, 5), before decommissioning node 5. * tests: add remove_cluster() to prevent stale decommissioned node state test_token_aware_composite_key and test_token_aware_with_local_table called use_singledc() without remove_cluster(), reusing the cluster left by the previous decommission-heavy test. Decommissioned nodes refused to restart with 'This node was decommissioned and will not rejoin the ring'. Adding remove_cluster() forces a fresh cluster.
…cylladb#762) test_idle_heartbeat snapshots each connection's request_ids keyed by id(connection), sleeps for a couple of heartbeat intervals, then looks the connections back up by id() to validate the heartbeats. If a connection object is replaced in between, the lookup raises KeyError. master already fixed the main source of this: wait_for_all_pools() only waits for the first connection per host, so shard-aware connections to the remaining shards could still be opening (and replacing placeholders) during the sleep window. 454f727 and f348637 added _wait_for_all_shard_connections(), called before the snapshot is taken, and deliberately dropped an earlier "skip unknown connections" band-aid so the assertions stay meaningful once the pool is known to be stable. That fix only guarantees connection *count* has stabilized, though. It doesn't rule out an existing connection being swapped for a new object in the same shard slot while the count stays constant: - HostConnection._open_connection_to_missing_shard() replaces a shard's connection once orphaned_threshold_reached, independent of pool size. - HostConnection.return_connection() -> _replace() does the same when a connection is found defunct or closed. Both can fire at any time, including during this test's own sleep, and are more likely to fire under CPU/IO contention (e.g. other tests running concurrently delaying heartbeat responses) - which lines up with the "why does this pop now, and only intermittently" puzzle from the scylladb#653 review discussion (with @dkropachev and @fruch) about this same test's flakiness. Guard against the resulting KeyError by filtering to connections that are still present in the snapshot, but without silently weakening the test: in the common case (no replacement) the filtered list is identical to the full connection list, so behavior is unchanged from master. Only when the filter actually drops something do we log a warning (so a recurrence remains visible in CI) and we still hard-fail if every single connection was replaced, since at that point nothing was validated. Compare both directions of the snapshot/current intersection, not just "every current connection is known". return_connection() has a genuine asynchronous gap: it pops a defunct/closed connection out of the pool's dict synchronously, but the replacement is inserted later by _replace(), which runs on another thread via session.submit(). A snapshot taken during that gap sees fewer current connections than were recorded, and every one of them is still "known" (nothing new has landed yet) - so "all current connections are known" is trivially true even though fewer connections than snapshotted are being validated. Tracking missing-from-current (snapshotted connections no longer present) in addition to unknown-in-current (new connections not in the snapshot) makes that partial-removal case surface as a warning too, per review feedback from @copilot-pull-request-reviewer. Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
…cylladb#799) Replace per-call struct.pack(">H%dsB" % l, l, p, 0) with pre-compiled uint16_pack(len(p)) + p + b'\x00'. This eliminates the format string interpolation and dynamic struct format creation on every call, using the pre-compiled uint16_pack (struct.Struct('>H').pack) instead. The routing key computation is called for every query when TokenAwarePolicy is in use, making this a hot path. Normalize each part to bytes before packing and build the segment with a single b''.join() instead of chained `+` concatenation. This avoids an extra intermediate bytes allocation per part, and also makes sure any buffer-protocol object (bytearray, memoryview -- including non-contiguous slices) is accepted as a routing key component, not just plain bytes. Adds unit test coverage for bytes/bytearray/ memoryview (contiguous and non-contiguous) routing key parts. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* Fix cross-lock race in libev reactor thread exit check (free-threaded 3.14t hang) LibevLoop._live_conns is written under a lock in connection_created()/connection_destroyed(), while _run_loop()'s exit check decides whether to stop the reactor thread. An earlier version of this fix moved the _live_conns read onto the same lock the writers used, but the started/shutdown flags were still read and set under a separate lock afterward -- leaving a gap where connection_created() could still register a connection between the read and the state transition. CI on that version reproduced the exact hang this fix is meant to eliminate. Fix: merge _lock and the former _conn_set_lock into a single lock that guards both _live_conns/_new_conns/_closed_conns *and* the _started/_shutdown transitions read in the exit check. This makes the exit decision and connection registration mutually exclusive rather than just reading from a shared lock: a concurrent connection_created() either finishes before the exit check's critical section (its connection is visible in _live_conns, so the reactor keeps running) or finishes after _started is set to False inside that same critical section (so the subsequent maybe_start() call, which always follows connection_created(), sees _started == False and starts a fresh thread). There is no interleaving in which the new connection is invisible to both checks, closing the race rather than narrowing it. The two locks didn't need to stay separate: _run_loop() already nested "with self._lock: with self._conn_set_lock:", and connection_created()/connection_destroyed()/_loop_will_run() never call anything that reacquires _lock, so merging them introduces no reentrancy or ordering issue. Also add a regression test (LibevLoopRaceTest) that forces the exact interleaving from issue scylladb#980 deterministically: it pauses the reactor thread's exit-check via an instrumented lock right after it starts deciding, then tries to register a connection from another thread. The test asserts connection_created() cannot complete until the reactor's decision is committed, and that maybe_start() correctly restarts the reactor if the connection lands just after. Verified this test fails (reliably, not flakily) against the git history's prior attempt at this fix and passes against this one. Fixes scylladb#980. Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com> * libevreactor: TODO remaining unguarded _shutdown/_live_conns accesses _cleanup() and _loop_will_run() still read/write these fields without _lock. Verified: copy-on-write set semantics rule out torn reads, and both sites self-heal (notify()+retry, or next loop tick) rather than risking the permanent hang this PR fixes for _run_loop's exit check. Lower severity, but flagging for a follow-up. No open PR covers these. Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com> --------- Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com>
…cylladb#1021) * Increase MV schema-notification wait budget to fix flaky metadata tests Materialized-view schema-change notifications arrive as a separate control-connection event from the base-table schema response, so there is an inherent lag between a base-table ALTER completing and the MV's own metadata reflecting it. The existing workaround in test_base_table_column_addition_mv and test_base_table_type_alter_mv capped this wait at 10 x 0.2s = 2s, which is too tight under CI load and flakes intermittently. Replace both hand-rolled retry loops with the existing tests.util.wait_until helper, raising the budget to 60 x 0.5s = 30s. This reuses the codebase's standard wait-for-condition primitive instead of duplicating another ad hoc loop, and gives the control connection realistic headroom to deliver the MV notification. Also drops the now-unused 'import time' left behind by removing the last time.sleep() calls in this file. Fixes: scylladb#1020 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * tests: fix wait_until off-by-one that timed out on a successful final poll wait_until() checked condition() first, then looked at the *attempt* counter to decide whether to raise, instead of the condition's own last result. When the condition finally became true exactly on the poll after the last sleep (attempt == max_attempts), the loop correctly exited, but the post-loop 'if attempt >= max_attempts: raise' fired anyway, reporting a false timeout. This was latent before but surfaced by PR scylladb#1021 raising the metadata tests' max_attempts from 10 to 60: a wider window makes hitting that exact boundary far more likely under real CI load. Cache each condition() call's own result and check that instead of the attempt counter. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…rk (scylladb#1097) * cluster: don't supersede in-flight add/up pool creation for a down host Since "cluster: prevent competing pool replacement (scylladb#317)" (scylladb#1079), update_created_pools() claims a new pool intent for every host. For a host that is still down, it then settles the shared intent completion False and drops its generation. When that host has an on_add()/on_up() in flight (e.g. a reconnector-triggered add racing another host's _finalize_add()), the add sees False, logs "Connection pool could not be created, not marking node up", discards its freshly built pool, and no reconnector is re-armed, so the host stays down forever. Skip down hosts whose add/up handling is in flight; that handling owns pool creation and reconciles again when it finishes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * cluster: notify wait() when cancelling pool completion proxies Since "cluster: prevent competing pool replacement (scylladb#317)" (scylladb#1079), the futures returned for pool creation are bare completion proxies rather than executor futures. Future.cancel() on a bare Future leaves it CANCELLED without notifying concurrent.futures.wait() waiters (only set_running_or_notify_cancel() does, which executors call for their own futures), and wait() never counts such a future as done. Session.shutdown() cancels still-pending initial-connect proxies (e.g. superseded by reconciliation, or whose work finished before its done callback settled them) and then waits on them with no timeout, so it blocks forever. The same applies to proxies cancelled by _PoolIntentCompletion when work is cancelled. Notify after a successful cancel so waiters wake up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…le control connection (scylladb#1094) * tests: wait for add_callbacks in test_concurrent deferred futures execute_async returns the fake future before the driver attaches callbacks, so succeed()/fail() could hit a None _callback. Make wait_for_response_futures also wait until every future is attached. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * tests: wait for a live control connection before reloading client routes After a node is decommissioned the cached control_connection._connection can be a closed NLB-proxied socket until the reconnect finishes, so handler.initialize raised ConnectionShutdown. Wait for a usable connection and re-read it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* ci: speed up GitHub Actions workflows - Add concurrency with cancel-in-progress to PR-triggered workflows (integration-tests, coverage, docs-pr, build-test). - Drop the JDK setup from integration-tests and coverage: Java-UDF tests are skipped when SCYLLA_VERSION is set and these jobs do not run simulacron. - Remove the ccm create/remove "Download Scylla" warm-up in both test workflows; the real run populates the cache. - Fix the broken paths-ignore lists (the last entry was folded into the previous scalar) and use **/*.md, **/*.rst, docs/** so docs-only changes stop triggering the heavy jobs. - lib-build: cache ~/.conan2 on Windows, collapse the double checkout into one, and add timeout-minutes. - Add timeout-minutes across jobs and explicit enable-cache on setup-uv; drop the needless fetch-depth: 0 in docs-pr. * fix: address review findings on CI speedups - Cache the real Scylla download dir (~/.ccm/scylla-repository, not ~/.ccm/repository) and add restore-keys so the cache actually works. - Honor target_tag in build-sdist so a tagged publish does not ship an sdist built from the default branch. - Raise build-wheels timeout-minutes to 120 for the aarch64/windows legs. - Make build-test cancel-in-progress consistent with the other workflows. - Add restore-keys to the Conan cache. * fix: force conan profile re-detect when the conan cache is restored conan profile detect fails if a default profile already exists, which happens whenever the new ~/.conan2 cache is restored. Add --force. * ci: use a per-wheel import smoke test on PR build-test The PR 'Test wheels building' run executed the full unit suite for every one of the ~7 built ABIs per platform (~5-11 min of the job) and installed the dev extra in the test env, which is what fails on Windows/PyPy (no cryptography wheel). Add a smoke_tests input to the reusable lib-build.yml that overrides cibuildwheel's per-platform test-command with a one-line 'import cassandra...' check and clears test-groups/test-extras. build-test.yml now passes smoke_tests: true; release/publish callers keep the full suite. Wheels are still built for every ABI and uploaded. * ci: build wheels with the runner's full CPU count The C/Cython extension build was pinned to CASS_DRIVER_BUILD_CONCURRENCY=2 while the runners expose 3 (macos-14) or 4 (ubuntu-24.04) vCPUs, leaving cores idle during the compile block that dominates a PR wheel job (~6m15s of 8m43s on macos-arm). Compute the core count at runtime (nproc, then getconf, then %NUMBER_OF_PROCESSORS%) in lib-build.yml and export it; setup.py already reads the variable. Drop the hard-coded value from the global and Windows cibuildwheel environments, and pass the host value into the Linux build container via environment-pass. * ci: guard wheel-build concurrency against an empty CPU count If 'nproc' and 'getconf _NPROCESSORS_ONLN' both succeed but print nothing (getconf can exit 0 with empty output on some platforms), 'cpus' would be empty and setup.py's int(os.environ['CASS_DRIVER_BUILD_CONCURRENCY']) would raise ValueError at build time. Default to 2 when the result is empty.
Executor-backed DOWN processing can outlive host removal and notify callbacks or publish a reconnector after removal completes. Fence queued and restarted work with the Host terminal state. Order DOWN callbacks with removal, stop policy fan-out after reentrant removal, and reject UP handoff for a removed object. Document that custom callbacks must not wait on cluster executor work while removal waits for them. Fixes scylladb#1034
…cylladb#1019) * tests: retry client-routes POST on transient 5xx after decommission A node's REST API can briefly return HTTP 500 for /v2/client-routes right after a decommission/bootstrap while gossip/topology settles. post_client_routes() had no retry, so test_should_survive_full_node_replacement_through_nlb flaked in CI. Retry up to 5 attempts with a 1s backoff, but only on 5xx responses - 4xx or other errors still raise immediately. Also log the response body on failure, which was previously discarded and made past failures hard to diagnose. Fixes scylladb#931 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com> * tests: tighten client-routes retry unit coverage Address review feedback on the retry tests: - scope the CASSANDRA_VERSION fallback to the import so it does not leak into later tests - assert the unreadable-body fallback is logged - assert the REST timeout is passed on every attempt - assert terminal HTTP errors are closed on exhaustion and immediate failure * tests: make client-routes REST helper cluster-free * tests: bound error-body read, scope retries, cap escaped log text Addresses review feedback on the client-routes 5xx retry: - Bound the error-body read by elapsed time (ERROR_BODY_DEADLINE), not just bytes: a trickling response could otherwise stall the retry loop past the socket timeout indefinitely. - Scope the retry to the post-decommission path via a retries parameter (default 1 = no retry). Only that path has evidence of a transient 5xx; other route-post assertions now surface server failures immediately. - Apply the log cap after escaping, so expanded control characters can no longer exceed the documented per-body character limit. --------- Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com> Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
test_the_same_session_again_keeps_the_deadline_it_had stored a 50 ms deadline, slept 60 ms and expected the entry gone. On Windows before Python 3.13, time.monotonic() advances in 15.625 ms ticks, so 60 ms of sleep can read as 46.9 ms and the entry is still returned. Store the first session already expired instead, as the neighbouring tests do. Re-storing the same session must keep that deadline, so the lookup still has to miss; no clock is involved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cache the results of _CassandraType.apply_parameters() in a class-level dict keyed by (cls, subtypes, names). This avoids the expensive type() metaclass machinery on repeated calls with the same type signature, which is the common case during result-set deserialization. Benchmark: 31.7x speedup (6.48 us/call -> 0.20 us/call) for cached hits. UserType and VectorType define their own apply_parameters() overrides (with their own, independently-keyed caching/lookup) and never read or write this cache, so this change cannot affect UDT or vector<...> dimension/subtype resolution. Fix a crash the cache key introduced for types that accept zero subtypes (num_subtypes == 'UNKNOWN', e.g. CompositeType, ColumnToCollectionType): `tuple(names) if names else names` left an explicit empty `names` list unconverted (`[]` is falsy), producing an unhashable list inside the cache-key tuple and raising TypeError on the dict lookup. Normalize with `is not None` instead, and add regression tests for the empty-names case and for confirming VectorType stays independent of the shared cache. Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com>
mykaul
force-pushed
the
perf/cache-apply-parameters
branch
from
October 6, 2026 14:49
e5f3e3b to
c2f9b10
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
apply_parameters()results so parameterized CQL type classes (e.g.ListType(UTF8Type),MapType(UTF8Type, Int32Type)) are created viatype()once and returned as stable singletons on subsequent calls(cls, tuple(subtypes), names)— only the base_CassandraType.apply_parametersis cached;UserTypeandVectorTypeoverrides remain uncachedid()-based lookups now see the same type objects across result setsBenchmark results (median, pytest-benchmark)
Single apply_parameters call
Batch apply (simulating read_type for result metadata)
All 116 unit tests pass (1 skipped — pre-existing test_datetype issue).