Repository navigation
fix(wal): keep a committed log segment when the manifest CAS answers 412 - #11
Conversation
yordis
commented
Oct 23, 2026
yordis
commented
- A CAS that landed but answered 412 deleted the committed log segment, leaving a manifest that points at a missing log.
- Brings in Keep a 412'd log segment unless the manifest proves it uncommitted tobi/walgit#104 by jilio (building on Don't delete a committed log segment when the manifest CAS answers 412 tobi/walgit#103 by 0bserver07), cherry-picked with original authorship; the upstream test fails on our main and passes with this change.
PR SummaryHigh Risk Overview Publishing now re-reads the manifest via The fault-injection layer gains Reviewed by Cursor Bugbot for commit 5afc103. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 58 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The S3 SDK retries failed calls three times by default and the GCS client retries uploads that carry a precondition, the manifest CAS included. When a CAS lands and its reply is lost, the retry gets a 412 from our own write. process_batch, publish_compact_classified and publish_settings_impl all read any 412 as a lost race and delete their log segment, which the committed manifest now lists: the writer reports a failure for a durable change and every reader then fails on the missing segment. Run the existing cas_landed check on a 412 too before dropping the segment, and keep an unknown outcome unknown when that check fails. The fault store gains the missing fault kind (applied, then 412), sharing the existing CAS roll so seeded simulations keep their sequences, and a test drives a push, a compaction and a settings change through it. One extra manifest GET on the 412 path only. (cherry picked from commit 9fb0223) Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
A 412 on the manifest CAS can answer a transport retry of our own landed
write. The previous commit looks before dropping the segment, but treats
"not listed" as "never committed", and the pack seal still drops on any
412:
- seal_pack_replacement answered a landed 412 by deleting the segment the
committed manifest lists: the same corruption, on the maintainer's
retirement path.
- A checkpoint that folds the log at or past our seq drops our segment
descriptor. If one lands between our CAS and the read-back, "not listed"
proves nothing: the writer deleted its committed (folded) segment and
re-verified its own push into a conflict against its own commit.
slot_evidence now classifies the fresh manifest as Committed (our exact
segment is listed), Unlisted (our key is not listed and nothing is folded
at or past our seq, so no committed manifest ever listed it; with a 412
no copy of our request can land later) or Unknown (folded, our key listed
with other contents, or no manifest). Every 412 path - push, compact,
settings and seal - drops the segment only on Unlisted; Unknown keeps it
and reports CommitUnknown. The non-412 paths are unchanged (cas_landed).
Round trips: unchanged. The 412 path still costs one manifest GET (plus
one log GET when our descriptor is listed); successful publishes do not
change.
Tests: a unit test for every evidence case; a_landed_seal_answered_412_
keeps_the_commit (fails before: "manifest lists log/...2.pb but it is
gone"); a_landed_cas_folded_before_the_412_is_resolved_stays_unknown
(fails before: the push is answered Conflict{expected: (new), actual:
<its own commit>}); and a sim liveness scenario,
liveness_after_a_landed_cas_answered_412.
(cherry picked from commit 69ba1b2)
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
fdc6d25 to
5afc103
Compare
