#36000 validation: prefetch blocks while connecting
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36000 validation: prefetch blocks while connecting | Waiting on author | 0 (+2) | Strong | 176 + 93 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Introduces an asynchronous block read-ahead mechanism using a thread pool owned by Chainstate to prefetch upcoming blocks from disk during block connection. A sliding window retains queued reads (two blocks per reader thread, defaulting to two threads and four blocks) while previous blocks are being connected, and prefetching begins on fork branches during reorgs. The configuration option -blockfetchthreads controls the number of worker threads, with zero disabling read-ahead.
Problem: Blocks stored on disk are read and deserialized synchronously on the main thread during block connection. This disk I/O stalls the validation thread and slows down both reindexing and initial block download.
Evidence of importance:
- Author benchmarks for reindex-chainstate up to block 964,469 show a 1.53x speedup on HDD (30,017s down to 19,609s).
- Author benchmarks for reindex-chainstate up to block 968,869 show a 1.30x speedup on SSD (12,430s down to 9,562s).
- Author benchmarks for initial block download up to block 968,869 show a 1.15x speedup on SSD (19,273s down to 16,766s).
Scope: Adds a new BlockFetcher class and introduces background worker threads that read blocks from disk while cs_main is held during chain activation.
Risk of a mistake: high (validation). A concurrency bug, race condition, or thread stall in the prefetcher could deadlock or stall block connection during initial block download, reindexing, or reorgs.
Changed since the previous assessment: Updated benchmark numbers in the PR description, reflecting 1.30x on SSD reindex-chainstate and 1.15x on SSD IBD.
Reviewability
Paused · Waiting on author
Open objections from andrewtoth, arejula27 with no author reply for 14 days; review would repeat what has been asked.
Waiting on (from the thread read):
- author: early return nit from arejula27 and doc comments suggestions from andrewtoth 2026-10-10
Agreement
Strong
substantive support, no open objection (andrewtoth, andrewtoth, andrewtoth, arejula27)
Thread notes: andrewtoth confirmed multi-submit performance after re-testing and accepted keeping it, suggested two doc comment cleanups on BlockFetcher, and re-ACKed the latest commit.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| fanquake (member) | objection, maintenance | Thread start and exit logging spams debug logs until rate-limiting kicks in | fixed · fix pushed · author kept fetcher alive on Chainstate in a follow-up push | changed | yes | 2026-08-18: 'Note that this spams blockread.* thread start/exit debug logs to the point that rate-limiting kicks' Status from: 2026-08-18: l0rinc: 'I’ve pushed a fix that keeps it alive on `Chainstate`' |
| andrewtoth (member) | suggestion, approach | Prefetch the fork index sibling block while disconnecting the old tip during reorgs | fixed · fix pushed · author cleared previous followups and prefetched from first sibling during reorg disconnect | changed | yes | 2026-08-23: 'We can make this useful for reorgs here by checking if the `pindexFork` is not our current tip, then clearing the prefetch queue and start fetching from the fork index.' Status from: 2026-08-26: l0rinc: 'Before disconnecting the current tip, activation now clears followups for the previous candidate and fills the queue fro' |
| andrewtoth (member) | suggestion, approach | Use a sliding window for the queue instead of waiting until empty to refill | fixed · fix pushed · author implemented sliding queue refill in FillQueue | changed | yes | 2026-08-24: 'I think this method might be more efficient as a sliding window rather than just adding all pending tasks only when the validation thread has emptied the queue.' Status from: 2026-08-25: l0rinc: 'FillQueue() (renamed) now starts from the number of queued followups and fills every missing slot.' |
| andrewtoth (member) | suggestion, approach | Extract BlockFetcher into src/node/blockfetcher.h to allow index reuse | fixed · fix pushed · author moved BlockFetcher to src/node/blockfetcher.{h,cpp} | changed | yes | 2026-08-28: 'What if we moved this out into a header in `src/node/blockfetcher.h`, and allowed passing the thread name in the constructor?' Status from: 2026-08-28: l0rinc: 'I moved `BlockFetcher` into `src/node/blockfetcher.{h,cpp}` in this push' |
| andrewtoth (member) | suggestion, approach | Use multi-Submit overload when submitting tasks to wake threads at once | fixed · fix pushed · author batched missing read submissions | changed | yes | 2026-09-03: 'maybe we can take advantage of the multi-`Submit` overload whenever we have an empty `m_followups`?' Status from: 2026-09-29: l0rinc: 'missing reads now use it, including refills of a partly drained queue.' |
| andrewtoth (member) | suggestion, interface | Add a configuration option or toggle to disable read-ahead for low-memory systems | fixed · fix pushed · author added -blockreadahead=<n> setting where 0 disables it | changed | yes | 2026-09-03: 'we might want to allow disabling this mechanism for low memory systems. Maybe worth it to just be an on/off toggle (that defaults to on)?' Status from: 2026-09-26: l0rinc: 'added `-blockreadahead=<n>` with a default of 2 readers and a queue capacity of `2*n` blocks. Setting it to `0` disables' |
| andrewtoth (member) | suggestion, maintenance | Update stale release notes to mention multiple worker threads | fixed · fix pushed · author updated release notes | answered | yes | 2026-09-08: 'The release notes are stale. There are now multiple threads.' Status from: 2026-09-24: l0rinc: 'the release note says “threads,”' |
| w0xlt (contributor) | objection, correctness | Prefetched blocks for an invalid branch stay in memory indefinitely | fixed · fix pushed · author cleared prefetched descendants for invalid blocks | changed | yes | 2026-09-16: 'When a block was rejected, blocks read ahead for its branch could remain in memory indefinitely.' Status from: 2026-09-24: l0rinc: 'an invalid block clears prefetched descendants' |
| andrewtoth (member) | suggestion, safety | Bound the configuration argument to a maximum value | fixed · fix pushed · author capped readers at 16 per chainstate | answered | yes | 2026-09-28: 'We should bound this to a max value, no?' Status from: 2026-09-29: l0rinc: 'Pushed a cap of 16 readers per chainstate.' |
| andrewtoth (member) | suggestion, maintenance | Add the setting to the reduce memory usage documentation | fixed · fix pushed · author updated docs to cover cap and queue capacity | answered | yes | 2026-09-28: 'Should we also add this to the reduce memory usage docs?' Status from: 2026-09-29: l0rinc: 'It was already there, but I updated it in the latest push to cover the cap and queue capacity.' |
| andrewtoth (member) | suggestion, maintenance | Remove line that appears to be a no-op | fixed · fix pushed · author removed the redundant line as FillQueue handles cleanup | answered | yes | 2026-09-28: 'I don't see the purpose of this line. Seems like it will always be a no-op. I think it should be removed.' Status from: 2026-09-29: l0rinc: 'FillQueue now handles cleanup when the read-ahead range ends, so it isn't needed anymore.' |
| andrewtoth (member) | suggestion, approach | Pass read_ahead_tip instead of time_point to ConnectTip to avoid adjusting timing variables | fixed · fix pushed · author moved loading and refilling into ConnectTip to preserve timing variables | answered | yes | 2026-09-28: 'Instead of passing a `time_point`, could we pass `const CBlockIndex* read_ahead_tip`?' Status from: 2026-09-29: l0rinc: 'Loading and refilling now happen inside `ConnectTip`, keeping the existing timing variables.' |
| w0xlt (contributor) | suggestion, maintenance | Disable block read-ahead in feature_proxy.py to avoid spinning up unused thread pools | fixed · fix pushed · author added read-ahead disabling to feature_proxy.py | answered | yes | 2026-09-29: 'With this PR, util.py sets blockreadahead=1 for every node, so each node now starts a reader thread that this test never uses.' Status from: 2026-09-29: l0rinc: 'added it to the memory savings options in the mentioned test.' |
| w0xlt (contributor) | suggestion, interface | Rename -blockreadahead to -blockreadaheadthreads to match -prevoutfetchthreads and clarify it sets thread count | fixed · fix pushed · author renamed the configuration option to -blockreadaheadthreads | answered | yes | 2026-09-29: 'would -blockreadaheadthreads be clearer, matching -prevoutfetchthreads?' Status from: 2026-09-29: l0rinc: 'I renamed it to be consistent with the prevout fetcher' |
| w0xlt (contributor) | suggestion, maintenance | Use BOOST_REQUIRE instead of Assert so test failures report cleanly and do not abort test_bitcoin prematurely | answered · author explained preferring Assert as a simple precondition check | answered | yes | 2026-09-29: 'Assert() aborts here instead of throwing, so if the retry path regresses, test_bitcoin dies with no Boost report and the remaining blockmanager_tests cases don't run.' Status from: 2026-09-29: l0rinc: 'I consider the loaded block a precondition for the hash comparison, so I used Assert() deliberately to keep it a simple ' |
| andrewtoth (member) | question, maintenance | Why is the assertion repeated in feature_reindex.py? | answered · author explained it verifies restoration after invalidate/reconsider | answered | yes | 2026-10-09: 'Why do we repeat this line here? Seems like a redundant assertion that can be removed?' Status from: 2026-10-10: l0rinc: 'This checks restoration after invalidate/reconsider, which the earlier assertion doesn't cover.' |
| andrewtoth (member) | suggestion, maintenance | Remove redundant blockcount assertion in feature_reindex.py | fixed · fix pushed · author removed the assertion and variable | answered | yes | 2026-10-10: 'I think we can remove this line and `blockcount = node.getblockcount()` above.' Status from: 2026-10-10: l0rinc: 'Thanks, removed the height assertion and its saved value' |
| andrewtoth (member) | suggestion, approach | Consider reverting to single task submission if multi-submit is slower and more complex | accepted · andrewtoth confirmed multi-submit was faster after re-running benchmarks | answered | yes | 2026-10-10: 'Maybe it is worth reverting to single task submission, which would make this a bit simpler?' Status from: 2026-10-18: andrewtoth: 'it seems the slowdown was an anomaly. I got 1h 42m and 1h 44m, both faster than pushing single tasks.' |
| arejula27 (contributor) | suggestion, approach | Return early when tasks is empty | open | answered | no | 2026-10-10: 'nit: when tasks is empty, could we return early here?' |
| andrewtoth (member) | suggestion, maintenance | Remove mention of block connection and synchronous fallback from block retrieval doc comment | open | answered | no | 2026-10-18: 'I don't think we need to mention connection or falling back to synchronous reading here.' |
| andrewtoth (member) | suggestion, maintenance | Remove mention of block connection from queueing doc comment | open | answered | no | 2026-10-18: 'Same here, the block connection is an external use case for this class.' |
Support:
- andrewtoth (member) (Concept ACK, 2026-08-23): The benchmarks look promising.
- andrewtoth (member) (ACK, 2026-09-08) [tested]: Managed to see speedup in IBD benchmarks by having one local node bind to 10 ports and the other connect to all 10. I did this and reindex-chainstate with an i7 and i5 machine. Nice speedups, especial
- andrewtoth (member) (2026-09-28) [tested]: I measured reindex-chainstate again with this latest commit and it's a blistering 1h 48m.
- w0xlt (contributor) (ACK, 2026-10-09): (no reason given) [not substantive]
- arejula27 (contributor) (Concept ACK, 2026-10-10) [read]: Checked potential race conditions with block writes, pruning, and indexes and found none
- andrewtoth (member) (ACK, 2026-10-18): (no reason given) [not substantive]
Participants:
- andrewtoth (member, support, 40 statements 2026-08-18 to 2026-10-18): tested and measured speedups on IBD and reindex-chainstate, reviewed revisions, and ACKed
- fanquake (member, objection, 1 statements 2026-08-18 to 2026-08-18): reported thread start/exit logs spamming until rate-limited
- w0xlt (contributor, support, 5 statements 2026-09-16 to 2026-10-09): ACKed after prior objection on invalid branch prefetching and test/config suggestions were resolved
- arejula27 (contributor, support, 2 statements 2026-10-10 to 2026-10-10): Concept ACK after checking concurrency and race conditions with block writes, pruning, and indexes
Files
100 lines under test/bench/ci.
- src/node/blockfetcher.cpp +70/-0
- test/functional/feature_reindex.py +53/-3
- src/node/blockfetcher.h +48/-0
- src/test/blockmanager_tests.cpp +28/-0
- src/validation.cpp +23/-5
- src/validation.h +8/-1
- doc/release-notes-36000.md +7/-0
- src/node/chainstatemanager_args.cpp +7/-0
- src/test/validation_chainstatemanager_tests.cpp +7/-0
- src/kernel/chainstatemanager_opts.h +5/-0
- test/functional/feature_proxy.py +2/-3
- doc/developer-notes.md +3/-0
- test/functional/test_framework/util.py +2/-1
- src/util/threadpool.h +1/-1
- doc/reduce-memory.md +1/-0
- src/CMakeLists.txt +1/-0
- src/init.cpp +1/-0
- src/kernel/CMakeLists.txt +1/-0
- src/test/util/setup_common.cpp +1/-0
Card
The short text the ranking pass compares across PRs.
Validation reads and deserializes blocks from disk synchronously on the main validation thread immediately before connecting them, stalling validation during initial sync and reindexing. This pull request introduces an asynchronous BlockFetcher thread pool that prefetches blocks in a sliding read-ahead window while earlier blocks are being connected, and queues sibling blocks ahead of time during reorgs. The feature is controlled by -blockfetchthreads, defaulting to two threads and four retained blocks. Author benchmarks show a 1.53x speedup on HDD and 1.30x speedup on SSD for reindex-chainstate, as well as a 1.15x speedup for initial block download. It has no open dependencies.
Data
thread read through 2026-10-18 (80 statements), head 81006fa18c
labels: Validation
Categories
Validation
P2 · speedup (ranking pass: Moved up: measured 1.15x IBD / 1.53x reindex gain is the largest well-benchmarked validation speedup in this band)
Prefetching blocks from disk significantly reduces validation thread stalls, showing measured speedups of 1.15x to 1.53x during IBD and reindexing. Multiple reviewers confirmed substantial real-world performance gains across HDD and SSD configurations without introducing race hazards in pruning or indexing.
Membership: Carries the Validation label and introduces asynchronous block read-ahead during block connection in validation.cpp and blockfetcher.cpp.
Factors: security/stability 0, bug 0, performance 2, user value 2, leverage 0