fix(server): truncate torn segment tails instead of resurrecting them by hubcio · Pull Request #3946 · apache/iggy

hubcio · 2026-08-21T11:52:52Z

After a crash left a half-written tail on a segment, recovery found
the correct end of valid data, but reopening the segment read the
raw file length back in and overwrote that result. The garbage tail
came back to life, the next restart refused to boot, and replicas
copied the bad bytes around.

Recovery now bounds every segment without touching disk, checks the
chain, and only then cuts torn tails off the files, rebuilding the
index when damaged. Damage in the middle of a segment is never cut
out silently: the partition is refused, its files set aside, and it
is rebuilt from replicas (PartitionRecoveryRefused, replacing
PartitionChainRefused and RecoveredSegmentSizeDivergence). Nodes
already stuck on this bug can boot again. Writers verify the
expected size against the file at open and refuse on mismatch.

codecov · 2026-08-21T11:55:18Z

Codecov Report

❌ Patch coverage is 66.08187% with 116 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.61%. Comparing base (e1c8681) to head (476302a).

Files with missing lines	Patch %	Lines
core/server/src/bootstrap.rs	7.84%	47 Missing ⚠️
core/server/src/server_error.rs	0.00%	46 Missing ⚠️
.../server_common/src/segment_storage/index_writer.rs	37.50%	5 Missing ⚠️
...rver_common/src/segment_storage/messages_writer.rs	37.50%	5 Missing ⚠️
foreign/go/errors/errors_gen.go	50.00%	5 Missing ⚠️
core/configs/src/server_config/validators.rs	87.50%	2 Missing and 1 partial ⚠️
core/partitions/src/iggy_index_writer.rs	95.65%	0 Missing and 1 partial ⚠️
core/partitions/src/messages_writer.rs	95.45%	0 Missing and 1 partial ⚠️
core/server/src/partition_reconciler.rs	99.06%	1 Missing ⚠️
core/server_common/src/reactor_yield.rs	94.44%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3946      +/-   ##
============================================
- Coverage     83.84%   82.61%   -1.24%     
  Complexity     1358     1358              
============================================
  Files          1212     1213       +1     
  Lines        166737   162883    -3854     
  Branches     134304   130567    -3737     
============================================
- Hits         139807   134570    -5237     
- Misses        23293    24294    +1001     
- Partials       3637     4019     +382

Components	Coverage Δ
Rust Core	`83.20% <66.46%> (-1.43%)`	⬇️
Java SDK	`66.67% <ø> (ø)`
C# SDK	`74.99% <ø> (-1.54%)`	⬇️
Python SDK	`90.13% <ø> (ø)`
PHP SDK	`84.48% <ø> (ø)`
Node SDK	`95.80% <100.00%> (-0.11%)`	⬇️
Go SDK	`68.29% <50.00%> (-0.03%)`	⬇️

Files with missing lines	Coverage Δ
core/common/src/error/iggy_error.rs	`100.00% <ø> (ø)`
core/configs/src/server_config/message_bus.rs	`97.97% <100.00%> (+0.36%)`	⬆️
core/journal/src/file_storage.rs	`67.56% <ø> (ø)`
core/partitions/src/iggy_partitions.rs	`87.36% <100.00%> (-0.59%)`	⬇️
core/partitions/src/state_transfer.rs	`63.27% <ø> (-0.08%)`	⬇️
core/server/src/partition_helpers.rs	`75.10% <ø> (ø)`
core/server/src/segment_recovery.rs	`88.96% <ø> (+29.82%)`	⬆️
core/simulator/src/lib.rs	`97.11% <ø> (ø)`
foreign/node/src/wire/error.code.ts	`99.26% <100.00%> (+<0.01%)`	⬆️
core/partitions/src/iggy_index_writer.rs	`89.02% <95.65%> (+1.92%)`	⬆️
... and 10 more

... and 173 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

After a crash left a half-written tail on a segment, recovery found the correct end of valid data, but reopening the segment read the raw file length back in and overwrote that result. The garbage tail came back to life, the next restart refused to boot, and replicas copied the bad bytes around. Recovery now bounds every segment without touching disk, checks the chain, and only then cuts torn tails off the files, rebuilding the index when damaged. Damage in the middle of a segment is never cut out silently: the partition is refused, its files set aside, and it is rebuilt from replicas (PartitionRecoveryRefused, replacing PartitionChainRefused and RecoveredSegmentSizeDivergence). Nodes already stuck on this bug can boot again. Writers verify the expected size against the file at open and refuse on mismatch.

Review of the torn-tail recovery found paths where boot could stall or destroy data. The residue probe now refuses anything wider than one maximum message and carries a byte budget, so ordinary zero- padded data cannot stall boot; exhausting either limit refuses recovery instead of truncating. The indexed walk refuses offset discontinuities instead of underflowing the stats counter. A rebuilt index is staged and renamed instead of written in place, so a crash cannot fabricate zero-run entries. A segment with no recoverable bytes is moved aside into the .fenced.N scheme instead of deleted, and at replica count one a refused partition is tombstoned instead of silently served empty. Operator-facing messages and the config notes now match the implemented behavior.

An fsync failure left the cursor already advanced, so the persist retry appended a byte-identical index entry that boot recovery now refuses as non-monotone. Match the messages writer: fsync first, advance after, so the retry overwrites the same slot. Also drop a writer test that passed with and without the size-guard fix.

The code is a post-condition assertion that cannot cross the wire today, so it should not occupy a slot inside a retired-code gap. Allocate above the highest 4xxx code, restore the previous allocation-rule wording, and regenerate the Go and Node tables.

The residue width cap refused the widest ordinary torn tails (a torn flush chunk can span hundreds of megabytes, not one record) and read a live config knob, so lowering max_message_size later would refuse healthy partitions on every replica. The probe now has no width gate: its budget charges one unit per candidate examined, scoped to the whole partition load, keeping work linear in residue regardless of file size. A frozen 256 MiB ceiling bounds max_message_size (new config validator) and every decoded batch header, so a bit-flipped length field can no longer allocate a segment-sized buffer on the boot path. The indexed walk refuses only backward offsets: a forward gap is minted by boot itself when the durable frontier passes the recovered end, so it is absorbed with a warning instead of refusing byte-clean data. The reactor yield now actually suspends: a fixed tiny sleep lost a machine- dependent race against the timer wheel's clock re-read, so a shared helper retries with growing durations until a timer registers, making the first poll suspend by construction.

The tombstone planted for a refused partition at replica count one did not hold. The reconciler consulted the tombstone only for already-routed namespaces, so the next pass rebuilt the partition fresh and clients hung on a routed-but-tombstoned namespace. And because quarantine moved the segment files but not the superblock, the following boot re-seeded an empty partition with no refusal logged at all. The reconciler now skips tombstoned namespaces outright and InsertOwned refuses to route one; the tombstone lifts only through ConfirmRemove, proof the deletion completed. A tombstone verdict no longer quarantines: the refused chain stays in place, so every boot re-derives and re-logs the refusal until an operator intervenes.

A .log with no .index beside it aborted the whole boot. Recovery opened the index unconditionally, and IggyIndexReader::new is a bare read-only open that folds every failure, ENOENT included, into CannotReadFile. That surfaces as a plain ServerError::Iggy, not a PartitionRecoveryRefused, so the shard builder cannot fence the one partition and take the node up without it. The pair is ordinary, not exotic: SegmentStorage::new creates the log before the index, so any crash between the two leaves exactly that shape, as does an operator restore that drops an index. The boot sweep collects every .log stem whether or not an index sits beside it, so such a segment always reaches this open. Nothing new is needed to repair it. A 0-byte index already routes to the index-less walk, which rebuilds the index from the batch headers it verifies. Stat the index through the NotFound-lenient file_len first and skip the reader when the length is zero, so an absent index takes that same path. Every other stat failure still fails stop, which moves the unopenable-index error from CannotReadFile to CannotReadFileMetadata without changing that the boot refuses and leaves the log byte-identical.

The indexed recovery walk absorbed a forward offset gap on the header alone: a crash can legitimately stamp the restored offset frontier into the tail segment, so a gap is not proof of damage. But base_offset is covered by the batch checksum, and an upward bit flip in an unverified header wears exactly the frontier-stamp shape: the walk adopted the garbage offset, poisoning the partition's offset counter at boot, while the same flip downward refused loudly as OffsetDiscontinuity. Checksum-verify the gap-opening batch before adopting its offset; the legit frontier stamp is server-minted and checksums clean. A failing gap batch breaks the walk instead, and the damage probe already classifies what follows: a torn tail truncates, a verifying batch past it refuses as InteriorDamage. Batches that continue the chain exactly stay header-trusted as before.

The indexed walk treated the two directions of an offset mismatch asymmetrically: a regression refused without verifying (so a single downward bit flip in a tail batch tombstoned a single-replica partition) while a verified forward gap was absorbed, minting a segment whose end offset covers offsets no batch holds -- a shape state transfer refuses to send or install, and whose fabricated offsets seed current_offset under an advance-only superblock persist. Neither walk compared a batch partition_id stamp against the partition being recovered, so an intact record misdirected from another partition was adopted along with its offsets. Hoist the checksum verify above both directions: an unverified mismatch is damage and breaks to the probe (a torn tail truncates regardless of flip direction), a verified mismatch refuses as OffsetDiscontinuity in either direction, and a verified batch stamped for another partition refuses as the new ForeignBatch -- evidence is preserved, never truncated or absorbed. This matches the index-less arm policy exactly and drops the absorption. When the batch that breaks the walk is the first thing past the index anchor, run the damage probe before the IndexLogDivergence verdict so a verifying survivor refuses as InteriorDamage instead of a divergence claiming the log holds nothing. The fixture flips now target header bytes 8..16 so they trip the batch checksum on base_offset itself, not a per-message checksum. Offset arithmetic in the stats seed and the chain guard is checked/saturating so an end offset at u64::MAX cannot overflow.

The probe budget charged only candidate enumeration, which advances strictly monotonically, so it provably could not fire -- while the checksum verifies it gates were charged nothing. Candidates advance one byte at a time, so the slices claimed by neighbouring plausible headers overlap, and residue packed with them (admissible producer payload, nothing corrupted) drives total verified bytes toward residue times the 256 MiB plausibility cap: terabytes of synchronous reads on the boot path, re-inflicted on every restart once the refusal tombstones at replica_count = 1. Add a second residue-derived counter that charges every slice handed to a verify by its byte length, checked before the read so exhaustion never pays for the slice that broke it. Exhaustion still refuses (UnverifiedResidue now reports both budgets) except when the walk proved not a single batch: there refusal and no-survivor converge on the same recover-as-empty outcome -- the pair is fenced aside whole either way -- so exhaustion degrades to that instead of trading a fence for a tombstone. The probe also yielded only per window, so N spill-read verifies inside one window landed in a single un-preemptible synchronous stretch in front of the bootstrap barrier. Yield after each verify that hit the spill path; take_refilled is a take, so the inner and outer yields cannot double-fire on one read. The new verify-budget test pins the handed-byte total against the residue-derived multiple on a shape with a walked prefix -- the tripwire whose absence let the previous bound get deleted.

…cadence At the doubling cap the yield retry was a fixed point: release builds spun forever allocating and polling one-second timers with no log, while the debug_assert named the one condition that cannot trigger it (a frozen clock makes registration succeed; reaching the cap needs the clock to ADVANCE between the two reads, 32 times). An unyieldable reactor should cost throughput, not the boot: bound the loop at 32 attempts and degrade to not yielding, loudly. The poll!-based rewrite drops the hand-written Future and one allocation per attempt. The module doc also inverted its own measurement: a bare 1 us sleep WINS the registration race ~99.8% of the time even cold in debug; the retry earns its keep because the losses are silent at ~512 refills per GiB. And the attempted duration does throttle the caller roughly linearly above ~10 us, so the doc no longer claims otherwise. With the yields real (~12 us each, previously dead at sleep(ZERO)), OFFER_HASH_CHUNK_LEN at 1 MiB spent over half the offer-hash pass on yields; 4 MiB keeps the un-yielded stretch a bounded ~80 us CPU pass and matches SCAN_WINDOW_CAPACITY.

The PartitionRecoveryRefused Display told the operator boot moves the refused files into .fenced.N and rebuilds -- but since the single-replica path started leaving files in place and tombstoning, one boot logged that claim two lines away from "leaving the refused segment files in place". The error is rendered for every refusal via the recover_partition_segments error wrapper, so the wrong half always showed. Disposition is decided by the bootstrap arms that catch the error and only they know which branch ran, so the Display now states the refusal alone and the arms own the disposition text. The tombstone line also carries the refusal reason now: it is the line an operator greps to enumerate dark partitions, so it must stand on its own.

A namespace tombstoned by a boot-time damage verdict was never materialised, so it sat in neither partitions.namespaces() nor the shards table: reconcile_removals could not reach it and ConfirmRemove (the only untombstone) was never enqueued. The fence had no exit through the API, and slab-key recycling made DeleteTopic + CreateTopic on the same ids hand the fresh, empty topic a fence for bytes it never had -- with restarts re-deriving the refusal off the same files. Sweep tombstoned namespaces absent from both the map and the committed target through tear_down_owned_partition: the disk delete removes the refused files FIRST (fired only once metadata says the partition is gone, so it destroys nothing an operator kept) and only its success enqueues the ConfirmRemove that lifts the fence. A bare untombstone would leave the cause for the next boot to re-derive, with a window where a fresh build truncates the refused files. The single-replica rebuild allowlist gated on the refusal VARIANT (Hole, EmptyNonTailSegment), but neither variant proves nothing is at stake: both fire over fully populated chains, which the rebuild then serves empty -- the exact loss-hiding the tombstone exists to surface. The chain-shape refusals now carry the planned chain's walked byte total, and the allowlist rebuilds only when it is zero; anything else tombstones until the operator-driven exit above.

MAX_MESSAGE_SIZE_UPPER_BYTES was documented as the bound on the widest batch record any admission path can persist, but only the bus-framed paths were validated against it: the HTTP produce handler builds its bus message in-process, so framing's max_message_size check never runs there and http.max_request_size (env-settable, never validated) was the only bound. Past ~342 MiB with multi- message batches, a legally admitted, checksum-valid record exceeds what boot-time recovery treats as plausible: the walk truncates the tail silently or refuses, and at replica_count = 1 the refusal tombstones. Validate http.max_request_size against the ceiling when HTTP is enabled (a produce request carries at most one batch and base64 leaves ~25% slack, so bounding the body bounds the record), and run message_bus.validate() before the artifact-floor check so an operator raising max_message_size past the ceiling meets the real error first instead of being sent to raise transfer_artifact_bytes_max and finding the ceiling one boot later. Document at the knobs what was previously discoverable only from code: the frozen ceiling and why it exists, the missing upgrade path for data produced above it, the Go SDK hard 64 MiB frame constant, and that an HTTP body above the bus cap can admit a batch no peer accepts a replication frame for.

The recovery paragraph in config.toml -- the only operator-facing description of a destructive boot path -- described behaviour the code no longer has: it claimed unclassifiable residue always refuses (the width gate is gone and the verify budget now decides), that an intact-index walk trusts headers unconditionally (contradicting batches checksum-verify), that a damaged index is rebuilt (only a LOST one is; damaged floors or refuses), and that refused files always stay in place at replica_count = 1 (the StorageSizeMismatch tombstone lasts one process, and zero-recoverable-byte chains rebuild). Rewrite it against the current head: the lost-index rebuild triggers and their full-rescan boot cost, the checksum gate on walk contradictions in both directions plus the partition_id check, the verify-budget refusal and its recover-empty convergence when nothing walked, the two tombstone exits (zero-byte rebuild, operator delete), and the second .fenced.N producer under a partition that keeps serving -- the one outcome that loses data while the partition stays up, previously taught backwards.

github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Aug 21, 2026