Skip to content

Partition commit stalls forever when a restarting replica can't repair a view-adopted suffix #4075

Description

@mattp5657

Summary

A partition backup's replication gap check accepts or drops an incoming Prepare based on
one comparison: whether its op number is exactly one past the local sequencer. It has no
way to tell "I already have this, it's a stale duplicate" apart from "my sequencer was
advanced past this during a view change, but the body never arrived." After a view change
that inherits an uncommitted op, and once anything else advances the sequencer past that
op, every future retransmission of it fails the same check, permanently. The op can never
be acked, so the commit point freezes below it, and every later write is blocked behind it,
including writes that have themselves reached full quorum. Producers get silence until
their read timeout expires. The group does not recover on its own.

This was discovered when:

cluster::fast_primary_rejoin::given_a_quic_producer_when_its_first_roster_hop_is_down_should_reach_the_partition_primary

continually failed for #3873

Reproduction

Fails roughly 70 to 90 percent of runs locally, and on all 4 nextest attempts in CI. Not
branch specific; reproduces on unrelated PRs.

Failing test:

cluster::fast_primary_rejoin::given_a_quic_producer_when_its_first_roster_hop_is_down_should_reach_the_partition_primary

1. Build the server binary first

cargo build --bin iggy-server

2. Build and run the test:

cargo test -p integration --no-run
# produces target/debug/deps/mod-<hash>

./target/debug/deps/mod-<hash> --exact \
  cluster::fast_primary_rejoin::given_a_quic_producer_when_its_first_roster_hop_is_down_should_reach_the_partition_primary

Failing runs take about 32s. Passing runs take about 10s.

3. Capture diagnostics on failure:

RUST_LOG=info,iggy=debug ./target/debug/deps/mod-<hash> --exact <test>

Per-test logs land in test_logs/cluster__fast_primary_rejoin__<testname>_<hash>/:
test_stdout.log (client SDK tracing) and server_{0,1,2}_stdout.log (servers, including
the iggy.sim VSR event trace). Note a RUST_LOG value that filters out the server's own
targets breaks harness startup; info,iggy=debug is known to work.

Failure output:

post-dead-roster-hop: an attempt outlived the whole 30s budget
(0/1 acked, 2 attempts, last error: Some(QuicError))

Evidence

Node-0 is the restarted original primary, node-1 is stopped, node-2 is the new partition
primary.

View change correctly inherits an uncommitted op, node-0 adopts it but repair stops one op
short and never widens:

view-change quorum merged; repairing up to the merged log before starting the view
  replica=2 view=2 op_head=21 commit_max=20
adopting view from StartView replica=0 old_view=2 new_view=2 op=21 commit=20
partition behind the group frontier; requesting repair from_op=1 commit_to_op=20 fetch_to_op=20 peer=2
partition journal repair complete shard=1 through_op=20

The primary retransmits the inherited op for the rest of the run; every retransmit is
rejected:

(node-2) prepare timeout: retransmitting un-acked prepares replica=2 view=2 targets=1 first_op=21
(node-0) WARN iggy.partitions.diag: op=21 sequence=22 ... dropping out-of-order prepare (gap)

The client's write reaches full quorum and still never commits:

PrepareAcked ... op=22 ack_from_replica=0 ack_count=2 quorum=2 quorum_reached=true

No OperationCommitted ever follows; at shutdown commit_max is still 20 on both survivors.
The client itself is a bystander: it reconnects to the correct new primary, sends, then logs
nothing for about 24 seconds until its read timeout fires. It never dials the stopped node.

Root cause

The backup admission check is one comparison
(core/partitions/src/iggy_partition.rs:3333-3352):

if is_backup && header.op != current_op + 1 {
    // "dropping out-of-order prepare (gap)"
    return;
}

current_op comes from the sequencer, not the journal. The check's own comment states its
limitation directly:

sequence is what separates the two shapes this line covers: a forward gap (op above the
sequencer, the hole the repair driver closes) and a retransmit of an op this replica
already sequenced. Without it they read identically.

Both shapes fail the same comparison for the same reason, and the check cannot tell them
apart. It does not fall back to the journal to disambiguate, and the comment explains why
not: on this plane the journal is memory-only and reads empty right after every restart, so
checking it here would misfire during ordinary post-restart catch-up.

This is a reasonable tradeoff on its own. It breaks because two things compound:

  1. Repair narrows its fetch window under a commit lag, which a just-restarted node
    always has
    (core/shard/src/lib.rs:8324): let fetch_to_op = if commit_lag { commit_to_op } else { head };. The adopted op's body is never requested.
  2. Once a later op is accepted, the sequencer advances past the hollow op,
    permanently.
    Prepare(22) passes the check (22 == 21+1), node-0 acks it and its
    sequencer moves to 22. Every later retransmission of op 21 now fails the identical check
    for the identical reason op 22 passed it. The window in which op 21 was still
    recoverable closes the instant anything else moves past it.

ack_quorum_reached (core/consensus/src/plane_helpers.rs:399) advances the commit point
by walking forward from commit_max and breaking at the first entry without quorum. Op 21
never gets a second ack, so the walk breaks there on every attempt, and op 22's own full
quorum is never reached. Replies are generated on commit, so the client is never answered.
With 3 configured replicas and 2 live, quorum has no spare capacity: one wedged backup
takes down the whole group.

Fix

Close the window rather than trying to fix the check's disambiguation problem. The repair
request already fetches a specific op range, not "everything": from_op = commit_min + 1,
bounded by fetch_to_op. The fix computes fetch_to_op correctly
(core/shard/src/lib.rs:8313-8324), using missing_suffix, which the code already
computed two lines above but never consulted in this branch:

let fetch_to_op = if commit_lag && !missing_suffix { commit_to_op } else { head };

The cost of widening under a commit lag, refetching the committed prefix, is already being
paid today regardless, since from_op is unconditionally commit_min + 1. This only
extends the endpoint to cover the inherited suffix, bounded by pipeline capacity. If the
body arrives before anything else can advance the sequencer past it, the gap check never
has to make a distinction it cannot reliably make.

Fixing the gap check itself would be a harder, separate problem: the obvious version,
checking the journal, is the exact thing its own comment rules out for the post-restart
case. Any fix there would need to recognize the specific adopted-but-unfilled range rather
than querying journal presence directly. Not needed here since the fix above closes the
window the gap check would otherwise have to reason about.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions