Skip to content

[BUG] Fast-fail bound double-subtracts a NACK voter later marked FAIL, causing spurious election resets #4626

Description

@xdk-amz

Describe the bug

clusterProcessFailoverAuthNack() (introduced by #3833, f2c1c5e9d) computes the upper bound on achievable votes as:

int max_possible_acks =
    server.cluster->size -
    server.cluster->size_fail -
    server.cluster->failover_auth_nack_count;

This calculation assumes that FAIL voters and NACK voters are disjoint. They are not disjoint when voter state changes during an election.

The nodeFailed(sender) guard handles one ordering: a NACK received from a voter already marked FAIL is ignored. It does not handle the opposite ordering:

  1. A voter sends a NACK while it is reachable.
  2. The NACK increments failover_auth_nack_count.
  3. The voter is later marked FAIL during the same election.
  4. clusterUpdateState() adds it to size_fail.

The voter is now subtracted twice, once through failover_auth_nack_count and once through size_fail. A later NACK causes the underestimated bound to be evaluated and can reset an election that still has enough possible votes to reach quorum.

There is a symmetric ordering involving ACKs. If a voter ACKs and is later marked FAIL, the ACK remains valid, but size_fail still removes that voter from max_possible_acks. The current scalar ACK and NACK counters do not preserve enough per-voter state to distinguish these cases.

Receiver-side per-voter tracking would also make ACK and NACK processing idempotent. Duplicate responses are not expected in the current protocol because one AUTH_REQUEST is sent per election over the TCP cluster bus, but relying on election-local voter identity makes the bound correct independently of delivery assumptions.

To reproduce

Use a cluster with five voting primaries. The required quorum is three.

  1. Voter V1 sends a NACK. At this point V1 is not FAIL, so failover_auth_nack_count becomes 1.
  2. V1 goes down and is marked FAIL during the same election.
  3. clusterUpdateState() recomputes size_fail as 1.
  4. Voter V2 sends a NACK, making failover_auth_nack_count equal to 2.
  5. The NACK handler computes:
max_possible_acks = 5 - 1 - 2 = 2
  1. Because 2 is below the quorum of 3, the election is reset:
server.cluster->failover_auth_time = 0;

The true upper bound is still three: V3, V4, and V5 have neither failed nor NACKed and can still vote for the candidate. V1 was counted in both excluded sets, causing the false reset.

The ACK ordering has the same shape. If V1 ACKs and then becomes FAIL, its received ACK remains usable, but the current formula subtracts V1 through size_fail.

Expected behavior

Each voter should have one election-local response state, such as:

  • ACKed
  • NACKed
  • failed without having ACKed
  • still able to respond

The upper bound should include votes already received plus voters that can still ACK:

max_possible_acks = acked_count + still_possible_count;

One possible implementation is to record the election epoch in which each peer ACKed or NACKed. When processing a response, ignore it if that peer already has the same response state for the current failover_auth_epoch.

The bound can then be computed along these lines:

max_possible_acks = acked_count;

for each current voting primary V:
    if (ackedThisElection(V))
        continue;

    if (!nodeFailed(V) && !nackedThisElection(V))
        max_possible_acks++;

An ACKed voter remains included even if it later becomes FAIL. A NACKed voter that later becomes FAIL remains excluded exactly once. Duplicate responses from one voter do not change the result.

Additional information

The impact is liveness and failover latency, not election safety. ACK quorum requirements are unchanged, but a replica can abandon a winnable election and start another epoch unnecessarily. During simultaneous primary failures or cluster churn, repeated false resets can extend recovery time instead of providing the fast-fail behavior introduced by #3833.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions