[merlimat]

Motivation

LeaderElectionImpl.handlePathNotification() ignores the notifications of the lock unless the election has
settled (LeaderIsPresent). But an election cycle reads the lock, and may write it, while it's
ElectionInProgress. When the store notifies the deletion of the lock before it delivers the result of a read or
a write that came before the deletion, the election drops the deletion and settles on what's gone:

• A candidate that read the lock of a leader that then goes away follows that leader. It keeps reporting it from
  getLeaderValue(), and nothing makes it elect again: Created notifications are ignored, elect() returns the
  current state once it isn't NoLeader, and only a Reconnected or SessionReestablished event revalidates.
  If no other participant takes the lock, nobody leads.
• An election that wrote the lock, which is then deleted, leads without it. Another participant can then take the
  lock and lead at the same time.

These elections pick the load manager leader and the extensible load manager's channel owner
(LeaderElectionService), the scalable topic controllers, the V5 transaction coordinators, and BookKeeper's
auditor with Pulsar's metadata drivers (PulsarLedgerAuditorManager).

Which stores are exposed:

• ZooKeeper isn't. ZKMetadataStore completes the read or the write, and dispatches the watch event, on its single
  event thread, in the order ZooKeeper delivered them. The election settles before it handles the deletion, and
  then re-elects.
• Oxia is. OxiaMetadataStore completes reads and writes on the Oxia client's threads, while
  AbstractMetadataStore.receivedNotification() dispatches the notifications, which come on a separate stream, on
  the event thread. OxiaMetadataStore emits no session events either ([fix][metadata] Deliver the session events on the oxia metadata store #26712 adds them), so nothing recovers an
  election that missed a deletion.
• The memory and RocksDB stores complete the read inline, under the election's monitor, so there's no window.

For example, on Oxia, broker B starts its election while broker A, the leader, shuts down in a rolling restart:

1. B's elect("B") sets ElectionInProgress and reads the lock, which still holds "A".

2. A's close() deletes the lock.

3. B's store dispatches the Deleted notification, and B's election drops it:

   void handlePathNotification(Notification notification) {
       synchronized (this) {
           if (internalState != InternalState.LeaderIsPresent) {
               // Ignoring notification since we're not trying to become leader
               return;
           }

4. The read completes on an Oxia client thread with "A", and B settles as A's follower in
   handleExistingLeaderValue():

   // If the existing value is different, it means there's already another leader
   leaderKnown(Optional.of(existingValue));
   changeState(LeaderElectionState.Following);
   return CompletableFuture.completedFuture(LeaderElectionState.Following);

B now reports A as the leader, although the lock is gone. If B is the only other candidate, nobody leads until A
comes back or B's session is re-established. If a third broker takes the lock, B still reports A.

The new tests force that order by holding the result of the read, or of the write, until the deletion was
dispatched. On master:

candidateDoesNotFollowALeaderGoneWhileReadingTheLock[2](ZooKeeper, localhost:61728) FAILED
    org.junit.ComparisonFailure: expected:<[Lead]ing> but was:<[Follow]ing>
candidateDoesNotFollowALeaderGoneWhileReadingTheLock[6](Oxia, oxia://...) FAILED
    org.junit.ComparisonFailure: expected:<[Lead]ing> but was:<[Follow]ing>
electionDoesNotLeadWithALockDeletedWhileTakingIt FAILED
    java.lang.AssertionError: [lock of the leader]
    Expecting Optional to contain a value but it was empty.

Modifications

The election counts the deletions of the lock it's notified of, whatever its state:

if (notification.getType() == NotificationType.Deleted) {
    lockDeletions++;
}

A cycle takes the count before it reads the lock and before it writes it. If the count changed by the time the
read or the write completes, the cycle reads the lock again instead of settling:

if (lockDeletions != lockDeletionsBeforeRead) {
    // The lock may have been deleted after it was read: the leader it names may be gone
    log.debug().attr("path", path)
            .log("A deletion of the lock was notified while reading it. Reading it again.");
    return elect();
}

if (internalState == InternalState.ElectionInProgress
        && lockDeletions != lockDeletionsBeforeWrite) {
    // The lock may have been deleted once written: read it again rather than lead without it
    log.debug().attr("path", path).attr("value", value)
            .log("A deletion of the lock was notified while taking it. Reading it again.");
    FutureUtil.completeAfter(result, elect());
}

In the example, B reads the lock again, finds it empty and takes it, or follows whoever took it first. B publishes
nothing before that, so the state listener and getLeaderValue() never see the gone leader, and the future that
elect() returned completes with the final state. A deletion notified after the election settled still takes the
existing path, which re-elects.

Each cycle compares against its own count, so an overlapping cycle (say, a session revalidation during a
re-election) doesn't hide a deletion from another one. The cost is one extra read when the notified deletion
didn't affect the result: a late notification of an older deletion, or of the election's own delete when it
replaces its lock from a previous session. The latter shows on Oxia and MockZooKeeper in the LeaderElectionTest
cases that replace a lock. It didn't show on ZooKeeper, whose server sends the watch event of a client's own delete
before the reply.

#26712 makes OxiaMetadataStore emit session events, so a revalidation would eventually catch a missed deletion.
This change fixes the election itself and doesn't depend on it.

Verifying this change

• Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

• New LeaderElectionImplTest.candidateDoesNotFollowALeaderGoneWhileReadingTheLock, on ZooKeeper and Oxia. A
  leader holds the lock through one store. A candidate elects through another, and the test holds the result of
  the candidate's read of the lock until the leader closed its election and the candidate's store dispatched the
  deletion. On master the candidate follows the gone leader. With the fix it takes the leadership.
• New LeaderElectionImplTest.electionDoesNotLeadWithALockDeletedWhileTakingIt, on the memory store. The test
  holds the result of the election's write of the lock until the lock was deleted and the deletion dispatched. On
  master the election leads without the lock. With the fix it takes the lock again.
• Each test fails with its own check disabled, and passes with the other check disabled. Both passed 20 times in
  a row on each store.
• With the fix, ./gradlew quickCheck passed, and so did all 98 tests in LeaderElectionImplTest,
  LeaderElectionTest, CoordinationServiceImplTest, LeaderElectionStaleLeaderCloseTest, ZKSessionTest,
  PulsarLedgerAuditorManagerTest and AuditorBookieTest.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

• Dependencies (add or upgrade a dependency)
• The public API
• The schema
• The default values of configurations
• The threading model
• The binary protocol
• The REST endpoints
• The admin CLI options
• The metrics
• Anything that affects deployment

[codelipenghui]

Reviewed head ccad7a776c1eb935377d8811f2d81728f0f6791c.

The deletion counter addresses the reported missed-notification races, but the successful-write retry introduces a shutdown regression: closing during the new reread can leave the election's own live ephemeral lock behind and prevent takeover. Please address the ownership-cleanup issue noted inline.

Validation: the existing LeaderElectionImplTest passed all 12 reported cases at this head, including the new read/write regressions. A deterministic review-only shutdown test failed on this head and passed with the parent production implementation. The original read and write defects were also reproduced against the parent. The shutdown reproduction uses controlled notification/future ordering on the memory store; it is not a natural-timing Oxia reproduction.

[codelipenghui]

[P1] Preserve ownership cleanup across this reread

A late deletion notification for the previous lock can change the counter even though this successful put's replacement lock still exists. If close() runs while elect() awaits its reread, leaderElectionState is still NoLeader, so asyncClose() skips deletion. The delayed read then rejects Closed without cleaning up the acquired lock.

With the metadata session still alive, other candidates keep following the closed participant and cannot take over until the abandoned key is deleted or the session ends. A deterministic local test holding this reread fails on this head with an own ephemeral lock remaining after close; the identical test passes with the parent implementation.

Please retain provisional ownership across revalidation and release it on close using ownership/version checks, without prematurely publishing Leading. Add a regression for late old deletion → successful creation → pending reread → close while the underlying store remains open.

[codelipenghui]

Code Review approval for commit 8985e6e.

Summary:

The previous P1 shutdown finding is resolved in 8985e6e3be: provisional ownership now triggers ownership/version-checked cleanup on close. The new regression and retained reproducer pass with the updated source; the new regression fails against the previous implementation. No actionable findings identified within the reviewed scope.