You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
pool: reject hosts displaced from metadata - #1166
#860 permits an asynchronous pool attempt to publish after metadata has replaced its Host. Metadata publishes the new owner before retiring the old object, so endpoint and generation checks can still accept the displaced lifecycle. A stale renewal can overwrite and close a healthy replacement pool during that interval.
Fix
Require the exact Host object to own its metadata entry before claiming pool intent and after load-balancing callbacks. Recheck metadata ownership for constructor and keyspace failures, and hold that ownership stable through final pool publication. Acquire Connection locks before the metadata guard, and run callbacks, network waits and candidate cleanup outside it.
Captured reconciliation snapshots and stale direct renewals leave the replacement pool, generation, intent and distance untouched. Existing completion behavior for competing attempts on the current Host is preserved. This changes internal Session pool lifecycle handling without public API or protocol changes.
Deterministic regressions cover both publication orders, endpoint moves, different-ID replacements at the same endpoint, equal-ID Host objects, missing metadata owners, policy callbacks, keyspace synchronization and constructor failures.
Validation
Affected cluster, lifecycle, pool, metadata, control-connection and response-future suites pass; 32 focused metadata-ownership regressions pass.
Full asyncio unit suites pass under TZ=UTC: 2,136 tests with the normal extension build and 2,058 with a clean pure-Python build. The fixture preparation commit also passes its affected suites (435 tests).
Live IP-change and background-shard keyspace scenarios pass with SCYLLA_VERSION=release:2025.2.
The default-timezone full runs reproduce one unrelated DateRange year-rounding failure, also confirmed on unchanged master. Both UTC runs avoid that timezone-dependent failure.
No actionable comments were generated in the recent review.
Summary
Summary by CodeRabbit
Bug Fixes
Improved connection pool handling when hosts are replaced, removed, or moved. Outdated pool operations can no longer affect the current host, and stale connections are shut down without incorrectly reporting a connection failure or marking a host as down.
Improved pool updates during concurrent host changes, helping ensure that only the current host’s connection pool is used.
Pool creation and reconciliation check whether each host is the current metadata object. Pool publication validates ownership before installing a candidate and discards stale candidates. Tests cover host replacement, removal, endpoint movement, and races during construction and publication.
Sequence Diagram(s)
sequenceDiagram
participant Metadata
participant Session
participant Pool
Session->>Metadata: Check host ownership
Session->>Pool: Construct candidate
Pool->>Session: Return candidate
Session->>Metadata: Validate ownership during publication
Session->>Session: Install candidate or discard it
Loading
Possibly related issues
scylladb/python-driver#921 — The change prevents stale pool publication after host replacement or endpoint movement.
scylladb/python-driver#1144 — Its endpoint-replacement tests cover stale candidate rejection addressed by this change.
Suggested reviewers:sylwiaszunejko
Merge Risk: Minimal · up to a1aa8
No identified issue blocks merging after normal checks.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Title check
Title "pool: reject hosts displaced from metadata" names the primary change: pool creation and publication now reject Host objects that metadata has replaced or moved. It is specific and scoped to the…
Description check
The description covers the issue (#860), the fix, and validation. It includes the Fixes: annotation and most checklist items. Two items remain unchecked: docstrings for public items and documentation …
Linked Issues check
[#860] Pool admission, reconciliation, failure handling, and final publication now require exact Host ownership in metadata. Final publication validates ownership while holding the usable-connection…
Out of Scope Changes check
The changes stay within [#860]. Metadata._host_ownership_guard supports the lifecycle checks. cassandra/cluster.py changes enforce stale-pool rejection. The test and fixture changes verify the req…
Full details: Title check
Explanation
Title "pool: reject hosts displaced from metadata" names the primary change: pool creation and publication now reject Host objects that metadata has replaced or moved. It is specific and scoped to the pool subsystem, and it matches the raw summary and objectives.
Full details: Description check
Explanation
The description covers the issue (#860), the fix, and validation. It includes the Fixes: annotation and most checklist items. Two items remain unchecked: docstrings for public items and documentation under ./docs/source/. The PR states no public API changes, so these may not apply, but the author did not address them. The description also lists a required integration scenario (live IP change and USE during background shard connection creation) that the validation section reports as passing.
Resolution
Confirm that no docs or docstrings are needed, or mark those two checklist items accordingly. The description states no public API changes, so the unchecked items may be intentional.
Full details: Linked Issues check
Explanation
[#860] Pool admission, reconciliation, failure handling, and final publication now require exact Host ownership in metadata. Final publication validates ownership while holding the usable-connection lock. Stale candidates are rejected and cleaned up. The added regressions cover endpoint moves, host replacement, removal, callbacks, constructor failures, keyspace synchronization, and publication ordering.
Full details: Out of Scope Changes check
Explanation
The changes stay within [#860]. Metadata._host_ownership_guard supports the lifecycle checks. cassandra/cluster.py changes enforce stale-pool rejection. The test and fixture changes verify the required race conditions. No unrelated behavior or public API change is demonstrated.
Full details: Docstring Coverage
Explanation
Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 18 functions across 5 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
CodeRabbit Verify
Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: QUIET
Plan: Essentials
Run ID: a3256d4b-52ab-4395-b301-08539cb8ee5b
Commits
Reviewing files that changed from the base of the PR and between c8fba18 and a1aa84e.
Files selected for processing: 2
cassandra/cluster.py
tests/unit/test_pool_metadata_ownership.py
Included review availability: This review used your included allowance. 2 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour.
The reason will be displayed to describe this comment to others. Learn more.
[Minor] 🟡 The not owns_host branch does not release the claimed intent, unlike the _is_removed branch below it (pops pool_intents[host] when current_intent is intent). If the host is displaced during distance(), the completed intent stays in _pool_intents holding the displaced Host until the replacement's next claim overwrites it or remove_pool/shutdown clears it. In the plain-removal case no replacement exists, so it lingers. Same shape in update_created_pools (line 5076). test_distance_callback_displaces_owner compares _pool_intents to a snapshot taken after the claim, so it accepts the leftover.
Metadata can publish a replacement Host before retiring its predecessor.
Endpoint and generation checks alone let the displaced lifecycle replace
a healthy session pool during that interval.
Validate exact metadata ownership before claiming pool intents, after
policy callbacks, on setup failures and through final pool publication.
Acquire Connection locks before the metadata guard and keep callbacks,
network waits and candidate cleanup outside it. Release displaced intents
only while they remain current, preserving replacement ownership.
Register metadata owners in existing pool fixtures and cover publication
orders, replacement identities, stale admission and setup failures. Check
intent cleanup and completion across policy distances and use bounded
thread probes for lock ordering.
Fixes: #860
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
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.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Issue
#860 permits an asynchronous pool attempt to publish after metadata has replaced its Host. Metadata publishes the new owner before retiring the old object, so endpoint and generation checks can still accept the displaced lifecycle. A stale renewal can overwrite and close a healthy replacement pool during that interval.
Fix
Require the exact Host object to own its metadata entry before claiming pool intent and after load-balancing callbacks. Recheck metadata ownership for constructor and keyspace failures, and hold that ownership stable through final pool publication. Acquire Connection locks before the metadata guard, and run callbacks, network waits and candidate cleanup outside it.
Captured reconciliation snapshots and stale direct renewals leave the replacement pool, generation, intent and distance untouched. Existing completion behavior for competing attempts on the current Host is preserved. This changes internal Session pool lifecycle handling without public API or protocol changes.
Deterministic regressions cover both publication orders, endpoint moves, different-ID replacements at the same endpoint, equal-ID Host objects, missing metadata owners, policy callbacks, keyspace synchronization and constructor failures.
Validation
TZ=UTC: 2,136 tests with the normal extension build and 2,058 with a clean pure-Python build. The fixture preparation commit also passes its affected suites (435 tests).SCYLLA_VERSION=release:2025.2.Fixes: #860
Pre-review checklist
./docs/source/.Fixes:annotations to PR description.Required integration scenarios: live IP change and USE during background shard connection creation (both verified on Scylla 2025.2).