feat(partitions): persist log and index concurrently, harden recovery - #3970
Merged
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3970 +/- ##
============================================
+ Coverage 84.76% 84.90% +0.13%
Complexity 1405 1405
============================================
Files 1224 1224
Lines 177971 179301 +1330
Branches 144285 145618 +1333
============================================
+ Hits 150854 152232 +1378
+ Misses 23100 23024 -76
- Partials 4017 4045 +28
🚀 New features to boost your workflow:
|
hubcio
force-pushed
the
parallel-log-index-fsync
branch
from
August 28, 2026 06:38
6fb6cbb to
2d6081e
Compare
hubcio
changed the title
hubcio
force-pushed
the
parallel-log-index-fsync
branch
2 times, most recently
from
August 28, 2026 15:59
bd76371 to
a1dac8b
Compare
hubcio
changed the title
numinnex
reviewed
Aug 28, 2026
numinnex
reviewed
Aug 28, 2026
numinnex
reviewed
Aug 28, 2026
hubcio
force-pushed
the
parallel-log-index-fsync
branch
from
August 29, 2026 16:53
a9b39c7 to
44230dc
Compare
Every journal flush issued two serialized fdatasyncs, log then index, so an fsync-gated topic paid two device round trips per ack. Both files now persist under one futures::future::join: 13.4k -> 26.1k msg/s and p50 5.83 -> 3.01 ms on ext4 with enforce_fsync and messages_required_to_save = 1, 8 producers into one partition. The cursors advance only after both saves succeed, so a failed half keeps its slot and the retry overwrites it; MessagesWriter::rewind lost its last caller and is gone. Recovery had to change first. The old order was a barrier only under enforce_fsync = true: with the shipped default the page cache already wrote the two inodes back in any order, and recovery's own pass C truncates the log before the index, so an index holding entries the log does not back was already ordinary. Boot refused it as IndexLogDivergence, and on a single-replica node that refusal was a permanent tombstone for a partition whose log was healthy. An index entry is derived from the log, so it is never evidence the log lacks. Recovery now steps back to the highest entry the log proves and floors the index there; when no entry proves, or the entries contradict each other (out of order, or below the segment start), it drops the index and rebuilds from the log. Nothing a client was promised is lost, since no ack precedes persist, and the walk still runs its residue probe, so a survivor past damage refuses as before. That walk now checksums every batch it accepts, because unordered files mean a durable index entry no longer implies a durable chunk and one chunk can hold several batches. The step-back is async, yields on refills and charges the shared probe budget, so a crafted tail refuses as UnverifiedResidue instead of stalling boot; FileScanner::slice_at anchors its backward moves to keep refills linear. An integration test covers the shape: a three-node cluster, one node SIGKILLed after an eager flush, its log cut behind the last index entry, then restarted. It asserts the floor marker in that node's stdout, the exact floored index length while the log is still exactly the cut length, the surviving prefix served and the node caught up. It is red before the recovery change and green with it. Measuring this needed three benchmark fixes. Reports took min, max and std_dev from the per-bucket moving average series, which averages the extremes away while the percentiles beside them used the raw samples; all three now come from those same sorted samples. The balanced-producer kind refused fewer than two partitions, but one is a legal target and the shape a durability benchmark needs. And --enforce-fsync claimed to fsync every write while the topic's flush threshold (server default 1024) still governed when the journal reached disk, so acks were not durability-gated unless it was also 1; that threshold is now exposed as --messages-required-to-save, plumbed into topic creation, named in the fsync help and shown in an example.
A sealed segment clones a cached descriptor on a poll, but the active segment had no read-state slot and reopened its log file on every single poll. Give it a slot too, filled on the first read miss and dropped when the segment seals. The active descriptor stays outside the sealed LRU. That cache is keyed by start offset and budgeted at twelve entries, so admitting the active segment would let unrelated sealed traffic evict it. Its lifetime is bound instead to the two sealed-ness transitions, rotation and the state-transfer un-seal. Only the descriptor is cached. The sparse index and the offset memo stay sealed-only because they assume an immutable file, and the active segment grows under the reader. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The legacy-report backfill tested min, max and standard deviation independently, so a run whose samples are all equal had its genuine zero standard deviation refilled from the per-bucket moving average. That put back the averaged-away extremes the raw-sample summary exists to avoid. Reports written before the three fields existed receive them from serde's default together, so all three reading zero is what identifies such a report. Key the backfill on that instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Neither save_indexes nor save_frozen_batches has a caller outside the crate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The anchor search keyed its yield on the scan window refilling. An index entry whose position is past the log end is rejected on arithmetic alone and reads nothing from it, so it never triggers a refill, and an index floored back to a short log is mostly those entries. The search therefore held the shard core, signal handling included, for one pread per entry across the whole file. Yield every ANCHOR_SEARCH_YIELD_STRIDE entries as well, and lift the refill check out of the inner loop so it is reached on every entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Recovery floored an index whose last entry the log could not back: it stepped down to the highest entry the log still proved and anchored a walk there. That anchor can sit above interior damage, and the residue probe only scans forward from where a walk stopped, so a torn chunk below it went unread while end_offset kept advertising the offsets over it. An index the log contradicts is no longer a description of that log, so no entry in it is a trustworthy anchor. Drop it whole and walk from byte 0 instead, rebuilding the index from the batches the walk proves: the walk reads the damage, sees the would-be anchor's own batch as a survivor past it, and refuses with every byte preserved. Under enforce_fsync the step-back depth is measured first, because there it is evidence about the log. Flushes are serialized and each fdatasyncs the whole log file, so an entry above entry N proves the log was synced through chunk N, and only the chunk in flight at the crash can strand one. A deeper gap means the log lost acknowledged bytes; rebuilding would re-mint offsets a client was already promised, so it refuses as FsyncedLogLoss. The consistency scan yields on a stride as well. It runs over the whole index on every clean boot and compares a window of entries per pread, so the per-read yield the walks rely on never fires and a megabyte-scale index held the shard core for its whole length.
hubcio
force-pushed
the
parallel-log-index-fsync
branch
from
August 29, 2026 16:54
44230dc to
a48b404
Compare
Four places asserted more than the code behind them proves. The fsync gate's comments said persistence completes before the client reply. It does not. The partition journal is in memory and is never replayed at boot, and enforce_fsync decides whether a flush fdatasyncs, never whether one happens, so at the default messages_required_to_save = 1024 up to 1023 acked messages sit unflushed and the one-entry rebuild can discard a chunk holding them. Only messages_required_to_save = 1 gates acks on fsync. The refusal itself is unaffected: an acked-but-unflushed message writes neither log bytes nor an index entry, so it can neither manufacture a step-back nor mask one, and the depth argument rests on serialized flushes rather than on reply timing. The backward anchor probe claimed a budget bounded it. That budget bounds hashed log bytes; the step count had nothing bounding it, one 24-byte pread per entry over an index that can run to megabytes. Cap it at 4096 entries. Only the first entry probed can leave the step-back inside the one-entry allowance, so past it the refusal is already decided and deeper probes only sharpen the byte it names. The refusal now carries the depth searched, so no provable entries in a capped window cannot be read as the log backing nothing. The index consistency scan rejected a first entry below the segment's start offset but accepted one above it, or at a non-zero position. Every producer mints that entry at the segment's own first batch, and the accepted path reads its timestamp as the segment's start timestamp, so a head belonging to another segment was not merely unused. Require it, and rebuild from the log when it is absent. The persist-failure comments called the panic partition-scoped. It unwinds onto the shard message pump and takes every partition on that shard with it. They also said a failed persist is re-read by the next commit, which holds only for the transfer-offer flush.
A local commit failing for an op the cluster had already committed panicked, deliberately: the replica is divergent, its commit_min never advanced, and the next advance would assert. The panic was meant to end the process. It never did. The commit path runs on the shard's message pump, and compio::runtime::spawn catches the unwind, so the process lived on with one pump dead. That pump drives every partition on its shard, so all of them stopped committing, ticking, heartbeating and replying, while the listeners and the other shards kept answering and the server looked healthy. Clients did not even get an error: once the dead shard's inbox filled, frames were dropped with a warning and a counter, and no reply was ever produced, so each request blocked until the client's own read timeout. The pump handle is only inspected during shutdown, so nothing noticed. The partition now records the fault instead of panicking, which also fences it: on_ack, commit_journal and the tick all turn into no-ops, so it cannot advance while the shard winds down. The tick reports the fault to the pump, which breaks, skips the queued drain (those frames would re-enter the commit path that just failed) and runs its final flush, the fenced partition included, since its resident prefix is cluster-committed data worth writing wherever the disk still takes it. The spawn site then flips the shared shutdown flag, so every sibling shard's watchdog drives its own graceful stop. The pump's verdict travels out through the existing drain path as ShardFatal, so the process exits non-zero. A node that stopped because it could not persist a committed op must not look to an orchestrator like a clean shutdown.
hubcio
changed the title
piotr-laserdata
approved these changes
Aug 30, 2026
spetz
approved these changes
Aug 30, 2026
numinnex
approved these changes
Aug 30, 2026
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
4 participants
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.
Concurrent log and index fsync
Every journal flush issued two serialized fdatasyncs, log then index, so an fsync-gated topic paid two device round trips per ack. Both files now persist under one
futures::future::join: 13.4k to 26.1k msg/s and p50 5.83 to 3.01 ms on ext4 withenforce_fsyncandmessages_required_to_save = 1, 8 producers into one partition. Cursors advance only after both saves succeed, so a failed half keeps its slot and the callers that survive rewrite the same positions rather than appending a duplicate.Recovery had to change first
The old ordering was a barrier only under
enforce_fsync = true. With the shipped default the page cache already wrote the two inodes back in any order, and recovery's own pass C truncates the log before the index, so an index holding entries the log does not back was already ordinary. Boot refused it asIndexLogDivergence, which on a single-replica node permanently tombstoned a partition whose log was healthy.An index entry is derived from the log, so it is never evidence about the log. Recovery now drops such an index whole and rebuilds it from a byte-0 walk rather than stepping back to the highest entry the log still proves. Stepping back is unsound: that anchor can sit above interior damage, and the residue probe only scans forward from where a walk stopped, so a hole under it goes unread while
end_offsetstill advertises the offsets over it. The byte-0 walk reads the damage and refuses with every byte preserved, and checksums every batch it accepts, since unordered files mean a durable index entry no longer implies a durable chunk.Under
enforce_fsync = truethe step-back depth is measured first, because there it is evidence about the log. Flushes are serialized and each fdatasyncs the whole log file, so an entry above entry N proves the log was synced through chunk N, and only the chunk in flight at the crash can strand one. A deeper gap refuses as the newFsyncedLogLossrather than rebuilding, which would let the partition re-mint offsets a client was already promised. The probe measuring that depth is capped, since past the cap the verdict is refusal either way.That allowance is not a promise that nothing acked is lost. The partition journal is in memory and never replayed at boot, and
enforce_fsyncdecides whether a flush fdatasyncs, never whether one happens, so at the defaultmessages_required_to_save = 1024up to 1023 acked messages sit unflushed. Onlymessages_required_to_save = 1gates every ack on its own fdatasync. The refusal is unaffected: an acked-but-unflushed message writes neither log bytes nor an index entry, so it can neither manufacture a step-back nor mask one.Called out rather than fixed: when the log does back the last index entry, the walk starts there and
[0, last.position)is not re-read at boot, where at-rest damage surfaces on the read path viavalidate_checksuminstead. Closing it would mean re-hashing every segment on every boot, since no clean-shutdown marker exists on disk.A persist failure now stops the server, not one shard
A local commit failing for an op the cluster had already committed panicked on purpose: the replica is divergent, its
commit_minnever advanced, and the next advance would assert. That panic was meant to end the process and never did. It runs on the shard's message pump, andcompio::runtime::spawncatches the unwind, so the process lived on with one pump dead. That pump drives every partition on its shard, so all of them stopped committing, ticking and replying while the listeners and other shards kept answering. Clients got no error either: frames to the dead shard were dropped once its inbox filled, so each request blocked until the client's own read timeout.The partition now records the fault instead of panicking, which also fences it, so
on_ack,commit_journaland the tick become no-ops. The tick reports it to the pump, which breaks, skips the queued tail drain (those frames re-enter the commit path that just failed) and still runs its final flush. The spawn site flips the shared shutdown flag so sibling shards stop gracefully, and the verdict leaves asShardFatal, so the process exits non-zero.Quarantining only the faulting partition was the alternative. A runtime persist failure is a device signal rather than one bad chain, and a quarantined partition cannot resume without a restart anyway.
Also in this PR
The active segment now fills the same per-segment read-state slot that sealed segments use. It was populated for sealed segments only, so a consumer following the head paid one
openatper poll, each an io_uring operation prone to an io-wq punt. It is demand-filled, dropped when the segment seals, and outside the sealed LRU budget so unrelated traffic cannot evict it.Three smaller fixes: latency extremes are backfilled only when all three of min, max and standard deviation are absent, since backfilling individually let a report mix a raw value with two derived ones. The segment writer save methods are narrowed to the crate. And both index scans yield on a stride, since each can run a megabyte-scale index with no disk read to key a yield on.
Tests and verification
The integration test SIGKILLs a node after an eager flush with its log cut at the last index entry's position, the only depth a crash can produce under
enforce_fsync, then asserts the rebuild marker in the boot log, that every rebuilt entry opens inside the truncated log, the surviving prefix served, nothing fenced, and the node caught up. Unit tests cover the refusal, the probe cap, and the commit fault fencing instead of panicking.fmt, sort and clippy clean with all features and targets. partitions 134, shard 45, server 326, and the
cluster::suite against a server binary rebuilt from the branch head, since the integration crate does not rebuild it.