From bea45ac241fffbf458fd24f40bdff5d09658db9b Mon Sep 17 00:00:00 2001 From: Yaniv Michael Kaul Date: Tue, 16 Jun 2026 15:33:26 +0300 Subject: [PATCH] fix: detect ScyllaDB via SUPPORTED protocol extensions, not shard count The driver previously identified ScyllaDB 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. The immediate consequence is that the driver enables peers_v2 (which ScyllaDB does not support) and fails to connect -- the same regression described in scylladb/gocql#902. Fix (mirrors scylladb/gocql#903): * Add ProtocolFeatures.is_scylla: set to True whenever ANY known Scylla-specific extension key (SCYLLA_LWT_ADD_METADATA_MARK, SCYLLA_RATE_LIMIT_ERROR, TABLETS_ROUTING_V1) is present in the SUPPORTED response, or whenever sharding_info is populated. This flag stays True even when sharding is disabled. * ProtocolFeatures.sharding_info remains None when sharding is disabled, so shard-aware connection pooling (pool.py) is correctly left inactive. * cluster.py: use is_scylla (not sharding_info is not None) to gate the peers_v2 disable and the USING TIMEOUT metadata request timeout. * metadata.py _is_not_scylla(): use is_scylla instead of the previous check (shard_id is None), which was always False after the OPTIONS exchange and therefore silently broke Cassandra trigger-metadata queries. * protocol_features.py parse_sharding_info(): fix a latent crash -- if SCYLLA_PARTITIONER or SCYLLA_SHARDING_ALGORITHM were present without SCYLLA_SHARD, int(None) would raise TypeError. Default shard_id to 0. Fixes: scylladb/python-driver# See also: scylladb/gocql#902, scylladb/gocql#903 Fixes: https://github.com/scylladb/python-driver/issues/909 --- cassandra/c_shard_info.pyx | 4 +- cassandra/cluster.py | 11 ++-- cassandra/metadata.py | 10 +++- cassandra/protocol_features.py | 34 +++++++++-- cassandra/shard_info.py | 4 +- tests/unit/test_protocol_features.py | 84 ++++++++++++++++++++++++++++ 6 files changed, 131 insertions(+), 16 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 1181c6f686..7b0325a3b7 100644 --- a/cassandra/cluster.py +++ b/cassandra/cluster.py @@ -3888,14 +3888,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 877998be7d..3eda2e7d3f 100644 --- a/cassandra/protocol_features.py +++ b/cassandra/protocol_features.py @@ -17,13 +17,15 @@ class ProtocolFeatures(object): sharding_info = None tablets_routing_v1 = False lwt_info = None + is_scylla = False - 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): @@ -31,7 +33,27 @@ 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) - return ProtocolFeatures(rate_limit_error, shard_id, sharding_info, tablets_routing_v1, lwt_info) + is_scylla = ProtocolFeatures.detect_scylla(supported, sharding_info) + return ProtocolFeatures(rate_limit_error, shard_id, sharding_info, tablets_routing_v1, lwt_info, 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): @@ -73,8 +95,12 @@ def parse_sharding_info(options): sharding_algorithm == "biased-token-round-robin" or sharding_ignore_msb): 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) + # SCYLLA_SHARD may be absent even when other shard fields are present + # (e.g. the connection landed on shard 0 and the server omits the field). + # Default to 0 to avoid int(None) crash. + resolved_shard_id = int(shard_id) if shard_id is not None else 0 + return resolved_shard_id, _ShardingInfo(shard_id, shards_count, partitioner, sharding_algorithm, sharding_ignore_msb, + shard_aware_port, shard_aware_port_ssl) @staticmethod 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..59cdfd67cc 100644 --- a/tests/unit/test_protocol_features.py +++ b/tests/unit/test_protocol_features.py @@ -22,3 +22,87 @@ 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.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.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.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.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.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.sharding_info is not None + assert pf.sharding_info.shards_count == 0