nick-boss-tech opened a new pull request, #5210: URL: https://github.com/apache/solr/pull/5210
🤖 *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`](https://github.com/apache/solr/blob/b73f03976fa17b857bc0a81bc8af2c6b1c31e0bb/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContext.java#L188) 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](https://github.com/apache/solr/blob/b73f03976fa17b857bc0a81bc8af2c6b1c31e0bb/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContext.java#L494). **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](https://github.com/apache/solr/blob/1fdc74f35021063d94cc6bb41a86892876ee5abb/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContext.java#L504)). [Closing the context](https://github.com/apache/solr/blob/1fdc74f35021063d94cc6bb41a86892876ee5abb/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContext.java#L85) 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`](https://github.com/nick-boss-tech/solr/blob/64624f4726f49dc723e319f6c6aa52ba81f52189/solr/core/src/test/org/apache/solr/cloud/ShardLeaderElectionRejoinTest.java) 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](https://github.com/nick-boss-tech/solr/compare/b73f03976fa17b857bc0a81bc8af2c6b1c31e0bb...64624f4726f49dc723e319f6c6aa52ba81f52189), head 64624f4726. 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 1fdc74f3502: 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](https://github.com/apache/solr/blob/b73f03976fa17b857bc0a81bc8af2c6b1c31e0bb/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContext.java#L127)), 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.* -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
