Repository navigation
IBD: flush each batch's tip in the background while the next batch runs - #1070
Conversation
There was a problem hiding this comment.
Summary
The IBD overlap preserves the durability order it claims. confirm_blocks_run_with_create_pins still mutates Class C and publishes the tip, then either fsyncs inline (Tip mode) or hands flush_class_c_tip to rbtc-tip-flush (Direct). The next tip_commit taker joins that thread before it touches Class C, so batch N+1 cannot commit a new tip, strong bits, or confirmed[] while N's barrier is in flight. The barrier itself is unchanged: headers, then strong_tx, header_txs, confirmed[] last, then tip_seal. A kill therefore loses at most the in-flight batch (bodies already dequeued, annotations and index puts possibly ahead of the durable tip); open still clamps an unsealed confirmed[] suffix, a spend marker above the revalidated tip, and filter/tweak rows above that tip. Seqsigwit prune uses the tip whose flush has already been joined, not the batch still in flight.
I would request changes for one error-path hole: a failed rbtc-tip-flush spawn leaves the batch committed in RAM with neither a running barrier nor TipFlush::Failed, and a one-block retry then returns success. The happy-path crash window and the sticky fsync failure do what the PR says. Two new tests do not fail if the race they name is removed.
Issue counts by severity
- bugs: 1
- suggestions: 2
- nits: 0
|
@rearden-grok please re-review the latest commit (cdcde2e). |
There was a problem hiding this comment.
Summary
The overlap keeps the durability order it claims. lock_tip_commit joins rbtc-tip-flush before the next Class C commit, disconnect, rewind, Query::flush, flush_header_archive, flush_for_shutdown, and Drop. A failed spawn flushes inline, and a failed barrier sticks as TipFlush::Failed. The one-block short-circuit cannot skip that, because the check now runs after the join. The store races match the code: the dirty low-water resets before the generation CAS and is restored if the CAS loses, needs_sync clears before the length is read and comes back on error, and one flusher at a time per array table. Seqsigwit prune uses the pre-batch tip while that flush is still running.
The earlier change request is addressed on cdcde2ee and still holds on this head: the spawn-fail path, the flush-lock test without a trailing flush, and the disconnect/drop hold. The new comments that stay are the counterintuitive ones (clean the low-water before the CAS, clear needs_sync before reading the length, do not retry a failed fsync).
Issue counts by severity
- bugs: 0
- suggestions: 0
- nits: 0
Rebase is not required. GitHub reports this head mergeable. The four master commits since the base record the BIP34 ancestor in RAM after class_c_commit returns, and drop it on disconnect. That does not change Class C flush order.
The clean step did the generation CAS, then reset the dirty low-water mark. A set between the two kept its generation bump but lost its index, so the next flush skipped an in-prefix write. It now resets the mark first and restores it when the CAS sees the set. Shared by the array tables and strong_tx. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
IBD write batches waited on flush_class_c_tip (up to ~8 drive flushes) before the next batch could start. The barrier now runs on an rbtc-tip-flush thread while the next batch archives and checks. Every tip_commit taker joins it first (the next commit, disconnect, rewind, Query::flush, flush_for_shutdown, drop), so the on-disk order is the same. Tip mode stays inline. A failed background flush sticks: every later commit fails until restart, since a failed fsync may have dropped dirty pages. A kill can lose one more write batch, which is fetched again. ibd: perf gains write.tip_flush_wait, the commit's wait for that flush. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
flush read the length, synced, then cleared needs_sync. A write in between set the flag and lost it, so the next barrier skipped the file. The flag now clears before the length is read and comes back on error. One flusher at a time, so a second never returns before the first sync. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A body repair flushes header_txs beside the IBD tip flush. The older snapshot could land after a newer flush had marked the table clean, leaving stale bytes on disk. Flushes of one table now take turns. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the IBD tip flush in the background, pruning below the in-RAM tip could drop witness data a crash-time tip still needs. Prune below the tip before this commit while its flush runs, and below the new tip after an inline flush. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A failed spawn returned after the tip was published in RAM, with no barrier running and the state still Idle, so a one-block retry took the already-confirmed short-circuit and returned Ok. The commit now runs the barrier on its own thread; a failure there sticks like a background one. Test hooks hold the helper or fail its spawn. The disconnect and drop journey now holds the flush, so it fails when either join is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Inline in Tip mode, rbtc-tip-flush in IBD, and the tip that is durable afterwards now live in flush_tip. The commit was over the CRAP gate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contiguous heights, tip+1 (or genesis), the one-block retry of an already confirmed block, and null header fks move out of the commit, which was still over the CRAP gate. Same checks, same order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
d8fa2f2 to
878ac55
Compare
There was a problem hiding this comment.
Summary
The earlier approval on d8fa2f2 still holds. 878ac55 is that series rebased onto 0a1d040, not a new patch. git range-diff of c70b341..d8fa2f2 against 0a1d040..878ac55 is nine commits. Seven are identical. The two range-diff marks are context only: master now calls remember_connected_bip34 immediately after class_c_commit, and the confirm-write row now says a crash inside the era can leave Class A above the tip. GitHub reports this head mergeable.
That rebase does not add a second Class C flush or drop a join. confirm_blocks_run_with_create_pins still takes lock_tip_commit (which joins rbtc-tip-flush) before check_run_extends_tip, the strong bits, and confirmed[]. flush_tip is still the only barrier: inline in Tip mode, spawn_tip_flush in IBD. Disconnect, rewind (rewind_to_height_inner / try_rewind_to_height), Query::flush, flush_header_archive, flush_for_shutdown, and Drop still join. forget_bip34_ancestor_if_reorged runs inside disconnect_tip_body after that join, and the ancestor note is RAM only. The commits that landed under the rebase (BIP34 ancestor, archived same-batch parent, submitblock witness reserved value, Class A above the tip) do not open another flush_class_c_tip path. The tip-hole cover re-fetches archived bodies above the durable tip; those blocks come back through the same confirm.
Batch N+1 cannot commit, publish a tip, write strong bits, or append confirmed[] while N's barrier is in flight. The next class_c_commit joins first. A failed spawn still reaches the inline flush in spawn_tip_flush after the tip is published; success leaves TipFlush::Idle with the barrier already done, and a failure stores TipFlush::Failed. join_tip_flush puts Failed back after mem::take, so a one-block retry hits that error in lock_tip_commit before check_run_extends_tip can return the already-confirmed fk. A failed fsync stays failed until restart. failed_ibd_tip_flush_stops_commits_until_restart still fails the retry after the directory is writable again.
A kill during the overlap can leave the in-flight batch's spend annotations and filter or tweak puts above the durable tip, and the next batch's Class A bodies can be archived above that tip. Open still clamps an unsealed confirmed[] suffix with tip_seal, replays annotations down to the tip, and trims both indexes. The tip-hole cover is what fetches those archived bodies again. Seqsigwit prune still uses the height flush_tip returns for IBD, which is the pre-batch tip while that flush runs (ibd_seqsigwit_prune_follows_the_durable_tip expects 300 - keep, not the in-RAM tip).
dirty_try_clean still resets the low-water, then CASes the generation, and restores the mark if the CAS loses. TableFile::flush_between still clears needs_sync before persist_and_sync reads the length, and stores it again on error. ArrayTable::flush_dirty_between still holds flush_lock across the snapshot, so a bad-body header_txs flush cannot land an older image after the tip flush. The flush thread runs Store::flush_class_c_tip only. It does not take tip_commit, tip_flush, or the BIP34 mutex. Joiners take tip_commit and then tip_flush, and they do not hold a store flush lock while they join.
disconnect_and_drop_wait_for_the_ibd_tip_flush holds the helper and fails if disconnect or Drop returns early. ibd_commit_flushes_inline_when_its_thread_cannot_start reopens on the committed tip. set_during_flush_clean_is_flushed_next, write_during_flush_stays_pending, and concurrent_flushes_keep_the_newer_image still fail if their hooks' races come back. changelog.d/ibd-tip-flush-overlap.md and the Flush section of docs/crash-recovery.md still match this order; the concurrency row kept the helper thread and picked up master's Class A sentence.
Issue counts by severity
- bugs: 0
- suggestions: 0
- nits: 0
Summary
Each IBD write batch ends with
flush_class_c_tip, about 8 drive flushes. The write thread used to wait for it. Now the flush runs on a helper thread and the next batch starts right away. The next commit waits for that flush before it touches Class C, so the order on disk does not change.tip_commitwaits for the flush first: the next commit, disconnect, rewind,Query::flush,flush_for_shutdown, and dropping theQuery.strong_tx).header_txs).--prune-seqsigwit, pruning now follows the tip that is already on disk, not the one still being flushed.write.tip_flush_wait: how long a commit waited for the previous flush.Evidence
Signet IBD from one local node, scripts skipped, fresh datadir each run, order base, new, new, base.
class_cwent from 109 s to 0.4 s. About half came back astip_flush_wait(50 s), because the macOS flush takes longer than the next batch.failed_ibd_tip_flush_stops_commits_until_restart: a read-only store dir makes the tip seal fail. Later commits fail, still fail once the dir is writable again, and a restart opens on the last durable tip. Fails on master.disconnect_and_drop_wait_for_the_ibd_tip_flush: a test hook holds the flush; disconnect and drop only return after it is released, then reopen on the right tip.ibd_commit_flushes_inline_when_its_thread_cannot_start: a test hook fails the spawn; the commit is still on disk.set_during_flush_clean_is_flushed_next,write_during_flush_stays_pending,concurrent_flushes_keep_the_newer_image: each store race, forced with a hook. Each fails before its fix.ibd_seqsigwit_prune_follows_the_durable_tip: pruned at 112 before the fix, 12 after (300 on disk, keep 288).Merge Danger
Door: two-way
Blast Radius: IBD durability
crash-recovery.mdandconcurrency.mddescribe the new order.🤖 Generated with Claude Code