Skip to content

Commit 2dce180

Browse files
Copilotmykaul
andcommitted
Add proper paging support for ControlConnection topology queries
- Implement _fetch_all_pages helper method to handle multi-page results - Apply paging to all topology queries that use fetch_size - Ensures all pages are fetched for large clusters (>1000 nodes) - Add test to verify multi-page fetching works correctly This addresses the review feedback that wait_for_responses doesn't handle paging_state, so we need to manually fetch all pages. Co-authored-by: mykaul <4655593+mykaul@users.noreply.github.com>
1 parent faa59e6 commit 2dce180

2 files changed

Lines changed: 84 additions & 1 deletion

File tree

cassandra/cluster.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3659,6 +3659,10 @@ def _try_connect(self, host):
36593659
peers_result = connection.wait_for_response(
36603660
peers_query, timeout=self._timeout)
36613661

3662+
# Fetch all pages if there are more results
3663+
peers_result = self._fetch_all_pages(connection, peers_result, peers_query, self._timeout)
3664+
local_result = self._fetch_all_pages(connection, local_result, local_query, self._timeout)
3665+
36623666
shared_results = (peers_result, local_result)
36633667
self._refresh_node_list_and_token_map(connection, preloaded_results=shared_results)
36643668
self._refresh_schema(connection, preloaded_results=shared_results, schema_agreement_wait=-1)
@@ -3719,6 +3723,26 @@ def _submit(self, *args, **kwargs):
37193723
pass
37203724
return None
37213725

3726+
def _fetch_all_pages(self, connection, result, query_msg, timeout):
3727+
"""
3728+
Fetch all pages for a paged query result.
3729+
Returns the result with all parsed_rows combined from all pages.
3730+
"""
3731+
if not result.paging_state:
3732+
return result
3733+
3734+
all_rows = list(result.parsed_rows)
3735+
3736+
while result.paging_state:
3737+
query_msg.paging_state = result.paging_state
3738+
result = connection.wait_for_response(query_msg, timeout=timeout)
3739+
if result.parsed_rows:
3740+
all_rows.extend(result.parsed_rows)
3741+
3742+
# Update the result with all rows
3743+
result.parsed_rows = all_rows
3744+
return result
3745+
37223746
def shutdown(self):
37233747
# stop trying to reconnect (if we are)
37243748
with self._reconnection_lock:
@@ -3807,6 +3831,10 @@ def _refresh_node_list_and_token_map(self, connection, preloaded_results=None,
38073831
fetch_size=self._schema_meta_page_size)
38083832
peers_result, local_result = connection.wait_for_responses(
38093833
peers_query, local_query, timeout=self._timeout)
3834+
3835+
# Fetch all pages if there are more results
3836+
peers_result = self._fetch_all_pages(connection, peers_result, peers_query, self._timeout)
3837+
local_result = self._fetch_all_pages(connection, local_result, local_query, self._timeout)
38103838

38113839
peers_result = dict_factory(peers_result.column_names, peers_result.parsed_rows)
38123840

@@ -3866,6 +3894,9 @@ def _refresh_node_list_and_token_map(self, connection, preloaded_results=None,
38663894
success, local_rpc_address_result = connection.wait_for_response(
38673895
local_rpc_address_query, timeout=self._timeout, fail_on_error=False)
38683896
if success:
3897+
# Fetch all pages if there are more results (though system.local has only 1 row)
3898+
local_rpc_address_result = self._fetch_all_pages(connection, local_rpc_address_result,
3899+
local_rpc_address_query, self._timeout)
38693900
row = dict_factory(
38703901
local_rpc_address_result.column_names,
38713902
local_rpc_address_result.parsed_rows)
@@ -4107,6 +4138,10 @@ def wait_for_schema_agreement(self, connection=None, preloaded_results=None, wai
41074138
timeout = min(self._timeout, total_timeout - elapsed)
41084139
peers_result, local_result = connection.wait_for_responses(
41094140
peers_query, local_query, timeout=timeout)
4141+
4142+
# Fetch all pages if there are more results
4143+
peers_result = self._fetch_all_pages(connection, peers_result, peers_query, timeout)
4144+
local_result = self._fetch_all_pages(connection, local_result, local_query, timeout)
41104145
except OperationTimedOut as timeout:
41114146
log.debug("[control connection] Timed out waiting for "
41124147
"response during schema agreement check: %s", timeout)

tests/unit/test_control_connection.py

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
from concurrent.futures import ThreadPoolExecutor
1818
from unittest.mock import Mock, ANY, call
1919

20-
from cassandra import OperationTimedOut, SchemaTargetType, SchemaChangeType
20+
from cassandra import OperationTimedOut, SchemaTargetType, SchemaChangeType, ConsistencyLevel
2121
from cassandra.protocol import ResultMessage, RESULT_KIND_ROWS, QueryMessage
2222
from cassandra.cluster import ControlConnection, _Scheduler, ProfileManager, EXEC_PROFILE_DEFAULT, ExecutionProfile
2323
from cassandra.pool import Host
@@ -323,6 +323,54 @@ def test_topology_queries_use_paging(self):
323323
assert isinstance(query_msg, QueryMessage)
324324
assert query_msg.fetch_size == self.control_connection._schema_meta_page_size
325325

326+
def test_topology_queries_fetch_all_pages(self):
327+
"""
328+
Test that topology queries fetch all pages when results are paged
329+
"""
330+
from unittest.mock import MagicMock
331+
332+
# Create mock connection
333+
mock_connection = MagicMock()
334+
mock_connection.endpoint = DefaultEndPoint("192.168.1.0")
335+
mock_connection.original_endpoint = mock_connection.endpoint
336+
337+
# Create first page of peers results with paging_state
338+
first_page = ResultMessage(kind=RESULT_KIND_ROWS)
339+
first_page.column_names = ["rpc_address", "peer", "schema_version", "data_center", "rack", "tokens", "host_id"]
340+
first_page.parsed_rows = [["192.168.1.1", "10.0.0.1", "a", "dc1", "rack1", ["1", "101", "201"], "uuid2"]]
341+
first_page.paging_state = b"has_more_pages"
342+
343+
# Create second page of peers results without paging_state
344+
second_page = ResultMessage(kind=RESULT_KIND_ROWS)
345+
second_page.column_names = ["rpc_address", "peer", "schema_version", "data_center", "rack", "tokens", "host_id"]
346+
second_page.parsed_rows = [["192.168.1.2", "10.0.0.2", "a", "dc1", "rack1", ["2", "102", "202"], "uuid3"]]
347+
second_page.paging_state = None
348+
349+
# Create local result without paging
350+
local_result = ResultMessage(kind=RESULT_KIND_ROWS)
351+
local_result.column_names = ["rpc_address", "schema_version", "cluster_name", "data_center", "rack", "partitioner", "release_version", "tokens", "host_id"]
352+
local_result.parsed_rows = [["192.168.1.0", "a", "foocluster", "dc1", "rack1", "Murmur3Partitioner", "2.2.0", ["0", "100", "200"], "uuid1"]]
353+
local_result.paging_state = None
354+
355+
# Setup mock: first call returns first page, second call returns second page
356+
mock_connection.wait_for_responses.return_value = (first_page, local_result)
357+
mock_connection.wait_for_response.return_value = second_page
358+
359+
# Test _fetch_all_pages
360+
self.control_connection._connection = mock_connection
361+
query_msg = QueryMessage(query="SELECT * FROM system.peers", consistency_level=ConsistencyLevel.ONE, fetch_size=1000)
362+
363+
result = self.control_connection._fetch_all_pages(mock_connection, first_page, query_msg, timeout=5)
364+
365+
# Verify that both pages were fetched
366+
assert len(result.parsed_rows) == 2
367+
assert result.parsed_rows[0][0] == "192.168.1.1"
368+
assert result.parsed_rows[1][0] == "192.168.1.2"
369+
assert result.paging_state is None
370+
371+
# Verify wait_for_response was called once to fetch the second page
372+
assert mock_connection.wait_for_response.called
373+
326374
def test_refresh_nodes_and_tokens_with_invalid_peers(self):
327375
def refresh_and_validate_added_hosts():
328376
self.connection.wait_for_responses = Mock(return_value=_node_meta_results(

0 commit comments

Comments
 (0)