fix(metastore): disable raft pipeline replication - #5518
Draft
aleks-p wants to merge 2 commits into
Draft
Conversation
On raft v1.7.3 a single failed AppendEntries can deadlock the leader's pipeline replication to one follower, permanently. The transport sizes both netPipeline channels at MaxRPCsInFlight-2, so the default of 2 leaves them unbuffered: when a follower answers Success=false, pipelineDecode returns and closes finishCh, leaving doneCh with no reader; the SEND loop then races between the closed finishCh and a ready triggerCh, and if it picks triggerCh it issues one more send that parks on inprogressCh while decodeResponses parks on doneCh. pipeline.Close is deferred inside pipelineReplicate, which is stuck in that select, so shutdownCh never closes. replicate() never returns either, so its deferred close(stopHeartbeat) never runs: heartbeats keep flowing, the follower stays a quiet Follower that never forces an election, and it is simply never replicated to again. It fails readiness on ErrLagBehind until the leader is restarted -- restarting the follower does not help, because the wedge is on the leader. The follower's own timeoutLogStore is one way to produce the triggering Success=false, since raft's appendEntries handler returns without setting Success when StoreLogs fails. Pinning MaxRPCsInFlight below raft's minInFlightForPipelining makes AppendEntriesPipeline return ErrPipelineReplicationNotSupported, which replicate() handles by staying in synchronous RPC mode. Raising it instead would only widen the window, since both channels stay bounded.
NewMetastoreSet hardcoded raft ports 10500/10502/10504. It is used from both pkg/metastore/test and pkg/segmentwriter, and go test runs packages in parallel, so the two could bind the same ports and fail with "address already in use". That was rare while every caller was short-lived. TestRaftPipelineDeadlock holds the ports long enough to make it reliable, so it surfaced as a consistent CI failure in pkg/segmentwriter rather than an occasional flake.
aleks-p
force-pushed
the
fix/metastore-raft-pipeline-deadlock
branch
from
August 14, 2026 18:56
f781778 to
8a3ada4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A metastore replica in one of our dev cells stops being replicated to and never recovers. Only restarting the leader clears it. #4892 and #4935 helped other symptoms but not this one, and it still happens with #5307 deployed.
One theory is that it is a deadlock in hashicorp/raft's pipeline replication.
Technical details
The transport sizes both `netPipeline` channels at `MaxRPCsInFlight-2`, and we leave `MaxRPCsInFlight` at the default 2, so both are unbuffered. When a follower answers `Success=false`, `pipelineDecode` returns and closes `finishCh`, leaving `doneCh` with no reader; the SEND loop then races between the closed `finishCh` and a ready `triggerCh`, and picking `triggerCh` issues one more send that parks on `inprogressCh` while `decodeResponses` parks on `doneCh`. `pipeline.Close` is deferred inside `pipelineReplicate`, which is stuck in that select, so `shutdownCh` never closes.replicate()never returns either, so its deferredclose(stopHeartbeat)never runs. That is why the victim looks healthy from the outside: heartbeats keep arriving, so it never times out and forces an election that would have healed things — it just never receives another log entry.Our own
timeoutLogStore(#4892) is one way to produce the triggeringSuccess=false, since raft'sappendEntrieshandler returns without settingSuccesswhenStoreLogsfails. A rejectedappendEntriesat leader election does it too.The theory was confirmed on local test cluster: the leader had four pipelines but only two
pipelineDecodegoroutines, with two senders parked innetPipeline.AppendEntries- blocked 4287 and 15948 minutes, one per unready replica. The 4287-minute one matched that replica's idlerunFSMand its single log-store write timeout to within a few minutes.Notes:
TestRaftPipelineDeadlockreproduces the interlock; it fails on round 1 without the change. It deliberately asserts only that the interlock never forms, and not that the follower catches up again, because recovering from a stall that lands an abandoned log-store write also needs WIP: fix(metastore): fix follower livelock from abandoned log-store writes #5307 - without it a released follower livelocks onnon-monotonic log entriesinstead. Worth tightening once WIP: fix(metastore): fix follower livelock from abandoned log-store writes #5307 lands.cc @simonswine