Repository navigation
SOLR-6213: Run leader election rejoins on their own thread - #5210
Open
nick-boss-tech wants to merge 6 commits into
Open
nick-boss-tech wants to merge 6 commits into
nick-boss-tech wants to merge 6 commits into
Conversation
…s do not grow the stack
…and keep an in-flight join intact
…to the thread's uncaught handler
The context is removed from the map there and only its election node was cancelled, so its rejoin thread and sync executor were never shut down.
nick-boss-tech
marked this pull request as ready for review
October 23, 2026 19:52
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
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.
🤖 AI text below 🤖 (reviewed by Nick Shanin)
Repeated leader rejoins nest on the same thread. When a shard leader's sync fails, its leader publish fails, or it cannot become leader in time,
rejoinLeaderElectioncallsjoinElection, which runs the leader process again. That can callrejoinLeaderElectionagain before the first call returns. Each round adds six frames to the thread's stack and nothing removes them. A replica that keeps failing can grow its stack until it overflows, as SOLR-6213 reports. The nested join is line 494 on main.Each rejoin now runs on its own thread. The three callers return right after
rejoinLeaderElection, so the join can run after they return. The rejoin handsjoinElectionto a lazy single-thread executor that keeps its thread for 10 seconds (line 504). Closing the context shuts that executor down. A rejoin that races with close is dropped with a debug log, and a queued join is skipped once the context is closed. A join already running is not interrupted; it finishes and the thread exits. AKeeperExceptionor runtime exception from the join is logged on the rejoin thread.ZkController.unregisternow also closes the election context it removes, which it did not do before; without that the rejoin thread stays alive for its 10 second keep-alive after the core is gone.Proof.
ShardLeaderElectionRejoinTestforces three failed syncs and records the stack depth at each rejoin. On main with the test's hook, the depth grows by 12 frames over 4 rejoins and the test fails. On this change it passes, in 20 seconds. Both runs are on 2026-10-23 on the test branch, head 64624f4. The hook forces the sync failure and lives only on that branch. This PR contains no test because the hook is not part of it. The neighboring tests (DeleteReplicaTest,ZkControllerTest,ReplaceNodeTest,TestCloudRecovery) pass on the PR head 1fdc74f: 24 tests, 0 failures, 2 skipped. The full module check passes at that head.The choice. Two routes were available:
LeaderElector.Context: each rejoin already waits at a leader throttle of five seconds (line 127), so rejoins are paced.
Ladder: Do the callers need the rejoin to finish before they return? No, each returns right after the call. Does the rejoin need the caller's thread? No, it reads and writes ZooKeeper state only. What does route 2 cost? A failure of the rejoin itself, such as a lost session, is now logged on the rejoin thread instead of thrown to the caller. When the rejoin runs at core registration,
ZkController.registergoes on to wait for a leader, up toleaderVoteWaitplus ten minutes, where it used to fail at once.Recommendation: route 2. It is the smaller change, and the callers keep their behavior. Is that trade-off acceptable?
Limits. No overflow was reproduced on this path. The test shows the growth per rejoin and the pause between rejoins. An overflow needs a long run of failed syncs, and the stack size of the thread was not measured. The test forces the sync-failure caller only; the term-wait caller (line 188) and the publish-failure caller (line 318) use the same method and are not forced. The wait in
registerafter a failed rejoin is not covered by a test. A different loop, a failed watch inLeaderElector.checkIfIamLeader, is the one in the 2025 reports on the ticket and is in a separate change. The two duplicate tickets, SOLR-5692 and SOLR-6285, are resolved and were not re-checked.AI-generated description. Nick Shanin directed the work and takes responsibility for this contribution.