Skip to content

Improvement: add Cluster.eager_prepare_scope for DC/rack-scoped eager prepare - prepare should be done only on local rack / local DC / etc. - #976

Closed
mykaul wants to merge 1 commit into
scylladb:masterfrom
mykaul:eager-prepare-scope
Closed

Improvement: add Cluster.eager_prepare_scope for DC/rack-scoped eager prepare - prepare should be done only on local rack / local DC / etc.#976
mykaul wants to merge 1 commit into
scylladb:masterfrom
mykaul:eager-prepare-scope

Conversation

@mykaul

Copy link
Copy Markdown

Motivation

scylla-drivers#127 asks why prepare_on_all_hosts and reprepare_on_up eagerly 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: an UNPREPARED response 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 new Cluster.eager_prepare_scope attribute, defaulting to ALL so existing behavior is unchanged for every current deployment. Session.prepare_on_all_hosts and Cluster._prepare_all_queries now 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 same isinstance pattern already used for allow_control_connection_query_fallback. As a secondary cleanup, on_up/on_add now pass their already-computed HostDistance into _prepare_all_queries instead of having it recompute the same value a second time.

Test

tests/unit/test_cluster.py covers the default value, constructor type validation, the full includes_distance truth table across all four scopes and HostDistance values, the scoping behavior of both _prepare_all_queries and Session.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 unmodified master.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: QUIET

Plan: Pro Plus

Run ID: 30357653-9149-4ca6-92d8-86bdbefc9d09

📥 Commits

Reviewing files that changed from the base of the PR and between 38d7fa8 and 4d4f731.

📒 Files selected for processing (1)
  • cassandra/cluster.py

📝 Walkthrough

Walkthrough

Adds Cluster.eager_prepare_scope with ALL, LOCAL_DC, LOCAL_RACK, and NONE options. The setting defaults to ALL and requires an EagerPrepareScope value. Host-up, host-add, repreparation, and prepare_on_all_hosts flows filter hosts by host distance. Tests and API documentation cover the setting and preparation behavior.

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
Loading

Suggested reviewers:lorak-mmk, sylwiaszunejko

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the new Cluster.eager_prepare_scope feature and its DC/rack-scoped eager preparation behavior.
Description check✅ PassedThe description explains the motivation, implementation, tests, default behavior, compatibility, and linked issue, although it omits the repository checklist.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai
coderabbitaiBot requested a review from Lorak-mmkAugust 11, 2026 11:52

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between e9773cd and 38d7fa8.

📒 Files selected for processing (4)
  • CHANGELOG.rst
  • cassandra/cluster.py
  • docs/api/cassandra/cluster.rst
  • tests/unit/test_cluster.py

Comment threadcassandra/cluster.py
@mykaulmykaul changed the title Add Cluster.eager_prepare_scope for DC/rack-scoped eager prepareImprovement: add Cluster.eager_prepare_scope for DC/rack-scoped eager prepare - prepare should be done only on local rack / local DC / etc.Aug 11, 2026
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
mykaulforce-pushed the eager-prepare-scope branch from 38d7fa8 to 4d4f731CompareAugust 11, 2026 12:29
@mykaul

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitaiBot commented Aug 12, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@dkropachev

Copy link
Copy Markdown

@mykaul, great idea, but we won't implement it until customer requests it, I expanded it and put together an feature proposal - #987

@mykaul

Copy link
Copy Markdown
Author

@mykaul, great idea, but we won't implement it until customer requests it, I expanded it and put together an feature proposal - #987

@dkropachev , OK, 4 things we could do:

  • close this PR (I did it based on something I saw in a customer environment, btw, I just did not want to change the default)
  • just change the defaults - at least for prepare_on_all_hosts which is just wrong - for multi node, multi DC setups.
  • Implement Add stateful host policy for automatic prepared-statement propagation #987 fully, if something is lacking here, we could add it - here or later.
  • not implement it here, but make sure the defaults at least for python-rs are better than for this driver! prepare_on_all_hosts should be False, reprepare_on_up should be limited to the local AZ/DC, as proposed here.

Your call.

@dkropachev

dkropachev commented Aug 18, 2026

Copy link
Copy Markdown

@mykaul , we can definitely can change default to prepare only on local nodes.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@mykaul@dkropachev