From 1c9c4f4c2479971da8ecc998bec1cab9cb686c35 Mon Sep 17 00:00:00 2001 From: Yaniv Michael Kaul Date: Mon, 29 Jun 2026 23:21:18 +0300 Subject: [PATCH 1/3] fix: detect ScyllaDB via SUPPORTED protocol extensions, not shard count The driver previously identified ScyllaDB solely by the presence of shard-related fields (SCYLLA_NR_SHARDS, SCYLLA_SHARD, etc.) in the SUPPORTED response. When shard-awareness is disabled on the server side (allow_shard_aware_drivers: false), those fields are absent, causing the driver to misidentify a ScyllaDB cluster as Cassandra. Changes: - Add ProtocolFeatures.detect_scylla() and is_scylla flag, set from known Scylla-specific extension keys (LWT, rate-limit, tablets routing) or sharding_info presence - Replace sharding_info check with is_scylla for peers_v2 and metadata_request_timeout in cluster.py and metadata.py - Guard int() calls against None in _ShardingInfo construction (c_shard_info.pyx, shard_info.py) - Add tests for is_scylla detection and regression tests for missing shard fields - Assert pf.shard_id normalization in test regressions (response to CodeRabbit review) --- cassandra/c_shard_info.pyx | 4 +- cassandra/cluster.py | 11 ++-- cassandra/metadata.py | 10 +++- cassandra/protocol_features.py | 36 ++++++++++- cassandra/shard_info.py | 4 +- tests/unit/test_protocol_features.py | 90 ++++++++++++++++++++++++++++ 6 files changed, 141 insertions(+), 14 deletions(-) diff --git a/cassandra/c_shard_info.pyx b/cassandra/c_shard_info.pyx index a8affd9bba..1564934185 100644 --- a/cassandra/c_shard_info.pyx +++ b/cassandra/c_shard_info.pyx @@ -29,10 +29,10 @@ cdef class ShardingInfo(): def __init__(self, shard_id, shards_count, partitioner, sharding_algorithm, sharding_ignore_msb, shard_aware_port, shard_aware_port_ssl): - self.shards_count = int(shards_count) + self.shards_count = int(shards_count) if shards_count else 0 self.partitioner = partitioner self.sharding_algorithm = sharding_algorithm - self.sharding_ignore_msb = int(sharding_ignore_msb) + self.sharding_ignore_msb = int(sharding_ignore_msb) if sharding_ignore_msb else 0 self.shard_aware_port = int(shard_aware_port) if shard_aware_port else 0 self.shard_aware_port_ssl = int(shard_aware_port_ssl) if shard_aware_port_ssl else 0 diff --git a/cassandra/cluster.py b/cassandra/cluster.py index 12ade2018f..83fd55ef07 100644 --- a/cassandra/cluster.py +++ b/cassandra/cluster.py @@ -3892,14 +3892,13 @@ def _try_connect(self, endpoint): "registering watchers and refreshing schema and topology", connection) - # Indirect way to determine if conencted to a ScyllaDB cluster, which does not support peers_v2 - # If sharding information is available, it's a ScyllaDB cluster, so do not use peers_v2 table. - if connection.features.sharding_info is not None: + # ScyllaDB does not support peers_v2. Use is_scylla (not sharding_info) + # so that clusters with shard-awareness disabled are still detected correctly. + if connection.features.is_scylla: self._uses_peers_v2 = False - # Only ScyllaDB supports "USING TIMEOUT" - # Sharding information signals it is ScyllaDB - self._metadata_request_timeout = None if connection.features.sharding_info is None or not self._cluster.metadata_request_timeout \ + # Only ScyllaDB supports "USING TIMEOUT". Use is_scylla for the same reason. + self._metadata_request_timeout = None if not connection.features.is_scylla or not self._cluster.metadata_request_timeout \ else datetime.timedelta(seconds=self._cluster.metadata_request_timeout) self._tablets_routing_v1 = connection.features.tablets_routing_v1 diff --git a/cassandra/metadata.py b/cassandra/metadata.py index 43399b7152..1cfaabd496 100644 --- a/cassandra/metadata.py +++ b/cassandra/metadata.py @@ -2578,8 +2578,14 @@ class SchemaParserV3(SchemaParserV22): _SELECT_VIEWS = "SELECT * FROM system_schema.views" def _is_not_scylla(self): - """Check if NOT connected to ScyllaDB by checking for shard awareness.""" - return getattr(getattr(self.connection, 'features', None), 'shard_id', None) is None + """Check if NOT connected to ScyllaDB. + + Uses the is_scylla flag from ProtocolFeatures, which is set from the + presence of Scylla-specific extension keys in the SUPPORTED response + (e.g. SCYLLA_LWT_ADD_METADATA_MARK, SCYLLA_RATE_LIMIT_ERROR) and + therefore remains True even when shard-awareness is disabled. + """ + return not getattr(getattr(self.connection, 'features', None), 'is_scylla', False) _table_name_col = 'table_name' diff --git a/cassandra/protocol_features.py b/cassandra/protocol_features.py index 1bad379208..c410e39d46 100644 --- a/cassandra/protocol_features.py +++ b/cassandra/protocol_features.py @@ -17,15 +17,17 @@ class ProtocolFeatures(object): sharding_info = None tablets_routing_v1 = False lwt_info = None + is_scylla = False # Keyword-only so that independently developed protocol extensions can add # new fields without conflicting over positional-argument order. - def __init__(self, *, rate_limit_error=None, shard_id=0, sharding_info=None, tablets_routing_v1=False, lwt_info=None): + def __init__(self, *, rate_limit_error=None, shard_id=0, sharding_info=None, tablets_routing_v1=False, lwt_info=None, is_scylla=False): self.rate_limit_error = rate_limit_error self.shard_id = shard_id self.sharding_info = sharding_info self.tablets_routing_v1 = tablets_routing_v1 self.lwt_info = lwt_info + self.is_scylla = is_scylla @staticmethod def parse_from_supported(supported): @@ -33,8 +35,28 @@ def parse_from_supported(supported): shard_id, sharding_info = ProtocolFeatures.parse_sharding_info(supported) tablets_routing_v1 = ProtocolFeatures.parse_tablets_info(supported) lwt_info = ProtocolFeatures.parse_lwt_info(supported) + is_scylla = ProtocolFeatures.detect_scylla(supported, sharding_info) return ProtocolFeatures(rate_limit_error=rate_limit_error, shard_id=shard_id, sharding_info=sharding_info, - tablets_routing_v1=tablets_routing_v1, lwt_info=lwt_info) + tablets_routing_v1=tablets_routing_v1, lwt_info=lwt_info, is_scylla=is_scylla) + + @staticmethod + def detect_scylla(supported, sharding_info): + """Detect ScyllaDB from SUPPORTED extensions, independent of shard awareness. + + ScyllaDB is identified by the presence of any known Scylla-specific + extension key in the SUPPORTED response. Checking only shard-related + fields (SCYLLA_NR_SHARDS, etc.) is insufficient because those are + absent when shard-awareness is disabled on the server side + (allow_shard_aware_drivers: false), which would cause the driver to + misidentify a ScyllaDB cluster as Cassandra and, for example, try + to query the peers_v2 table that ScyllaDB does not support. + """ + return ( + LWT_ADD_METADATA_MARK in supported + or RATE_LIMIT_ERROR_EXTENSION in supported + or TABLETS_ROUTING_V1 in supported + or sharding_info is not None + ) @staticmethod def maybe_parse_rate_limit_error(supported): @@ -76,6 +98,16 @@ def parse_sharding_info(options): sharding_algorithm == "biased-token-round-robin" or sharding_ignore_msb): return 0, None + if shard_id is None or shards_count is None: + # ScyllaDB gates all sharding fields together behind a single + # allow_shard_aware_drivers server-side config, so this should + # not happen in practice. If it does, don't fabricate a partial + # ShardingInfo (that would break the invariant, relied on + # elsewhere, that a non-None sharding_info implies a real, + # positive shard count) -- just disable shard-aware routing. + log.warning("Incomplete Scylla sharding info in SUPPORTED (options=%s); disabling shard-aware routing", options) + return 0, None + return int(shard_id), _ShardingInfo(shard_id, shards_count, partitioner, sharding_algorithm, sharding_ignore_msb, shard_aware_port, shard_aware_port_ssl) diff --git a/cassandra/shard_info.py b/cassandra/shard_info.py index 8f62252193..b671281021 100644 --- a/cassandra/shard_info.py +++ b/cassandra/shard_info.py @@ -21,10 +21,10 @@ class _ShardingInfo(object): def __init__(self, shard_id, shards_count, partitioner, sharding_algorithm, sharding_ignore_msb, shard_aware_port, shard_aware_port_ssl): - self.shards_count = int(shards_count) + self.shards_count = int(shards_count) if shards_count else 0 self.partitioner = partitioner self.sharding_algorithm = sharding_algorithm - self.sharding_ignore_msb = int(sharding_ignore_msb) + self.sharding_ignore_msb = int(sharding_ignore_msb) if sharding_ignore_msb else 0 self.shard_aware_port = int(shard_aware_port) if shard_aware_port else None self.shard_aware_port_ssl = int(shard_aware_port_ssl) if shard_aware_port_ssl else None diff --git a/tests/unit/test_protocol_features.py b/tests/unit/test_protocol_features.py index 895c384f7e..71be4c3dae 100644 --- a/tests/unit/test_protocol_features.py +++ b/tests/unit/test_protocol_features.py @@ -22,3 +22,93 @@ class OptionsHolder(object): assert protocol_features.rate_limit_error == 123 assert protocol_features.shard_id == 0 assert protocol_features.sharding_info is None + + # ----------------------------------------------------------------- + # Tests for is_scylla detection (independent of shard awareness) + # Regression for: ScyllaDB misidentified as Cassandra when sharding + # is disabled (allow_shard_aware_drivers: false). + # ----------------------------------------------------------------- + + def test_is_scylla_detected_via_lwt(self): + """ScyllaDB is recognised from SCYLLA_LWT_ADD_METADATA_MARK alone.""" + pf = ProtocolFeatures.parse_from_supported({ + 'SCYLLA_LWT_ADD_METADATA_MARK': ['LWT_OPTIMIZATION_META_BIT_MASK=8'], + }) + assert pf.is_scylla is True + assert pf.shard_id == 0 + assert pf.sharding_info is None # no shard-aware connections expected + + def test_is_scylla_detected_via_rate_limit(self): + """ScyllaDB is recognised from SCYLLA_RATE_LIMIT_ERROR alone.""" + pf = ProtocolFeatures.parse_from_supported({ + 'SCYLLA_RATE_LIMIT_ERROR': ['ERROR_CODE=42'], + }) + assert pf.is_scylla is True + assert pf.shard_id == 0 + assert pf.sharding_info is None + + def test_is_scylla_detected_via_tablets(self): + """ScyllaDB is recognised from TABLETS_ROUTING_V1 alone.""" + pf = ProtocolFeatures.parse_from_supported({ + 'TABLETS_ROUTING_V1': [''], + }) + assert pf.is_scylla is True + assert pf.shard_id == 0 + assert pf.sharding_info is None + + def test_is_scylla_detected_via_sharding(self): + """ScyllaDB with full sharding is recognised and sharding_info is populated.""" + pf = ProtocolFeatures.parse_from_supported({ + 'SCYLLA_SHARD': ['3'], + 'SCYLLA_NR_SHARDS': ['12'], + 'SCYLLA_PARTITIONER': ['org.apache.cassandra.dht.Murmur3Partitioner'], + 'SCYLLA_SHARDING_ALGORITHM': ['biased-token-round-robin'], + 'SCYLLA_SHARDING_IGNORE_MSB': ['12'], + 'SCYLLA_LWT_ADD_METADATA_MARK': ['LWT_OPTIMIZATION_META_BIT_MASK=8'], + }) + assert pf.is_scylla is True + assert pf.shard_id == 3 + assert pf.sharding_info is not None + assert pf.sharding_info.shards_count == 12 + + def test_cassandra_is_not_scylla(self): + """Pure Cassandra SUPPORTED response must not set is_scylla.""" + pf = ProtocolFeatures.parse_from_supported({ + 'CQL_VERSION': ['3.0.0'], + 'COMPRESSION': ['lz4', 'snappy'], + }) + assert pf.is_scylla is False + assert pf.sharding_info is None + + def test_scylla_without_sharding_no_crash(self): + """ + Regression test for F1: SCYLLA_PARTITIONER present but SCYLLA_NR_SHARDS + and SCYLLA_SHARDING_IGNORE_MSB absent must not raise TypeError. + Mirrors the scenario where only some shard fields are advertised. + """ + # Should not raise even though shards_count / sharding_ignore_msb are None. + pf = ProtocolFeatures.parse_from_supported({ + 'SCYLLA_PARTITIONER': ['org.apache.cassandra.dht.Murmur3Partitioner'], + 'SCYLLA_LWT_ADD_METADATA_MARK': ['LWT_OPTIMIZATION_META_BIT_MASK=8'], + }) + assert pf.is_scylla is True + # SCYLLA_PARTITIONER passes the sharding guard, so sharding_info is populated + # with zero defaults rather than crashing. + assert pf.shard_id == 0 + assert pf.sharding_info is not None + assert pf.sharding_info.shards_count == 0 + assert pf.sharding_info.sharding_ignore_msb == 0 + + def test_scylla_sharding_algorithm_only_no_crash(self): + """ + Regression: SCYLLA_SHARDING_ALGORITHM present without SCYLLA_NR_SHARDS + must not raise TypeError. + """ + pf = ProtocolFeatures.parse_from_supported({ + 'SCYLLA_SHARDING_ALGORITHM': ['biased-token-round-robin'], + 'SCYLLA_RATE_LIMIT_ERROR': ['ERROR_CODE=42'], + }) + assert pf.is_scylla is True + assert pf.shard_id == 0 + assert pf.sharding_info is not None + assert pf.sharding_info.shards_count == 0 From e9f95b6dfab2e69ec827484123f90a05b98cf6b2 Mon Sep 17 00:00:00 2001 From: Yaniv Michael Kaul Date: Wed, 29 Jul 2026 18:17:41 +0300 Subject: [PATCH 2/3] fix: don't fabricate partial ShardingInfo when shard fields are incomplete ScyllaDB gates all SCYLLA_SHARD*/SCYLLA_NR_SHARDS/SCYLLA_PARTITIONER fields behind the same allow_shard_aware_drivers server-side config, so partial sharding info cannot occur in practice. Constructing a ShardingInfo with a zeroed-out shards_count would still violate the invariant, relied on elsewhere (e.g. pool.py's shard-aware pooling), that a non-None sharding_info implies a real, positive shard count. Revert the int(x) if x else 0 defensive guards in _ShardingInfo / ShardingInfo back to plain int(x) (cassandra/shard_info.py, cassandra/c_shard_info.pyx now match master exactly again), and instead tighten ProtocolFeatures.parse_sharding_info to detect an incomplete field set up front and return (0, None) rather than ever constructing a partial ShardingInfo. Also simplify SchemaParserV3._is_not_scylla(): ProtocolFeatures.is_scylla is always present once features is set, so the only thing worth guarding against is features itself being None/absent. Update the two crash-regression tests to assert the corrected behavior (is_scylla True, sharding_info None) instead of asserting a fabricated zero-shard ShardingInfo. Co-Authored-By: Claude Sonnet 5 --- cassandra/c_shard_info.pyx | 4 ++-- cassandra/metadata.py | 10 +++++----- cassandra/shard_info.py | 4 ++-- tests/unit/test_protocol_features.py | 25 ++++++++++++------------- 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/cassandra/c_shard_info.pyx b/cassandra/c_shard_info.pyx index 1564934185..a8affd9bba 100644 --- a/cassandra/c_shard_info.pyx +++ b/cassandra/c_shard_info.pyx @@ -29,10 +29,10 @@ cdef class ShardingInfo(): def __init__(self, shard_id, shards_count, partitioner, sharding_algorithm, sharding_ignore_msb, shard_aware_port, shard_aware_port_ssl): - self.shards_count = int(shards_count) if shards_count else 0 + self.shards_count = int(shards_count) self.partitioner = partitioner self.sharding_algorithm = sharding_algorithm - self.sharding_ignore_msb = int(sharding_ignore_msb) if sharding_ignore_msb else 0 + self.sharding_ignore_msb = int(sharding_ignore_msb) self.shard_aware_port = int(shard_aware_port) if shard_aware_port else 0 self.shard_aware_port_ssl = int(shard_aware_port_ssl) if shard_aware_port_ssl else 0 diff --git a/cassandra/metadata.py b/cassandra/metadata.py index 1cfaabd496..72bb5ff4da 100644 --- a/cassandra/metadata.py +++ b/cassandra/metadata.py @@ -2580,12 +2580,12 @@ class SchemaParserV3(SchemaParserV22): def _is_not_scylla(self): """Check if NOT connected to ScyllaDB. - Uses the is_scylla flag from ProtocolFeatures, which is set from the - presence of Scylla-specific extension keys in the SUPPORTED response - (e.g. SCYLLA_LWT_ADD_METADATA_MARK, SCYLLA_RATE_LIMIT_ERROR) and - therefore remains True even when shard-awareness is disabled. + Uses ProtocolFeatures.is_scylla, which is set from Scylla-specific + extension keys in the SUPPORTED response and therefore stays True + even when shard-awareness is disabled server-side. """ - return not getattr(getattr(self.connection, 'features', None), 'is_scylla', False) + features = getattr(self.connection, 'features', None) + return not (features is not None and features.is_scylla) _table_name_col = 'table_name' diff --git a/cassandra/shard_info.py b/cassandra/shard_info.py index b671281021..8f62252193 100644 --- a/cassandra/shard_info.py +++ b/cassandra/shard_info.py @@ -21,10 +21,10 @@ class _ShardingInfo(object): def __init__(self, shard_id, shards_count, partitioner, sharding_algorithm, sharding_ignore_msb, shard_aware_port, shard_aware_port_ssl): - self.shards_count = int(shards_count) if shards_count else 0 + self.shards_count = int(shards_count) self.partitioner = partitioner self.sharding_algorithm = sharding_algorithm - self.sharding_ignore_msb = int(sharding_ignore_msb) if sharding_ignore_msb else 0 + self.sharding_ignore_msb = int(sharding_ignore_msb) self.shard_aware_port = int(shard_aware_port) if shard_aware_port else None self.shard_aware_port_ssl = int(shard_aware_port_ssl) if shard_aware_port_ssl else None diff --git a/tests/unit/test_protocol_features.py b/tests/unit/test_protocol_features.py index 71be4c3dae..a7a7cd0e57 100644 --- a/tests/unit/test_protocol_features.py +++ b/tests/unit/test_protocol_features.py @@ -82,27 +82,27 @@ def test_cassandra_is_not_scylla(self): def test_scylla_without_sharding_no_crash(self): """ - Regression test for F1: SCYLLA_PARTITIONER present but SCYLLA_NR_SHARDS - and SCYLLA_SHARDING_IGNORE_MSB absent must not raise TypeError. - Mirrors the scenario where only some shard fields are advertised. + Regression test for F1: SCYLLA_PARTITIONER present but SCYLLA_SHARD / + SCYLLA_NR_SHARDS absent must not raise TypeError. ScyllaDB gates all + sharding fields together server-side, so this is not expected in + practice, but if it happens the driver must not fabricate a partial + ShardingInfo -- it should just disable shard-aware routing while + still recognising the server as ScyllaDB via the LWT extension key. """ - # Should not raise even though shards_count / sharding_ignore_msb are None. pf = ProtocolFeatures.parse_from_supported({ 'SCYLLA_PARTITIONER': ['org.apache.cassandra.dht.Murmur3Partitioner'], 'SCYLLA_LWT_ADD_METADATA_MARK': ['LWT_OPTIMIZATION_META_BIT_MASK=8'], }) assert pf.is_scylla is True - # SCYLLA_PARTITIONER passes the sharding guard, so sharding_info is populated - # with zero defaults rather than crashing. + # Incomplete sharding fields -- no ShardingInfo is fabricated. assert pf.shard_id == 0 - assert pf.sharding_info is not None - assert pf.sharding_info.shards_count == 0 - assert pf.sharding_info.sharding_ignore_msb == 0 + assert pf.sharding_info is None def test_scylla_sharding_algorithm_only_no_crash(self): """ - Regression: SCYLLA_SHARDING_ALGORITHM present without SCYLLA_NR_SHARDS - must not raise TypeError. + Regression: SCYLLA_SHARDING_ALGORITHM present without SCYLLA_SHARD / + SCYLLA_NR_SHARDS must not raise TypeError, and must not fabricate a + partial ShardingInfo. """ pf = ProtocolFeatures.parse_from_supported({ 'SCYLLA_SHARDING_ALGORITHM': ['biased-token-round-robin'], @@ -110,5 +110,4 @@ def test_scylla_sharding_algorithm_only_no_crash(self): }) assert pf.is_scylla is True assert pf.shard_id == 0 - assert pf.sharding_info is not None - assert pf.sharding_info.shards_count == 0 + assert pf.sharding_info is None From ce9ac1a104e482af1a7205c1c4054f1933ccf9e6 Mon Sep 17 00:00:00 2001 From: Yaniv Michael Kaul Date: Wed, 29 Jul 2026 20:06:29 +0300 Subject: [PATCH 3/3] test: isolate is_scylla-via-sharding detection from LWT key (review feedback) --- tests/unit/test_protocol_features.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_protocol_features.py b/tests/unit/test_protocol_features.py index a7a7cd0e57..3ab5aee820 100644 --- a/tests/unit/test_protocol_features.py +++ b/tests/unit/test_protocol_features.py @@ -57,14 +57,17 @@ def test_is_scylla_detected_via_tablets(self): assert pf.sharding_info is None def test_is_scylla_detected_via_sharding(self): - """ScyllaDB with full sharding is recognised and sharding_info is populated.""" + """ScyllaDB with full sharding is recognised and sharding_info is populated. + + Deliberately omits SCYLLA_LWT_ADD_METADATA_MARK so this test isolates + is_scylla detection via sharding_info, not the LWT extension key. + """ pf = ProtocolFeatures.parse_from_supported({ 'SCYLLA_SHARD': ['3'], 'SCYLLA_NR_SHARDS': ['12'], 'SCYLLA_PARTITIONER': ['org.apache.cassandra.dht.Murmur3Partitioner'], 'SCYLLA_SHARDING_ALGORITHM': ['biased-token-round-robin'], 'SCYLLA_SHARDING_IGNORE_MSB': ['12'], - 'SCYLLA_LWT_ADD_METADATA_MARK': ['LWT_OPTIMIZATION_META_BIT_MASK=8'], }) assert pf.is_scylla is True assert pf.shard_id == 3