pool: release stopped host reconnection handlers - #1032
dkropachev wants to merge 6 commits into
Conversation
Authentication failures and exhausted retry schedules left their handlers installed on the host. Every later DOWN event then treated the stopped handler as an active reconnection and permanently skipped that host. Clear the slot under the host lock when the handler will not run again. Only clear it when it still belongs to that handler, so a concurrent replacement remains installed.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: QUIET Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
Suggested reviewers: Priority: ➖ Normal Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The change enables fresh reconnection attempts after terminal failures and avoids reconnecting stale or removed hosts. The covered behaviors leave no concrete merge-blocking risk. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Out of Scope Changes checkExplanation The PR adds unrelated host-removal API and behavior in
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. Comment |
After a terminal handler released its slot, ordinary DOWN events still skipped hosts already marked down. Empty retry schedules also stopped before the slot could be released.\n\nRestart reconnection directly for a down host with no active handler, and release a host handler when its schedule is empty before the first attempt.
Repeated DOWN events could start a new reconnector before the original executor task notified policies and removed pools. Removed hosts could take the same path and be reintroduced after a successful attempt. Track pending DOWN work separately from the reconnector slot, ignore lifecycle work for hosts no longer in metadata, and release pending state when processing or submission ends.
Topology refresh removed missing peers from metadata directly, leaving sessions and load-balancing policies unaware. Route removals through Cluster so DOWN guards cannot strand stale hosts.
A topology refresh already owns a complete peer snapshot, so starting another refresh for each missing host can recurse and race the outer token-map rebuild. Carry refresh suppression through the normal removal lifecycle while preserving reconnects when the active control host is removed.
Topology refresh can temporarily index one Host under both its old and new host IDs. Remove missing entries by snapshot ID and run removal lifecycle only when the Host is no longer tracked, so endpoint replacements remain available.
This branch has not been deployed
1 participant
Problem
_HostReconnectionHandlerstayed installed inhost._reconnection_handlerafter an authentication failure or after exhausting its retry schedule.Cluster.on_down()then treated that stopped handler as an active reconnection and skipped every later attempt, permanently losing the host for the lifetime of the cluster.Fix
Release the host reconnection slot whenever its handler will not run again. The update holds the host lock and clears the slot only when it still points to that handler, so a concurrent replacement cannot be removed.
Add unit coverage for authentication failure, schedule exhaustion, continued retries, and the replacement race. Document the fix in the changelog.
No public API, wire protocol, or compatibility behavior changes. The change only corrects internal host-reconnection bookkeeping.
Fixes #1026
Testing
TZ=UTC uv run pytest -rf tests/unit— 1061 passed, 40 skippeduvx --from build pyproject-build— sdist and Cython wheel built successfullygit diff --checkPre-review checklist
./docs/source/. (Not applicable: internal behavior fix; changelog updated.)Fixes:annotations to PR description.