Skip to content

Commit b8b714c

Browse files
committed
DRIVER-153: tests for SCYLLA_USE_METADATA_ID 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.
1 parent fa14e80 commit b8b714c

7 files changed

Lines changed: 1014 additions & 12 deletions

File tree

cassandra/cluster.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4736,7 +4736,7 @@ class ResponseFuture(object):
47364736
_host = None
47374737
_control_connection_query_attempted = False
47384738
_TABLET_ROUTING_CTYPE = None
4739-
_bound_result_metadata = []
4739+
_bound_result_metadata = None
47404740

47414741
_warned_timeout = False
47424742

tests/integration/standard/test_prepared_statements.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -614,13 +614,15 @@ def _test_updated_conditional(self, session, value):
614614
prepared_statement = session.prepare(
615615
"INSERT INTO {}(a, b, d) VALUES "
616616
"(?, ? , ?) IF NOT EXISTS".format(self.table_name))
617-
first_id = prepared_statement.result_metadata_id
618-
LOG.debug('initial result_metadata_id: {}'.format(first_id))
617+
LOG.debug('initial result_metadata_id: {}'.format(prepared_statement.result_metadata_id))
619618

619+
# The cached (result_metadata, result_metadata_id) pair is not asserted on:
620+
# a METADATA_CHANGED response refreshes it for a conditional statement like
621+
# for any other, so its contents are the server's business. What must hold
622+
# is that each result is decoded against the metadata describing it, whether
623+
# the conditional update applied (narrow shape) or not (whole row).
620624
def check_result_and_metadata(expected):
621625
assert session.execute(prepared_statement, (value, value, value)).one() == expected
622-
assert prepared_statement.result_metadata_id == first_id
623-
assert prepared_statement.result_metadata is None
624626

625627
# Successful conditional update
626628
check_result_and_metadata((True,))
Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
# Copyright 2026 ScyllaDB, Inc.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
import unittest
16+
from unittest.mock import patch
17+
18+
import pytest
19+
20+
from cassandra.cluster import ResponseFuture
21+
from tests.integration import use_singledc, SCYLLA_VERSION, BasicSharedKeyspaceUnitTestCase, \
22+
drop_keyspace_shutdown_cluster
23+
24+
pytestmark = pytest.mark.skipif(SCYLLA_VERSION is None, reason="SCYLLA_USE_METADATA_ID is a Scylla-only protocol extension")
25+
26+
27+
def setup_module():
28+
use_singledc()
29+
30+
31+
class ScyllaMetadataIdTests(BasicSharedKeyspaceUnitTestCase):
32+
"""
33+
Live-server coverage for the SCYLLA_USE_METADATA_ID protocol extension (DRIVER-153).
34+
"""
35+
36+
@classmethod
37+
def setUpClass(cls):
38+
cls.common_setup(1)
39+
# Skip the whole class if this Scylla build does not advertise the
40+
# extension (e.g. a version predating scylladb#23292). Without this the
41+
# tests below would error out instead of skipping on an unsupporting node.
42+
try:
43+
if not cls._negotiated_use_metadata_id():
44+
raise unittest.SkipTest(
45+
"Scylla node does not advertise SCYLLA_USE_METADATA_ID")
46+
except Exception:
47+
# setUpClass raising means unittest never calls tearDownClass, so the
48+
# cluster and keyspace created above are torn down here explicitly.
49+
drop_keyspace_shutdown_cluster(cls.ks_name, cls.session, cls.cluster)
50+
raise
51+
52+
@classmethod
53+
def _negotiated_use_metadata_id(cls):
54+
"""Whether this class's data-path connections negotiated SCYLLA_USE_METADATA_ID.
55+
56+
Reads the pool's existing connection rather than borrowing one:
57+
borrow_connection() pops a stream id that only Connection.process_msg gives
58+
back, so borrowing without sending a message would leak it.
59+
"""
60+
pool = next(iter(cls.session.get_pools()))
61+
return next(iter(pool._connections.values())).features.use_metadata_id
62+
63+
def setUp(self):
64+
self.table_name = "{}.{}".format(self.keyspace_name, self.function_table_name)
65+
self.session.execute("CREATE TABLE {} (a int PRIMARY KEY, b int, c int)".format(self.table_name))
66+
self.session.execute("INSERT INTO {} (a, b, c) VALUES (1, 1, 1)".format(self.table_name))
67+
68+
def tearDown(self):
69+
self.session.execute("DROP TABLE {}".format(self.table_name))
70+
71+
def test_extension_is_negotiated(self):
72+
"""
73+
Sanity check that SCYLLA_USE_METADATA_ID was actually negotiated on this
74+
connection. Without this, the tests below could pass vacuously if
75+
negotiation silently failed.
76+
"""
77+
assert self._negotiated_use_metadata_id() is True
78+
79+
def test_metadata_changed_recovers_after_schema_change(self):
80+
"""
81+
Normal METADATA_CHANGED path: after ALTER TABLE, the next EXECUTE must
82+
come back with a fresh result_metadata_id and updated column metadata,
83+
picked up automatically without re-preparing.
84+
"""
85+
prepared = self.session.prepare("SELECT * FROM {} WHERE a = ?".format(self.table_name))
86+
id_before = prepared.result_metadata_id
87+
assert id_before is not None
88+
assert len(prepared.result_metadata) == 3
89+
90+
self.session.execute(prepared.bind((1,)))
91+
92+
self.session.execute("ALTER TABLE {} ADD d int".format(self.table_name))
93+
self.session.execute(prepared.bind((1,)))
94+
95+
assert prepared.result_metadata_id is not None
96+
assert prepared.result_metadata_id != id_before
97+
assert len(prepared.result_metadata) == 4
98+
99+
def test_empty_sentinel_id_triggers_metadata_changed(self):
100+
"""
101+
Statements prepared before the extension was negotiated (e.g. mid rolling
102+
upgrade) start with result_metadata_id=None and must send the empty b''
103+
sentinel on their first EXECUTE. This must not be treated as a protocol
104+
error by the server: it must be treated as a mismatch, causing Scylla to
105+
respond with METADATA_CHANGED (fresh id + full metadata), which the
106+
driver then caches.
107+
"""
108+
prepared = self.session.prepare("SELECT * FROM {} WHERE a = ?".format(self.table_name))
109+
assert prepared.result_metadata_id is not None
110+
111+
# Simulate "prepared before the extension was known" by dropping the
112+
# cached id while keeping the cached metadata (mirrors the java-driver's
113+
# should_handle_empty_metadata_id_when_executing_statement_when_supported).
114+
prepared.update_result_metadata(prepared.result_metadata, None)
115+
assert prepared.result_metadata_id is None
116+
117+
# The table was not altered, so the statement is still valid server-side and
118+
# nothing should re-prepare it. Spying on _reprepare keeps this test honest:
119+
# if the id came back via an UNPREPARED/reprepare round trip instead, the
120+
# METADATA_CHANGED-on-ROWS path in ResponseFuture._set_result would not
121+
# actually be under test here.
122+
with patch.object(ResponseFuture, '_reprepare', autospec=True,
123+
side_effect=ResponseFuture._reprepare) as reprepare_spy:
124+
result = self.session.execute(prepared.bind((1,)))
125+
126+
assert reprepare_spy.call_count == 0
127+
assert list(result) == [(1, 1, 1)]
128+
assert prepared.result_metadata_id is not None
129+
130+
def test_conditional_statement_metadata_is_stable_across_outcomes(self):
131+
"""
132+
Conditional (LWT) statements get no special handling, and this pins the
133+
server behaviour that makes that correct.
134+
135+
Cassandra returns NO_METADATA for a conditional statement at PREPARE and
136+
then varies the result shape per execution — ``(True,)`` when applied, the
137+
conflicting row when not — which is what PYTHON-847 is about. Scylla
138+
instead describes the result up front as ``[applied]`` plus every column of
139+
the row, filling nulls when the update applied. The shape does not
140+
alternate, so the cached metadata id stays valid across both outcomes and
141+
``skip_meta`` is exactly as safe here as for any other statement. A schema
142+
change still changes the id, and the driver must pick that up.
143+
"""
144+
prepared = self.session.prepare(
145+
"INSERT INTO {} (a, b, c) VALUES (?, ?, ?) IF NOT EXISTS".format(self.table_name))
146+
id_before = prepared.result_metadata_id
147+
assert id_before is not None
148+
assert len(prepared.result_metadata) == 4 # [applied], a, b, c
149+
150+
# a=2 is free, so the insert applies; the row columns come back null.
151+
assert self.session.execute(prepared.bind((2, 2, 2))).one() == (True, None, None, None)
152+
153+
# a=1 exists (setUp), so this one does not apply and the conflicting row is
154+
# returned — same metadata, so the cached pair is still the right one.
155+
assert self.session.execute(prepared.bind((1, 9, 9))).one() == (False, 1, 1, 1)
156+
assert prepared.result_metadata_id == id_before
157+
158+
self.session.execute("ALTER TABLE {} ADD d int".format(self.table_name))
159+
160+
# The result gained a column, so the server must report a new id and the
161+
# driver must adopt it — for a conditional statement like for any other.
162+
assert self.session.execute(prepared.bind((1, 9, 9))).one() == (False, 1, 1, 1, None)
163+
assert prepared.result_metadata_id != id_before
164+
assert len(prepared.result_metadata) == 5
165+
166+
assert self.session.execute(prepared.bind((3, 3, 3))).one() == (True, None, None, None, None)

0 commit comments

Comments
 (0)