Propagate repaired sealed tail into loglet to prevent read-stream freeze#5054
Draft
pcholakov wants to merge 1 commit into
Draft
Propagate repaired sealed tail into loglet to prevent read-stream freeze#5054pcholakov wants to merge 1 commit into
pcholakov wants to merge 1 commit into
Conversation
Follow-up to #5049. During a seal + reconfiguration race a replicated loglet can be left sealed at a stale tail. The chain then records the repaired sealed tail (check_chain's TailLsnSet), but the read stream only cached it on the LogletWrapper and never propagated it to the loglet's known_global_tail watch -- the value the background ReadStreamTask parks on. The follower stays parked in State::Reading indefinitely, applying nothing while still reporting Active, until the process is restarted. Add Loglet::notify_known_tail (default no-op; ReplicatedLoglet forwards to known_global_tail.notify, which is monotonic and cannot regress or unseal) and call it wherever the read stream learns a TailLsnSet: the Reading-state recheck (widened from the #5049 trim-only find_segment_for_lsn to the full check_chain), and the AwaitingReconfiguration / AwaitingOrSealChain arms. The parked task then wakes, drains to the sealed tail, and hands off to the next segment. Rename trim_checked_version to chain_checked_version to match its widened role. Adds a Layer-B regression test (follower_unfreezes_on_notify_known_tail) that parks a remote-sequencer ReplicatedLoglet reader and asserts notify_known_tail unparks it; reverting the override body makes it time out. Likely fixes #5033.
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.
Problem
Follow-up to #5049 (which handles recovery from a chain prefix-trim). This addresses the onset: a Bifrost replicated-loglet reader can be stranded in
State::Readingon a sealed tail it can never reach, applying nothing — with no error and no log output — until the process is restarted. The partition keeps reportingActiveand (for a leader) keeps accepting appends, so nothing obvious surfaces until traffic on that partition hangs.Root cause
During a seal + reconfiguration race, a replicated loglet can be left sealed at a stale tail (
notify_sealsets the sealed flag without advancing the offset;PeriodicTailCheckerthen exits on the sealed flag, and the local-sequencerfind_tailkeeps returning the stale watch). The backgroundReadStreamTaskgates entirely on the loglet'sknown_global_tailwatch — with a stale sealed tail it believes it is at the tail and parks forever.The chain does learn the authoritative repaired tail during reconfiguration (
check_chain→Decision::TailLsnSet), but the read stream only wrote it to aLogletWrapperfield (set_tail_lsn) — a value the underlyingReadStreamTasknever consults. So the one component that knew the true tail never told the component that was parked on the wrong one.Fix
Loglet::notify_known_tail(default no-op).ReplicatedLogletforwards it toknown_global_tail.notify(true, tail), which is monotonic — it can only advance the offset and set (never clear) the sealed bit, so it cannot regress a fresher view or unseal.TailLsnSet: theReading-state recheck (widened from Observe chain prefix-trim in Bifrost LogReadStream Reading state #5049's trim-onlyfind_segment_for_lsnto the fullcheck_chain), and theAwaitingReconfiguration/AwaitingOrSealChainarms. The parked task wakes, drains to the sealed tail, and hands off to the next segment.trim_checked_version→chain_checked_versionto match its widened role (it now gates the full chain recheck, not just trim scanning).Test
follower_unfreezes_on_notify_known_tail(in the existing single-noderun_in_test_env): builds a remote-sequencerReplicatedLogletreader (which genuinely parks — its watch startsOpen(OLDEST), noPeriodicTailChecker), asserts it is parked, then callsnotify_known_tailand asserts records drain under a bounded timeout. RevertingReplicatedLoglet::notify_known_tailto the no-op default makes the post-notify read time out — so the test pins the production line, not just a call site. Alsoreading_state_propagates_repaired_tail_to_logletpins the read-stream call sites.Verification
cargo nextest run -p restate-bifrost --all-features→ 55/55.cargo clippy -p restate-bifrost --all-features --tests -- -D warnings→ clean;cargo fmtclean.Scope / follow-up
This is the propagation fix for the case where reconfiguration completes (the chain records a repaired tail). It intentionally does not touch
PeriodicTailChecker/find_tail(the local-sequencer branch that trusts a frozenSealed(stale)watch), nor add a bounded Reading-state safety-net timer — those cover the narrower case where reconfiguration never records a tail, and are tracked as a separate defense-in-depth follow-up.Likely fixes #5033 (validated against the reporter's logs: all affected partitions had a completed chain reconfiguration whose repaired tail the reader never learned).