Skip to content

SOLR-6213: Run leader election rejoins on their own thread - #5210

Open
nick-boss-tech wants to merge 6 commits into
apache:mainfrom
nick-boss-tech:solr-6213-rejoin-fix
Open

nick-boss-tech wants to merge 6 commits into
apache:mainfrom
nick-boss-tech:solr-6213-rejoin-fix

Conversation

@nick-boss-tech

Copy link
Copy Markdown
Contributor

🤖 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, rejoinLeaderElection calls joinElection, which runs the leader process again. That can call rejoinLeaderElection again 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 hands joinElection to 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. A KeeperException or runtime exception from the join is logged on the rejoin thread. ZkController.unregister now 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. ShardLeaderElectionRejoinTest forces 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:

  1. Turn the nested calls into a loop inside the election. The stack stays flat, but control flow changes in the three callers and in LeaderElector.
  2. Hand each rejoin to its own thread (this PR). The callers are unchanged, and each rejoin starts with a clean stack.

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.register goes on to wait for a leader, up to leaderVoteWait plus 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 register after a failed rejoin is not covered by a test. A different loop, a failed watch in LeaderElector.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.

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
nick-boss-tech marked this pull request as ready for review October 23, 2026 19:52
@github-actions github-actions Bot added the tests label Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant