[merlimat]

Problem

SingleThreadExecutor.awaitTermination() returns the opposite of what it should, and waits forever for a timeout
under 1 ms:

// SingleThreadExecutor, master
public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedException {
    runner.join(unit.toMillis(timeout));   // 0 when timeout < 1 ms: waits until the thread ends
    return runner.isAlive();               // true while still running
}

Every OrderedExecutor is made of SingleThreadExecutors, so:

• OrderedExecutor.awaitTermination() returns false as soon as the first thread has terminated, without waiting for
  the others. BookKeeper.close() logs The mainWorkerPool did not shutdown cleanly precisely when it did, and goes
  on while the other worker threads may still be running tasks.
• OrderedExecutor.forceShutdown(), which BookieRequestProcessor uses for the bookie read, write and high priority
  thread pools, calls shutdownNow() on the threads that terminated and leaves alone the ones still running after the
  timeout.

Example

// OrderedExecutor.awaitTermination(), master
for (int i = 0; i < threads.length; i++) {
    ret = ret && threads[i].awaitTermination(timeout, unit);   // false once thread 0 has terminated
}

// OrderedExecutor.forceShutdown(), master
if (!threads[i].awaitTermination(timeout, unit)) {   // false for a terminated thread
    threads[i].shutdownNow();
}

Change

public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedException {
    // Unlike join(unit.toMillis(timeout)), doesn't wait forever for a timeout under 1 ms
    unit.timedJoin(runner, timeout);
    return !runner.isAlive();
}

TimeUnit.timedJoin() doesn't wait for a timeout of 0 or less and rounds a sub-millisecond one up to 1 ms.

Testing

• New TestSingleThreadExecutor.testAwaitTermination: after shutdown(), while a task still runs,
  awaitTermination(100 ms) must return false, then true once the task ends. Fails on master
  (Terminated while running a task).
• New TestSingleThreadExecutor.testAwaitTerminationWithTimeoutUnderOneMillisecond: while a task runs,
  awaitTermination(0, MILLISECONDS) returns false without waiting and awaitTermination(500, MICROSECONDS) after
  1 ms. On master the first call waits until the 10 s JUnit timeout.
• All 164 tests of bookkeeper-common pass. With this bookkeeper-common in the same reactor build,
  BookKeeperTest, BookKeeperCloseTest, LedgerCloseTest, BookieClientTest, TestPerChannelBookieClient,
  TestBookieRequestProcessor, ForceReadOnlyBookieTest and BookieShutdownTest pass (64 tests), and their logs no
  longer have The mainWorkerPool did not shutdown cleanly. With master's bookkeeper-common, BookKeeperTest logs
  it 67 times, BookKeeperCloseTest 20 and LedgerCloseTest 5.
• Checkstyle and SpotBugs pass.

[Copilot]

🟢 Approval recommended

No active review issues are reported for the executor termination and timeout changes.

No open issues 🎉

What changed in this PR

Corrects SingleThreadExecutor.awaitTermination(), restoring termination reporting and timeout behavior used by OrderedExecutor shutdown paths.

Changes:

• Returns true only when the worker thread has terminated.
• Uses TimeUnit.timedJoin() to avoid indefinite waits for zero or sub-millisecond timeouts.
• Adds regression tests for termination reporting and short timeouts.

File	Description
bookkeeper-common/​src/​test/​java/​org/​apache/​bookkeeper/​common/​util/​TestSingleThreadExecutor.java	Adds termination and short-timeout regression tests.
bookkeeper-common/​src/​main/​java/​org/​apache/​bookkeeper/​common/​util/​SingleThreadExecutor.java	Fixes the wait mechanism and termination return value.

_{Review effort: Balanced}

[codelipenghui]

Reviewed head ec0dfe0. No actionable findings identified within the reviewed scope. The localized fix correctly restores termination reporting and avoids unbounded waits for zero/sub-millisecond timeouts.

Validation caveats: affected-module and prerequisite test compilation passed, but the focused local test invocation stopped in a prerequisite module before executing the new tests. The existing metadata-driver integration CI failure remains unexplained.