Skip to content

[BUG] Cluster: stale FAILOVER_AUTH_NACK from an older election can be counted against a newer one, causing spurious election resets #4627

Description

@xdk-amz

Describe the bug

clusterMsgDataFailoverNack, introduced by #3833 (f2c1c5e9d), carries only a one-byte reason. It does not identify the election epoch of the AUTH_REQUEST being rejected.

The receive path currently uses the NACK sender's cluster-bus header epoch:

if (server.cluster->failover_auth_time &&
    server.cluster->failover_auth_sent &&
    clusterNodeIsVotingPrimary(sender) &&
    sender_claimed_current_epoch >= server.cluster->failover_auth_epoch) {
    clusterProcessFailoverAuthNack(sender, msg);
}

sender_claimed_current_epoch is the voter's currentEpoch when it constructs the NACK, not the epoch from the rejected request. A NACK for an older election can therefore pass this gate and be counted against a newer election.

The same comparison is used for FAILOVER_AUTH_ACK, where it is sound. Before granting a request for epoch E, a voter advances its currentEpoch to E, then constructs an ACK claiming epoch E. A stale ACK from epoch E consequently fails the >= E+1 check during the next election.

NACKs have the opposite invariant. REQ_EPOCH_OLD is generated specifically when:

request epoch < voter currentEpoch

A voter rejecting an epoch-E request may therefore construct the NACK while claiming E+1 or later. That stale NACK passes the receiver's gate during election E+1. Changing the comparison operator cannot fix this because the header epoch identifies the voter's state, not the rejected request.

failover_auth_nack_count is also a scalar with no epoch or per-voter identity. Once a stale NACK is attributed to election E+1, the same voter can later respond to the real epoch-E+1 request. It may then be counted again as a genuine NACK, or even grant an ACK while its stale NACK still reduces max_possible_acks. Either case invalidates the achievable-vote bound and can reset an election that remains winnable.

To reproduce

  1. Replica R starts an election in epoch E and broadcasts AUTH_REQUEST.
  2. Delivery or processing of R's request is delayed on its cluster-bus link to voter V.
  3. Responses from other voters cause R's epoch-E election to fast-fail. R starts election E+1 and sends a new AUTH_REQUEST.
  4. Before processing R's delayed epoch-E request, V advances its currentEpoch to E+1, for example by processing another replica's epoch-E+1 election request.
  5. V processes R's old request. Since E < E+1, it replies with REQ_EPOCH_OLD. The NACK header claims V's current epoch, E+1.
  6. R receives the stale NACK while its epoch-E+1 election is active. The check sender_claimed_current_epoch >= failover_auth_epoch passes, so failover_auth_nack_count is incremented for an election V has not yet rejected.
  7. V can subsequently process R's actual epoch-E+1 request and produce another response, allowing one voter to affect the election tally twice.

The independent cluster-bus links make this possible without packet reordering within a TCP connection: R can receive enough responses from other voters to advance its election while its request to V remains delayed.

Expected behavior

A NACK should only be counted against the election whose AUTH_REQUEST it rejected.

Echo the rejected request's epoch in the NACK payload using an explicit wire layout:

typedef struct {
    uint64_t epoch;      /* Epoch from the rejected AUTH_REQUEST. */
    uint8_t reason;
    uint8_t reserved[7];
} clusterMsgDataFailoverNack;

Change clusterSendFailoverNack to accept the rejected request epoch. All current call sites are inside clusterSendFailoverAuthIfNeeded, where requestCurrentEpoch is already available.

The sender should encode it in network byte order:

nack->epoch = htonu64(requestCurrentEpoch);

The receiver should retain the existing sender and active-election checks, but only count the NACK when its echoed epoch exactly matches the active election:

uint64_t nack_epoch = ntohu64(msg->data.failover_nack.nack.epoch);

if (nack_epoch == server.cluster->failover_auth_epoch) {
    clusterProcessFailoverAuthNack(sender, msg);
}

An exact epoch match also prevents normal-operation duplicate counting across elections. The failover state machine broadcasts one AUTH_REQUEST per election, so a voter can normally produce at most one NACK carrying that election's epoch. Explicit per-voter tracking could still be added as defense in depth against malformed or duplicated messages, but it is not required to fix this bug.

Regression coverage should verify that:

  1. A voter at epoch E+1 rejecting a request from epoch E echoes E.
  2. A candidate running election E+1 ignores a NACK carrying epoch E.
  3. A candidate running election E+1 counts a NACK carrying epoch E+1.

Additional information

This is a liveness issue, not a data-safety issue. ACK counting and quorum authorization are unchanged. The failure mode is a spurious election reset, causing the additional election rounds that FAILOVER_AUTH_NACK was intended to avoid.

FAILOVER_AUTH_NACK sending is already capability-gated, and the feature has not appeared in a released version, so extending the payload has no released-version compatibility cost. Pre-fix and post-fix development builds advertising the same capability bit will disagree on the NACK packet length and should not be mixed during testing.

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