[merlimat]

Problem

AuditorElector.shutdown() runs the close of the ledger auditor manager on the elector's single thread and waits up
to 10 seconds for it. On a bookie that is not the auditor, that thread is waiting in
LedgerAuditorManager.tryToBecomeAuditor() until the bookie becomes the auditor, so the close never runs there: every
shutdown of a non-auditor elector waits the full 10 seconds (Failed to close auditor manager in 10 seconds), then
interrupts the election (Interrupted while performing auditor election, logged at ERROR) and closes the manager from
the calling thread. All the bookies but the auditor pay this whenever their auto recovery stops, e.g. at each step of
a rolling restart.

ZkLedgerAuditorManager.close() doesn't release a waiting candidate either, and tryToBecomeAuditor() returns
normally when its loop sees the manager closed, as if the bookie had been elected.

Example

// AuditorElector.shutdown(), master
try {
    submitShutdownTask().get(10, TimeUnit.SECONDS);   // queued behind the election task
    executor.shutdown();
} catch (TimeoutException e) {
    log.warn().exception(e).log("Failed to close auditor manager in 10 seconds");
    executor.shutdownNow();                           // interrupts tryToBecomeAuditor()
    shutdownTask.run();
}

// ZkLedgerAuditorManager.tryToBecomeAuditor(), master
while (!isClosed) {
    ...
    CountDownLatch latch = new CountDownLatch(1);
    if (null == zkc.exists(previousVote, event -> latch.countDown())) {
        continue;
    }
    latch.await();   // close() doesn't release it
}
// returns normally once closed: the caller starts an Auditor

Change

• AuditorElector.shutdown() closes the auditor manager first, on a separate thread, then shuts the executor down
  and waits up to 10 seconds for the election task to end (and interrupts it after that, as before). On the auditor's
  own elector the election task has already returned, so nothing changes there. Closing the manager deletes the vote,
  which waits for ZooKeeper to be reachable, so like the close queued on the elector's thread before, it gets
  10 seconds before being interrupted, and is interrupted right away when the shutdown itself is (fourth commit).

  closeAuditorManager();
  executor.shutdown();
  if (!executor.awaitTermination(10, TimeUnit.SECONDS)) {
      log.warn("Auditor election didn't stop in 10 seconds after closing the auditor manager, "
              + "interrupting");
      executor.shutdownNow();
  }

  private void closeAuditorManager() throws InterruptedException {
      Thread closer = new Thread(shutdownTask, "AuditorElector-close-" + bookieId);
      closer.setDaemon(true);
      closer.start();
      try {
          closer.join(TimeUnit.SECONDS.toMillis(10));
      } catch (InterruptedException e) {
          closer.interrupt();
          throw e;
      }
      if (closer.isAlive()) {
          log.warn("Failed to close auditor manager in 10 seconds, interrupting");
          closer.interrupt();
      }
  }

• An interrupted shutdown() stops the election, and still shuts the auditor down and closes its own BookKeeper
  client before rethrowing the InterruptedException.

• The election task doesn't start an Auditor if tryToBecomeAuditor() returns after the shutdown began, and when
  the shutdown ends the election it neither logs an error nor submits another shutdown task.

• ZkLedgerAuditorManager.close() releases a candidate waiting for the previous vote in line, and
  tryToBecomeAuditor() throws IllegalStateException("Auditor manager closed, give up to become auditor") on a
  closed manager instead of returning. Since close() can now run while the candidate creates its vote, the candidate
  checks again after creating it and deletes a vote close() might have missed (myVote is now volatile), and a vote
  that close() deleted under it no longer fails as My vote has disappeared (second commit). An elected candidate
  checks that the manager isn't closed and publishes its id in the election path under the lock close() takes
  before deleting the vote, so that a closed candidate neither overwrites the id of the next auditor nor returns as
  elected (third commit, with the separate thread and the interrupted shutdown).

Another LedgerAuditorManager whose close() doesn't release a waiting candidate keeps the old behavior: 10 seconds,
then an interrupt.

Testing

• New AuditorBookieTest.testShutdownFollowerElector: stops the elector of a bookie that isn't the auditor.
  shutdown() must return within 5 seconds, the elector's vote must be gone and the auditor unchanged. Fails on master
  (TimeoutException).
• New AuditorBookieTest.testCloseReleasesWaitingCandidate: closes a ZkLedgerAuditorManager whose
  tryToBecomeAuditor() waits behind the running electors. The call must fail with IllegalStateException within
  5 seconds, and its vote must be gone. Fails on master (TimeoutException: the call keeps waiting).
• New AuditorBookieTest.testCloseWhileCandidateIsElected: closes a ZkLedgerAuditorManager whose only candidate
  has listed the votes, its own first, but not yet published itself. The call must fail with IllegalStateException,
  and the election path must not hold the candidate's id. Fails on master and without the third commit
  (A closed candidate became the auditor).
• New AuditorBookieTest.testInterruptedShutdownStopsAuditor: shuts the auditor's elector down from an interrupted
  thread. shutdown() must throw InterruptedException after stopping the auditor, and the vote must be gone. Fails
  on master and without the third commit (Auditor still running).
• New AuditorBookieTest.testInterruptedShutdownInterruptsManagerClose: interrupts a shutdown while the auditor
  manager's close blocks, as for an unreachable ZooKeeper. shutdown() must throw InterruptedException, and the
  close must be interrupted within 5 seconds. Fails without the fourth commit (TimeoutException: the close is never
  interrupted).
• The org.apache.bookkeeper.replication tests (19 classes) plus BookKeeperAdminTest, BookieDecommissionTest,
  TestHttpService and BookieInitializationTest pass, except 4 BookieInitializationTest tests that fail locally
  on master too (they can't resolve the machine's hostname). AuditorBookieTest and AutoRecoveryMainTest passed
  3 more times after the second commit, and all of them again with the third (198 tests; the 23 classes took
  1,172 s), and AuditorBookieTest again with the fourth (10 tests). Checkstyle and SpotBugs pass.

Those 23 classes on master and with this change, one run each (other test runs were going on at the same time; the
waits removed have fixed lengths):

Class	master (s)	this change (s)
AuditorBookieTest	217.2	21.1
AuditorLedgerCheckerTest	212.1	77.6
BookieAutoRecoveryTest	136.3	68.8
BookieDecommissionTest	221.0	151.2
AuditorPeriodicCheckTest	269.9	198.8
BookKeeperAdminTest	59.3	29.3
TestAutoRecoveryAlongWithBookieServers	42.4	13.4
all 23 classes	1,777	1,169

Failed to close auditor manager in 10 seconds appears 68 times in master's logs of these classes, and never with
this change.

[Copilot]

🟡 Changes recommended

Critical shutdown and election issues can cause indefinite blocking, skip resource cleanup after interruption, and allow a closed candidate to overwrite a replacement auditor’s ID.

3 open issues

• Synchronize election success publication with candidate close · New
• Bound or isolate synchronous vote deletion during shutdown · New
• Complete election and auditor cleanup when termination is interrupted · New

What changed in this PR

This PR reduces auto-recovery shutdown delays for non-auditor bookies by releasing waiting election candidates rather than queuing cleanup behind the election task.

Changes:

• Close the auditor manager before waiting for the election executor to stop.
• Wake waiting ZooKeeper candidates on close and handle concurrent vote creation and deletion.
• Add regression tests for follower shutdown and closing a waiting candidate.

File	Description
bookkeeper-server/​src/​test/​java/​org/​apache/​bookkeeper/​replication/​AuditorBookieTest.java	Tests prompt shutdown, candidate failure, and vote cleanup.
bookkeeper-server/​src/​main/​java/​org/​apache/​bookkeeper/​replication/​AuditorElector.java	Reorders shutdown and suppresses expected shutdown-related election errors.
bookkeeper-server/​src/​main/​java/​org/​apache/​bookkeeper/​meta/​ZkLedgerAuditorManager.java	Releases waiting candidates and adds closure checks and vote cleanup.

_{Review effort: Balanced}

[codelipenghui]

Reviewed head 158e0b74122f006563390d8c3346e33754b5bc2b.

The change addresses the blocked election-queue shutdown and fits the existing architecture. Please address the P2 cleanup issue noted inline: interrupting the shutdown caller can leave the new manager-close thread uncancelled.

Validation limitations: the focused Maven test invocation stopped in an upstream module because the selected server tests did not match there. A separate compilation attempt stopped at the native dependency because no default Rust toolchain was configured. The server tests did not execute; the finding is based on source analysis, not a runtime reproduction.

[codelipenghui]

Code Review approval for commit a6fda44.

Summary:

The previous P2 is addressed: interrupted join now cancels the closer before propagating, with a latch-controlled regression test for that path. No actionable findings identified within the reviewed scope. Recommend approval of the code change; replication CI passed, but integration CI remains red and local tests were not rerun.