Improvement: add Cluster.eager_prepare_scope for DC/rack-scoped eager prepare - prepare should be done only on local rack / local DC / etc. - #976
Open
mykaul wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: QUIET Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds Sequence Diagram(s)sequenceDiagram
participant Cluster
participant LoadBalancingPolicy
participant _prepare_all_queries
participant HostConnection
Cluster->>LoadBalancingPolicy: calculate host distance
Cluster->>_prepare_all_queries: prepare queries with host distance
_prepare_all_queries->>_prepare_all_queries: check eager_prepare_scope
_prepare_all_queries->>HostConnection: prepare eligible host
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cassandra/cluster.py`:
- Around line 1302-1305: Update the Cluster constructor parameter ordering
around eager_prepare_scope so existing positional arguments, including
execution_profiles and all later parameters, retain their original positions.
Append eager_prepare_scope after the existing constructor parameters and
preserve its current default and behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: QUIET
Plan: Pro Plus
Run ID: d2c94c4c-10ce-4176-be65-1ba9588af28d
📒 Files selected for processing (4)
CHANGELOG.rstcassandra/cluster.pydocs/api/cassandra/cluster.rsttests/unit/test_cluster.py
mykaul
changed the title
scylla-drivers#127 asks why prepare_on_all_hosts and reprepare_on_up eagerly prepare statements on every pooled host, including hosts kept open only as cross-DC fallback (HostDistance.REMOTE), and what other drivers do about it. Neither correctness path depends on this: an UNPREPARED response always triggers on-demand reprepare-and-retry regardless of these settings, so eager preparation is purely a latency optimization that large multi-DC clusters can end up paying for on rarely-queried remote hosts. Add EagerPrepareScope (NONE/LOCAL_RACK/LOCAL_DC/ALL) and a new Cluster.eager_prepare_scope attribute, defaulting to ALL so existing behavior is unchanged. Session.prepare_on_all_hosts and Cluster._prepare_all_queries now consult it before eagerly preparing against a candidate host, falling back to lazy on-first-use preparation for hosts outside scope. Cluster.__init__ validates the value the same way allow_control_connection_query_fallback already does. on_up/on_add now pass their already-computed HostDistance into _prepare_all_queries instead of having it recompute it a second time. Tests: tests/unit/test_cluster.py covers the default value, type validation, the full includes_distance truth table, and the scoping behavior of both _prepare_all_queries and Session.prepare_on_all_hosts (including that a caller-provided distance is reused rather than recomputed). Full unit suite run clean modulo three pre-existing failures unrelated to this change and present on unmodified master.
mykaul
force-pushed
the
eager-prepare-scope
branch
from
August 11, 2026 12:29
38d7fa8 to
4d4f731
Compare
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Motivation
scylla-drivers#127 asks why
prepare_on_all_hostsandreprepare_on_upeagerly prepare statements on every host with an open connection pool, including hosts kept connected only as cross-DC fallback (HostDistance.REMOTE), and what other drivers do about it. Neither setting is load-bearing for correctness: anUNPREPAREDresponse from the server always triggers on-demand reprepare-and-retry regardless of these flags, so eager preparation is purely a latency optimization. On large, multi-DC clusters that optimization currently gets paid for unconditionally on remote hosts that are rarely or never queried on the happy path.Change
Adds
EagerPrepareScope(NONE/LOCAL_RACK/LOCAL_DC/ALL) and a newCluster.eager_prepare_scopeattribute, defaulting toALLso existing behavior is unchanged for every current deployment.Session.prepare_on_all_hostsandCluster._prepare_all_queriesnow consult this scope before eagerly preparing against a candidate host; hosts outside the configured scope simply fall back to lazy, on-first-use preparation.Cluster.__init__validates the value with the sameisinstancepattern already used forallow_control_connection_query_fallback. As a secondary cleanup,on_up/on_addnow pass their already-computedHostDistanceinto_prepare_all_queriesinstead of having it recompute the same value a second time.Test
tests/unit/test_cluster.pycovers the default value, constructor type validation, the fullincludes_distancetruth table across all four scopes andHostDistancevalues, the scoping behavior of both_prepare_all_queriesandSession.prepare_on_all_hosts, and that a caller-provided distance is reused rather than recomputed (verified by temporarily reverting that fix and confirming the new test fails against the broken version). Full unit suite passes modulo three pre-existing failures unrelated to this change that are also present on unmodifiedmaster.