(improvement) cqltypes: cache apply_parameters for parameterized types - #10
Draft
mykaul wants to merge 111 commits into
Draft
(improvement) cqltypes: cache apply_parameters for parameterized types#10mykaul wants to merge 111 commits into
mykaul wants to merge 111 commits into
Conversation
mykaulforce-pushed
the
perf/cache-apply-parameters
branch
from
March 13, 2026 12:03
6057c76 to
6556867Comparemykaul
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>
When connection is closed by the server, but there is no other error, it will be close (is_cloes == True) without setting `last_error`. This is true for all reactors apart from Twisted as far as I can tell. If we try to use such connection, we'll quickly discover that its broken, but we can slightly optimize this process by raising directly from factory().
The *i686 skip pattern in cibuildwheel config matches only Linux 32-bit identifiers (e.g. cp310-manylinux_i686), not Windows 32-bit (which uses win32, e.g. cp310-win32). Add *win32 to skip 32-bit Windows builds, which fail because: - c_shard_info.c uses __uint128_t (GCC extension, unsupported by MSVC) - cryptography test dependency fails to build for i686-pc-windows-msvc due to missing OpenSSL
The docstring and test promised ValueError for invalid scope values (e.g. 'planet'), but the validation was never implemented in the method body. Add an explicit check against the three SchemaAgreementScope members. Introduced in commit 0d215f4 (cluster: add Session.wait_for_schema_agreement).
Resolves Dependabot alert scylladb#67 (GHSA-w2fm-2cpv-w7v5 / CVE-2026-22815): aiohttp <= 3.13.3 allows unlimited trailer headers, leading to possible uncapped memory usage (CWE-400/CWE-770). Fixed in aiohttp 3.13.4. aiohttp is a transitive runtime dependency pulled in only by the docs toolchain via gremlinpython==3.7.4. Bumped to 3.14.1 (>= 3.13.4 patched).
Statements with SERIAL or LOCAL_SERIAL consistency level are serialized through the Paxos path on the server, but TokenAwarePolicy only checked is_lwt() (from server prepare metadata) when deciding whether to skip replica shuffling. This meant serial-consistency reads could be routed with shuffled replicas instead of the deterministic order needed for optimal Paxos coordination. Now TokenAwarePolicy also checks the statement's consistency level and skips shuffling for SERIAL/LOCAL_SERIAL, matching LWT routing behavior. Fixes: scylladb#886
Add a guard in the retry execution path that prevents any retry policy from downgrading SERIAL/LOCAL_SERIAL to a non-serial consistency level, which would break serial read (Paxos) guarantees. Also add a unit test verifying DowngradingConsistencyRetryPolicy does not downgrade serial consistency on read timeout or unavailable. Fixes: https://scylladb.atlassian.net/browse/DRIVER-613
test_watchers_are_finished calls libev__cleanup(), which stops the shared libev loop preparer. If a timer test runs later, the stopped preparer means timers are never scheduled and the test can hang. Restore _global_loop._shutdown and restart _global_loop._preparer after the cleanup path runs. Put the restoration in a finally block so the shared loop is left usable even if the post-cleanup watcher assertions fail.
Python 3.12 removed distutils from the standard library. Some Cython test helpers import pyximport, which still imports distutils through setuptools' compatibility shim during collection. Add setuptools to the dev test dependencies so those tests can collect in cibuildwheel's test environment.
Tests skip themselves when their requirements are missing (a library is absent, the wrong event loop is selected, the C extensions aren't built). That is convenient locally but a footgun in CI, where a test may be silently skipped because a dependency was not installed. The libev unit tests were effectively not running in any CI configuration. Add a CASS_DRIVER_NO_SKIP-gated pytest hook in tests/conftest.py that turns skips into failures (xfail untouched), and enable it in the cibuildwheel test-commands where the C extensions are mandatory (Linux/macOS). Tests that genuinely cannot run in the default configuration are listed explicitly via -k/--ignore (reactor tests run separately per EVENT_LOOP_MANAGER; asyncore, column_encryption and a few upstream-disabled/flaky tests excluded). Add -v to every pytest invocation and install the compress-lz4 extra so the lz4 tests actually run. Pass --import-mode=append in the cibuildwheel pytest commands so the installed compiled wheel takes precedence on sys.path over the in-tree pure-Python cassandra source during wheel tests. Keep the global pytest addopts unchanged so local pytest runs keep their normal import behavior. Windows and PyPy keep no-skip off: the extensions are optional on Windows and are never built on PyPy (setup.py forces is_pypy to skip libev/cmurmur3/Cython), so their extension-dependent skips are legitimate. The PyPy override also drops the compress-lz4 extra (no prebuilt PyPy lz4 wheel), uses cross-shell quoting, and deselects the known PyPy/Windows/macOS-incompatible tests.
The final callback was invoked with host_errors (the errors from only the last pool to finish) instead of the accumulated errors dict. If the last pool succeeded, failures from other pools were silently lost. Pass the aggregated errors dict, matching the method's docstring.
The atexit subprocess test inserted the project root at sys.path[0], which shadows the installed compiled wheel with the in-tree pure-Python source. Under cibuildwheel that source lacks the libev C extension, so the import failed and the subprocess produced no output. Append the project path instead so the installed wheel takes precedence, falling back to the source tree only when the driver is not installed. Also assert the subprocess return code and include stdout/stderr in the failure message so future subprocess import/runtime failures are easier to diagnose.
Bumps [soupsieve](https://github.com/facelessuser/soupsieve) from 2.8.3 to 2.8.4. - [Release notes](https://github.com/facelessuser/soupsieve/releases) - [Commits](facelessuser/soupsieve@2.8.3...2.8.4) --- updated-dependencies: - dependency-name: soupsieve dependency-version: 2.8.4 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Make ProtocolFeatures.__init__ keyword-only and build it by keyword in parse_from_supported. Independently developed protocol extensions (SCYLLA_USE_METADATA_ID, TABLETS_ROUTING_V2) each add fields to this class; keyword construction lets them do so without conflicting over positional-argument order. All existing callers already used keywords.
Connection.send_msg now passes the connection's negotiated ProtocolFeatures to the encoder; _ProtocolHandler.encode_message accepts it as a new required protocol_features argument (passed by keyword from send_msg) and forwards it to every message's send_body, which gains the same parameter. Messages carry connection-independent request data; send_body decides the wire format from (protocol_version, protocol_features), so fields belonging to a negotiated protocol extension are emitted exactly on the connections that negotiated it — on every send path, including the control-connection fallback, and without mutating shared message objects per attempt. This is pure plumbing: no message consumes the parameter yet, so no bytes on the wire change. It is groundwork for the SCYLLA_USE_METADATA_ID (scylladb#770) and TABLETS_ROUTING_V2 (scylladb#913) extensions, which must serialize extension fields based on what the serving connection negotiated. The encode side becomes symmetric with decode_message, which already receives protocol_features. This changes the contracted signature of encode_message: custom protocol handlers overriding it must accept the protocol_features keyword argument. The argument is deliberately required, with no default and no fallback for old-style encoders: extensions are negotiated per connection at STARTUP before the per-request handler is known, so an encoder unaware of protocol_features could silently omit fields a negotiated extension requires; omitting it fails fast with TypeError instead. Tests: send_msg hands the connection's features to the encoder; encode_message forwards them into send_body (plain and compressed paths) and raises TypeError when the argument is omitted; a byte-identity suite pins frames for representative messages (v3/v4/v5) to the exact bytes produced before this change, both without features and with all-default features. Co-authored-by: Dawid Mędrek <dawid.medrek@scylladb.com>
Note in the protocol API docs that both contracted _ProtocolHandler methods receive the connection's negotiated ProtocolFeatures, spelling out the calling conventions: decode_message receives it positionally, encode_message as the required protocol_features keyword argument, so overrides must keep that parameter name. Add a CHANGELOG entry with upgrade guidance for custom protocol handlers (accept protocol_features, prefer **kwargs for future-proofing, forward it when delegating to send_body).
Problem: shard-aware endpoint selection treated legacy ssl_options as SSL-enabled only when the dict was truthy. An explicit empty ssl_options={} was therefore handled like plaintext and could select the non-SSL shard-aware port, diverging from the cluster-level SSL-enabled check.
Fix: treat SSL as enabled when ssl_context is set or ssl_options is not None. SSL-enabled configurations now use the SSL shard-aware port when advertised and otherwise fall back to regular non-shard-aware connections instead of the plaintext shard-aware port. Unit coverage now includes ssl_context, non-empty ssl_options, and empty ssl_options.Implement the SCYLLA_USE_METADATA_ID protocol extension, which backports the CQL v5 prepared-statement metadata-id mechanism to earlier protocol versions. When negotiated, the server includes a hash of the result metadata in the PREPARE response; the driver sends it back with every EXECUTE, allowing the server to omit result metadata from responses (skip_meta) and to report schema changes with METADATA_CHANGED plus fresh metadata, which the driver adopts automatically. protocol_features.py: parse the extension from SUPPORTED, echo it in STARTUP, expose it as ProtocolFeatures.use_metadata_id. protocol.py: ExecuteMessage carries connection-independent request data (skip_meta, result_metadata_id) fixed at construction; serialization decides the wire format from the (protocol_version, protocol_features) that Connection.send_msg supplies for the serving connection: - The metadata-id field is written iff the connection speaks CQL v5+ or negotiated the extension - always, on such connections. An empty sentinel (b'') is written when the statement has no id (prepared before the extension was active, e.g. during a rolling upgrade, or an LWT statement): the sentinel mismatch makes the server respond with METADATA_CHANGED plus the current id and metadata, so such statements acquire an id on their first execution. This also fixes a TypeError on v5 when result_metadata_id was None. - _SKIP_METADATA_FLAG is written only when the SCYLLA_USE_METADATA_ID extension is negotiated on the connection; without the metadata-id mechanism a schema change after PREPARE would leave the driver decoding rows with stale cached metadata. This is deliberately narrower than the metadata-id field above: on native CQL v5 the field is part of the frame layout, but the driver does not request skip there. Upstream never emitted _SKIP_METADATA_FLAG on any version (_write_query_params never wrote it), and enabling the skip optimization for native v5 is a separate change kept out of scope for this Scylla extension. Because messages are immutable after construction, every send path is correct without per-path setup - including the control-connection fallback - and concurrent sends of the same message (speculative executions) cannot race on per-connection state. query.py: PreparedStatement stores (result_metadata, result_metadata_id) as one tuple replaced in a single attribute assignment, read through compatibility properties and updated via update_result_metadata(). Response callbacks update statements while request threads read them; a torn pair (fresh id + stale metadata) would make the server skip sending metadata while rows are decoded against the wrong columns, with no recovery. The compatibility setters are documented as non-atomic relative to each other - update_result_metadata() is the atomic path; the setters exist only for callers assigning the old individual attributes. cluster.py: _create_response_future snapshots the pair once and requests skip_meta only when the statement has both an id and usable cached metadata (result_metadata is None for NO_METADATA/LWT statements and [] for zero-column statements; neither can nor needs to skip metadata). The same snapshot is handed to the ResponseFuture, so a skip_meta response is decoded against the metadata that pairs with the id the message sent - not a later re-read of the statement cache, which a concurrent METADATA_CHANGED could have replaced between construction and send (and which also keeps speculative sends of one message internally consistent). _set_result adopts a METADATA_CHANGED response by replacing the pair atomically; a response carrying a new id without column metadata is ignored with a warning, since adopting the id alone would create the unrecoverable stale-decode state. skip_meta additionally stays off for continuous paging (@dkropachev): Connection.process_msg hardcodes result_metadata=None for every page after the first, since it isn't threaded through the paging session - a skip_meta response has nothing to decode page 2+ against, and would crash on it. _execute_after_prepare refreshes the pair from exactly what the reprepare response carries, including the id (@dkropachev): falling back to the previously cached id when the response has none risks pairing it with metadata from a different schema version than the one that id was computed for - e.g. if the schema changed and then reverted between the two PREPAREs, the old id can become valid again for the current schema while paired locally with an intermediate version's metadata, with no server-side mismatch to catch it. Dropping it instead lets the next id-aware execute re-acquire a correctly paired id through the same b'' sentinel self-healing path a never-prepared statement uses. docs/scylla-specific.rst: documents the extension and its behaviour, worded so the skip_meta optimization reads as conditional on the extension being negotiated rather than pre-existing default behaviour. CHANGELOG.rst: add a Features entry for the extension.
Unit tests for the extension across its layers: test_protocol_features.py: SCYLLA_USE_METADATA_ID parsed from SUPPORTED and echoed in STARTUP options; absent by default. test_protocol.py (wire format): - metadata-id field written on v4 iff the connection negotiated the extension, with the exact bytes asserted; empty sentinel (b'') when the statement has no id, on both the extension path (v4) and the v5 native path (previously a TypeError); - _SKIP_METADATA_FLAG written when skip_meta is requested and the SCYLLA_USE_METADATA_ID extension is negotiated (v4 or v5), and NOT set on a native v5 connection without the extension (the id field is still written there, but the driver does not request skip); also suppressed - together with the id field - on a v4 connection without the extension, even when the statement carries an id; - PREPARED response decoding reads result_metadata_id iff the extension was negotiated (or v5); METADATA_CHANGED/NO_METADATA flag handling. test_query.py: PreparedStatement stores the (result_metadata, result_metadata_id) pair atomically - constructor, update_result_metadata, and the backwards-compatible single-attribute setters all replace the pair as one unit, and previously-taken snapshots stay internally consistent. test_response_future.py: - _create_response_future builds ExecuteMessage from a single pair snapshot: skip_meta only with both an id and usable cached metadata; disabled for id-less statements, NO_METADATA/LWT statements (result_metadata None) and zero-column statements (result_metadata []), while the id still rides on the message; - _query sends the message exactly as constructed (no per-connection mutation - regression test for the speculative-execution race) and decodes a skip_meta response against the metadata snapshotted when the message was built, not a later read of the statement cache (regression for a concurrent METADATA_CHANGED racing the send); - _set_result METADATA_CHANGED path replaces the cached pair atomically; a response with a new id but no column metadata (empty or absent) is ignored with a warning, leaving the cached pair unchanged - adopting the id alone would poison the cache with a stale-metadata/current-id pair the server would never refresh; - _execute_after_prepare refreshes the pair from exactly what the reprepare response carries, including the id, and no longer keeps the previous id when the response has none (@dkropachev: doing so risked pairing a stale id with metadata from a different schema version - test_execute_after_prepare_no_metadata_id_in_response_clears_id); - a statement with valid cached metadata+id must still get skip_meta=False when continuous_paging_options is set (@dkropachev: Connection.process_msg hardcodes result_metadata=None for paging-session pages after the first, so a skip_meta response would crash decoding them - test_create_execute_message_continuous_paging_disables_skip_meta). tests/integration/standard/test_scylla_metadata_id.py: live-server coverage against a real Scylla node via CCM, closing the one gap unit tests can't - whether Scylla actually treats the empty result_metadata_id sentinel as a mismatch rather than a protocol error. Confirms extension negotiation, the normal METADATA_CHANGED-after-ALTER-TABLE path, and the sentinel round trip: a statement forced back to result_metadata_id=None (simulating one prepared before the extension was known, e.g. mid rolling-upgrade) executes without error and comes back with a fresh id. Mirrors the equivalent live test already merged in the Java driver (scylladb/java-driver#758, should_handle_empty_metadata_id_when_executing_statement_when_supported). Run locally against Scylla 2026.1.9 via CCM; see PR description for setup and log excerpt.
mykaulforce-pushed
the
perf/cache-apply-parameters
branch
from
July 29, 2026 20:38
5d25b9b to
9355678CompareUpdates docs theme to 1.9.3.
mykaulforce-pushed
the
perf/cache-apply-parameters
branch
from
July 30, 2026 21:19
9355678 to
6c96f8bCompareCache 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>
mykaulforce-pushed
the
perf/cache-apply-parameters
branch
from
August 15, 2026 07:44
6c96f8b to
e5f3e3bCompare
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 freeto 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).