[dkropachev]

Problem

Executor-backed DOWN handling can outlive host removal. Stale work could notify policies, sessions, the control connection, and listeners; install a reconnector; and later enter UP handling for a terminal Host.

Fix

• Reject new, queued, and restarted DOWN work after terminal removal.
• Serialize each DOWN callback with removal so it runs wholly before removal or is skipped.
• Reject UP handoff for an already removed host.
• Keep the existing locked reconnector-installation fence.

Scope

This PR fixes only #1034. It does not handle:

• topology-discovered removal or refresh ordering: Route topology-discovered host removals through the cluster lifecycle #1007 / cluster: coordinate topology publication and host lifecycle #1036;
• stale request, pool, or connection work: ResponseFuture can return or borrow connections from a stale endpoint pool #857, HostConnection can signal failures from stale or replaced pools #858, HostConnection can publish replacement shard connections after endpoint changes #859, Session pool creation can publish stale pools to replacement hosts #860, Cluster down handling can lose replacement endpoint events while old down work is pending #861;
• endpoint replacement or generation fencing: Introduce explicit endpoint replacement flow for existing host_id #923 / Add topology-generation fencing for pools and connections #925;
• interpreter cleanup racing an accepted connect: cluster: prevent scheduler cleanup from overtaking an accepted connect #1072;
• ordered tablet routing bypassing child-policy exclusions: token-aware policy: preserve child exclusions for ordered tablet routing #1073;
• in-flight UP completion or cleanup after removal/replacement: cluster: in-flight UP completion can mutate a same-ID replacement #1074;
• general UP/ADD policy callback serialization: cluster: serialize positive policy callbacks with host removal #1075;
• replacement-add cleanup after callback failures: cluster: continue replacement-add recovery after cleanup callback errors #1076;
• unrelated no-pool-future UP completion behavior: Cluster: host-up transition without pool futures skips listener notification #852 / cluster: fix host state pool refresh races #854.
• queued host-lifecycle processing that avoids executor workers waiting on user callbacks: Fix pool management #382.

No public API, wire-protocol, or compatibility behavior changes.

Fixes #1034

Testing

• TZ=UTC uv run pytest -q tests/unit — 1388 passed, 40 skipped, 158 subtests passed
• TZ=UTC uv run pytest -q tests/unit/test_cluster.py — 161 passed, 8 subtests passed after review changes
• git diff --check

Pre-review checklist

• I have split my patch into logically separate commits.
• All commit messages clearly explain what they change and why.
• I added relevant tests for new features and bug fixes.
• All commits compile, pass static checks and pass test.
• PR description sums up the changes and reasons why they should be introduced.
• I have provided docstrings for the public items that I want to introduce. (Not applicable: no public items.)
• I have adjusted the documentation in ./docs/source/. (Not applicable: internal lifecycle fix.)
• I added appropriate Fixes: annotations to PR description.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: QUIET
• Plan: Advanced
• Run ID: 51c72003-4eba-430b-a54d-49535666e82d

📥 Commits

Reviewing files that changed from the base of the PR and between 9377cf7 and ebe3aa2.

📒 Files selected for processing (1)

• cassandra/policies.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Host lifecycle processing ignores removed hosts during UP and DOWN handling. DOWN callbacks check removal state and event generation. A per-host lock serializes callback dispatch with removal. Tests cover queued work, stale metadata indexing, callback removal, and concurrent removal.

Sequence Diagram(s)

sequenceDiagram
  participant Cluster
  participant Executor
  participant Host
  participant DownCallbackLock
  participant Callbacks
  Cluster->>Executor: submit DOWN processing
  Executor->>Cluster: run queued DOWN processing
  Cluster->>DownCallbackLock: acquire for callback dispatch
  DownCallbackLock->>Host: check removal state and event generation
  Host-->>DownCallbackLock: return lifecycle state
  DownCallbackLock->>Callbacks: invoke callback if checks pass
  Cluster->>DownCallbackLock: acquire before removal
  DownCallbackLock->>Host: mark down and clear pending-addition state

Loading

Suggested reviewers: nikagra

Priority: ➖ Normal

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to ebe3a

Host removal now waits for an in-flight DOWN callback to finish. The updated documentation warns that executor waits can stall callbacks; no unresolved merge-blocking risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to ebe3a

The change prevents stale DOWN work from acting on removed hosts. However, removal now depends on application callbacks returning: a callback that waits synchronously for removal-related work can stall cleanup. This is a conditional availability risk, not a demonstrated privilege or authentication bypass.

Retained concerns

• Low · reliability · inferred: Terminal removal now waits for an external DOWN callback to finish. If that callback synchronously waits for work that removes the same Host, the work waits for the callback lock and neither can complete. Unlike pre-existing executor starvation, this cycle can occur with a spare worker. It can strand removal after metadata deletion but before the terminal mark, pool cleanup, and reconnector cancellation. The documentation warns against synchronous waits, and no inspected callback demonstrates an actual cycle.

Security review details

Security Blast Radius

• inferred — The direct synchronization scope is one Host, but stalled removal can delay shared Cluster lifecycle work. Removal cleans up policies, sessions, listeners, the control connection, and reconnectors; control-connection topology refresh also invokes removal during host replacement. The supported consequence is availability and cleanup disruption within the affected driver instance, not demonstrated cross-tenant or privilege expansion.

Security Findings and Attack Paths

• inferred — The identified adverse path requires an installed listener or policy that waits for work reaching removal of the same Host. The evidence establishes a conditional callback/removal wait cycle, but does not establish an attacker-controlled implementation, an unprivileged trigger sufficient to exploit it, or a verified security vulnerability.

Trust Boundaries and Controls

• observed — The routed public-entrypoint change in policies.py is documentation, not a new callable interface. Its runtime relationship is callback dispatch into configured listeners and policies. The new control checks terminal removal and event generation before invoking a callback, while avoiding holding the Host state lock across external code.

Resilience and Maintainability Implications

• observed — Dispatch and removal use the same lock order, and context-managed locking releases the callback lock when a callback raises. Nevertheless, removal has no timeout on acquiring that lock, so an indefinitely blocked callback can prevent the terminal mark and downstream cleanup indefinitely.

Hardening Proposals

• proposed — Before adopting the change, audit application-supplied DOWN callbacks for synchronous waits, especially waits on topology refresh or removal work, and keep deferred work asynchronous. This addresses the documented callback contract rather than an observed application defect.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	#1034 remains unmet. The final DOWN-handler installation fence in cassandra/cluster.py checks the event generation under host.lock, but it does not check host._is_removed. If removal marks the h…	Check host._is_removed under host.lock at the final reconnector-installation point. Add a deterministic test that removes the host before that check and verifies that no reconnector is installed.
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.58% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 31 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Out of Scope Changes check	✅ Passed	The callback serialization, removed-host checks, UP rejection, tests, and callback contract documentation support #1034. The diff shows no unrelated changes.
Title check	✅ Passed	The title clearly and concisely describes the main change: ignoring stale DOWN work for removed hosts.
Description check	✅ Passed	The description explains the problem, fix, scope, testing, and checklist items. It includes the required Fixes annotation and marks the applicable checklist items complete.

Full details: Linked Issues check

Explanation

#1034 remains unmet. The final DOWN-handler installation fence in cassandra/cluster.py checks the event generation under host.lock, but it does not check host._is_removed. If removal marks the host removed before the DOWN task acquires that lock, the task can still install a reconnector after removal. The tests do not cover this interleaving.

• Fix all pre-merge checks with AI

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[nikagra]

LGTM, one Minor on removal now waiting for user callbacks.