validation: prefetch blocks while connecting #36000

l0rinc commented at 10:31 PM on August 17, 2026: contributor

Problem: Connecting a block already on disk requires reading and deserializing it. Doing this on the validation thread stalls validation and slows reindexing and initial block download.

This is a follow-up to #35295, which parallelized input prevout fetching during block connection.

Fix: Add a Chainstate-owned block fetcher that reads later blocks from disk while the current block is connected. Start with synchronous 1-block read-ahead, then move reads to an eagerly started worker pool that persists across activations. Keep caller-provided blocks on the direct path and refill a sliding read-ahead window. During reorgs, start reading the first sibling while disconnecting the old tip. Retry speculative failures synchronously when the block is needed.

-blockfetchthreads=<n> selects readers per chainstate, with a default of 2 and a cap of 16. Setting it to 0 disables read-ahead. The window retains up to two blocks per reader, giving 4 by default. Measurements favor 2 readers with a window of 4 or 8 blocks, so the default uses the smaller window.

Credit: The idea comes from bitcoindev1337, who had already implemented a similar design and reported comparable results.

<details><summary>Benchmark results</summary>

---- reindex-chainstate ----

1d6656c6b0 refactor: prepare block fetcher wiring
3547915bfb doc: add block read-ahead release note

2026-09-02 | reindex-chainstate | 964469 blocks | dbcache 2000 | i7-hdd | x86_64 | Intel(R) Core(TM) i7-7700 CPU @ 3.60GHz | 8 threads | 62Gi RAM | HDD

Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=964469 -dbcache=2000 -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0 (COMMIT = 1d6656c6b024578db910f45f4a131f18d75a2ed0)
  Time (abs ≡):        30017.350 s               [User: 44516.820 s, System: 1392.820 s]

Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=964469 -dbcache=2000 -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0 (COMMIT = 3547915bfb240f280188954c7c5b60ec90145ac5)
  Time (abs ≡):        19609.006 s               [User: 46710.915 s, System: 1466.050 s]

Relative speed comparison
        1.53          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=964469 -dbcache=2000 -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0 (COMMIT = 1d6656c6b024578db910f45f4a131f18d75a2ed0)
        1.00          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=964469 -dbcache=2000 -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0 (COMMIT = 3547915bfb240f280188954c7c5b60ec90145ac5)

ed7dd7cf4e Merge bitcoin/bitcoin#34371: wallet: allow importprunedfunds for spending transactions
c5aafe2557 doc: add block read-ahead release note

2026-09-29 | reindex-chainstate | 968869 blocks | dbcache 2000 | ssd-ryzen | x86_64 | AMD Ryzen 7 3700X 8-Core Processor | 16 threads | 62Gi RAM | SSD

Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=968869 -dbcache=2000 -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0 (COMMIT = ed7dd7cf4e1561a97edf72eba29a67b14e28c717)
  Time (abs ≡):        12430.900 s               [User: 36689.125 s, System: 2521.533 s]

Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=968869 -dbcache=2000 -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0 (COMMIT = c5aafe25576d341451d679816ae0d1f8c8405c7e)
  Time (abs ≡):        9562.801 s               [User: 37275.991 s, System: 2185.462 s]

Relative speed comparison
        1.30          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=968869 -dbcache=2000 -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0 (COMMIT = ed7dd7cf4e1561a97edf72eba29a67b14e28c717)
        1.00          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=968869 -dbcache=2000 -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0 (COMMIT = c5aafe25576d341451d679816ae0d1f8c8405c7e)

---- IBD ----

ed7dd7cf4e Merge bitcoin/bitcoin#34371: wallet: allow importprunedfunds for spending transactions
7c4efd1174 doc: add block read-ahead release note

2026-09-29 | IBD | 968869 blocks | dbcache 4000 | ssd-ryzen | x86_64 | AMD Ryzen 7 3700X 8-Core Processor | 16 threads | 62Gi RAM | ext4 | SSD

Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=968869 -dbcache=4000 -blocksonly  -assumevalid=00000000000000000000748969ec33043c0e52a763c6dd5193861f559f2c72e3 -printtoconsole=0 (COMMIT = ed7dd7cf4e1561a97edf72eba29a67b14e28c717)
  Time (mean ± σ):     19273.259 s ± 504.339 s    [User: 25801.170 s, System: 3635.246 s]                                                                                                                          
  Range (min … max):   18916.638 s … 19629.881 s    2 runs                                               
                                                    
Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=968869 -dbcache=4000 -blocksonly  -assumevalid=00000000000000000000748969ec33043c0e52a763c6dd5193861f559f2c72e3 -printtoconsole=0 (COMMIT = 7c4efd1174070eba967fb0803c2080500b77ee10)
  Time (mean ± σ):     16766.243 s ± 323.458 s    [User: 25575.079 s, System: 2691.147 s]                                                                                                                          
  Range (min … max):   16537.523 s … 16994.962 s    2 runs                                               
                                                    
Relative speed comparison                           
        1.15 ±  0.04  COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=968869 -dbcache=4000 -blocksonly  -assumevalid=00000000000000000000748969ec33043c0e52a763c6dd5193861f559f2c72e3 -printtoconsole=0 (COMMIT = ed7dd7cf4e1561a97edf72eba29a67b14e28c717)
        1.00          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=968869 -dbcache=4000 -blocksonly  -assumevalid=00000000000000000000748969ec33043c0e52a763c6dd5193861f559f2c72e3 -printtoconsole=0 (COMMIT = 7c4efd1174070eba967fb0803c2080500b77ee10)

</details>

DrahtBot added the label Validation on Aug 17, 2026

DrahtBot commented at 10:31 PM on August 17, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

External sites

Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36000
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36000.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36000#author-ralph

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	arejula27
Stale ACK	andrewtoth, w0xlt

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36074 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36074.svg"> (scripted-diff: [test] Add util/check.h includes for assertions by maflcko)
#36066 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36066.svg"> (validation: Separate check-only version of ConnectBlock by optout21)
#35731 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35731.svg"> (Indexes: Harden the flush-error notification invariant by arejula27)
#35071 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35071.svg"> (Reindex: save progress to continue after interruption by pinheadmz)
#35003 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35003.svg"> (validation: improve block data I/O error handling in P2P paths by furszy)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake commented at 10:24 AM on August 18, 2026: member

Note that this spams blockread.* thread start/exit debug logs to the point that rate-limiting kicks:

2026-08-18T10:20:43Z blockread.02 thread exit
2026-08-18T10:20:43Z blockread.03 thread exit
2026-08-18T10:20:43Z blockread.01 thread exit
[*] 2026-08-18T10:20:43Z [warning] Excessive logging detected from ./util/thread.cpp:19 (TraceThread): >1048576 bytes logged during the last time window of 3600s. Suppressing logging to disk from this source location until time window resets. Console logging unaffected. Last log entry.
[*] 2026-08-18T10:20:43Z blockread.00 thread start
[*] 2026-08-18T10:20:43Z UpdateTip: new best=00000000000002376bd10d0e9734df8894b2628dce1aacb9f86169dfd890eb0f height=198324 version=0x00000001 log2_work=68.682795 tx=6990764 date='2012-09-11T15:33:52Z' progress=0.005005 cache=336.1MiB(2535259txo)

in src/validation.cpp:3411 in b2f43075c1

3406 | @@ -3357,6 +3407,9 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr<
3407 |          return Assume(false);
3408 |      }
3409 |  
3410 | +    // Persists across cs_main scopes for use by each activation step.
3411 | +    BlockFetcher fetcher{m_blockman, m_chainman.m_options.block_fetch_parallelism};

andrewtoth commented at 6:18 PM on August 18, 2026:

This is creating a new ThreadPool on each invocation of ActivateBestChain. This is what's causing the logs in #36000 (comment). The BlockFetcher or at least a shared pointer to a ThreadPool that can be passed to it should be owned by the Chainstate, so it can keep the fetcher threads alive throughout IBD.

This works fine for -reindex-chainstate, because it is one long ActivateBestChain call. But for IBD this gets called many times.

l0rinc commented at 6:38 PM on August 18, 2026:

Yes, thanks @fanquake and @andrewtoth, I also just noticed that BlockFetcher was indeed recreated on every ActivateBestChain() call: I’ve pushed a fix that keeps it alive on Chainstate, and will remeasure IBD performance. Added you both as coauthors, thanks for the tests!

I’ve also reduced the default queue size from 4 to 2, since the results so far indicate that 4 isn’t worthwhile and single threaded for now for simplicity - proper HDD measurements may change this again later: <img width="235" height="201" alt="image" src="https://github.com/user-attachments/assets/3225e3ad-a924-46bb-89b4-6124cb4862b9" />

Also removed configurability to make the patch even simpler - we can add it back later if needed.

l0rinc force-pushed on Aug 19, 2026

in src/validation.cpp:3262 in 01d895b4c1

andrewtoth commented at 2:29 PM on August 23, 2026:

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. We would then be fetching the sibling while we read the current tip synchronously in DisconnectTip.

l0rinc commented at 6:58 PM on August 26, 2026:

Thanks, implemented. Before disconnecting the current tip, activation now clears followups for the previous candidate and fills the queue from the first sibling. This overlaps that read with DisconnectTip().

in src/validation.cpp:3227 in 01d895b4c1 outdated

3222 | +        auto block{m_pending[0].get()};
3223 | +        m_pending.pop_front();
3224 | +        return block && block->GetHash() == hash ? block : nullptr;
3225 | +    }
3226 | +
3227 | +    void Prefetch(const CBlockIndex& index_most_work, int next_height) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)

andrewtoth commented at 2:30 PM on August 23, 2026:

Should we just pass in height here, and internally start at height + 1? We are adding the + 1 to callsites, so this would probably be cleaner to handle it in here instead.

    void Prefetch(const CBlockIndex& index_most_work, int height) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)

l0rinc commented at 4:57 AM on August 25, 2026:

I think next_height is clearer here because the argument is the first height to enqueue: there's no "current" height here, it doesn't make sense to give an unrelated block height to the fetcher. But the names should be consolidated (pending vs queued vs followups vs provided vs saved), I kept renaming these and they don't match anymore.

andrewtoth commented at 2:38 PM on August 23, 2026: contributor

Concept ACK.

The benchmarks look promising. I am still trying to reproduce the benchmarks locally, but my usual IBD from a single local peer does not produce any speedup. This makes sense because all blocks will be downloaded in-order from the single peer, so the next block to connect will always be the one in memory.

I am trying to simulate out-of-order IBD by having my local node bind to 10 different ports and then -connect=ing to each of the ports. I am still working on getting consistent results with this, but will report back when I get this working properly.

A reindex-chainstate should show the highest theoretical speedup, since every block will be read ahead in-order.

This will also not show any improvement for connecting new blocks to tip at steady-state, since the new block will always be the one in memory. However, I think we can modify this to improve reorgs at steady-state. We can prefetch the blocks on the new fork that will be connected, while we read the blocks to disconnect.

in src/validation.cpp:3230 in 01d895b4c1 outdated

3225 | +    }
3226 | +
3227 | +    void Prefetch(const CBlockIndex& index_most_work, int next_height) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
3228 | +    {
3229 | +        AssertLockHeld(::cs_main);
3230 | +        if (!m_pending.empty()) return;

andrewtoth commented at 6:41 PM on August 24, 2026:

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. The current way has the worker thread go idle once it hits the max queue size and waits until validation catches up, whereas we can make sure the worker thread is always either working or bumping up against the max queue size.

        while (m_pending.size() < QUEUE_SIZE) {
            auto* next{index_most_work.GetAncestor(next_height + static_cast<int>(m_pending.size()))};
            if (!next || !Enqueue(*next)) break;
        }

l0rinc commented at 4:43 AM on August 25, 2026:

Thanks, implemented. FillQueue() (renamed) now starts from the number of queued followups and fills every missing slot.

I originally waited for the queue to empty so adjacent reads would be submitted together (from the same file probably), which I thought might improve file-cache locality, but your measurements indicate that's not the case, so I'll remeasure. Instead of the while loop above (which might be infinite if m_pending doesn't get updated in the loop) I used a bounded loop.

l0rinc force-pushed on Aug 25, 2026

andrewtoth commented at 4:30 PM on August 25, 2026: contributor

While I figure out how to do out-of-order IBD, I can share reindex-chainstate benchmarks I did on my laptop. I tried variants with 2 threads and queue depth of 4, as well as the sliding-window change with queue depth of 2 and 4. The results are very impressive! The sliding-window with 2 depth had the best time.

Done on an i9-14900HX, default dbcache, stopatheight=961000.

Reindex	Time	Speedup
master	2h 33m 27s	1.00x
PR	1h 56m 19s	1.32x
sliding-window 1 thread, queue of 4	1h 52m 19s	1.37x
PR 2 threads, queue of 4	1h 51m 58s	1.37x
sliding-window 1 thread, queue of 2	1h 50m 44s	1.39x

l0rinc force-pushed on Aug 26, 2026

l0rinc commented at 7:39 PM on August 26, 2026: contributor

Thanks for the review! I took all your suggestions (rebased separately to keep the diff focused).

The provided block is already decoded and selected by ActivateBestChain(), so it now stays on that path instead of being stored in BlockFetcher. This simplified the fetcher state: it only owns followups now, while read_ahead_tip stops at the provided block's parent to avoid rereading it. Accordingly, m_pending/PopPending() became m_followups/PopFollowup(), Prefetch() became FillQueue(), and the new argument is provided_block.

Also switched to a sliding 1-worker/depth-2 queue, added sibling prefetch during reorg disconnection, fixed the final followup being skipped when no block is provided, and removed futures before calling get(). feature_reindex.py now checks exact cache-hit counts, worker reuse, submitblock results, and reorg read-ahead.

I'll experiment further with different threads and queue sizes and whether we can call the context-free checks here.

in src/validation.cpp:3195 in b5e48454d5

3190 | @@ -3201,19 +3191,92 @@ void Chainstate::PruneBlockIndexCandidates() {
3191 |      assert(!setBlockIndexCandidates.empty());
3192 |  }
3193 |  
3194 | +/** Supplies blocks to validation. Destruction waits for any queued reads. */
3195 | +class Chainstate::BlockFetcher

andrewtoth commented at 4:05 PM on August 28, 2026:

What if we moved this out into a header in src/node/blockfetcher.h, and allowed passing the thread name in the constructor? Then with a simple commit on top (and shortening index thread names)

<details><summary>Index patch</summary>

diff --git a/src/index/base.cpp b/src/index/base.cpp
index 5820448bb7..4225796fc0 100644
--- a/src/index/base.cpp
+++ b/src/index/base.cpp
@@ -11,6 +11,7 @@
 #include <interfaces/types.h>
 #include <kernel/types.h>
 #include <node/abort.h>
+#include <node/blockfetcher.h>
 #include <node/blockstorage.h>
 #include <node/context.h>
 #include <node/database_args.h>
@@ -209,6 +210,7 @@ void BaseIndex::Sync()
 {
     const CBlockIndex* pindex = m_best_block_index.load();
     if (!m_synced) {
+        node::BlockFetcher fetcher{m_chainstate->m_blockman, m_thread_name};
         auto last_log_time{NodeClock::now()};
         auto last_locator_write_time{last_log_time};
         while (true) {
@@ -249,8 +251,14 @@ void BaseIndex::Sync()
             }
             pindex = pindex_next;
 
-
-            if (!ProcessBlock(pindex)) return; // error logged internally
+            std::shared_ptr<const CBlock> loaded;
+            WITH_LOCK(::cs_main, {
+                loaded = fetcher.Load(pindex->GetBlockHash());
+                if (const auto* tip{m_chainstate->m_chain.Tip()}) {
+                    fetcher.FillQueue(*tip, pindex->nHeight + 1);
+                }
+            });
+            if (!ProcessBlock(pindex, loaded.get())) return; // error logged internally
 
             auto current_time{NodeClock::now()};
             if (current_time - last_log_time >= SYNC_LOG_INTERVAL) {

</details>

we could have parallelism for all indexes. With this patch I measured speedups of 30% for txindex, 50% for blockfilterindex, and 6% for coinstatsindex. We could improve the latter two even more with an optional prefetch of the undo data as a follow-up.

cc @furszy what do you think of this approach to parallelizing the indexing code? It would stack easily with #34489 for txospenderindex and txindex; it should be as simple as increasing the threadcount and queue depth of the blockfetcher (since each append would no longer be writing so the bottleneck would just be reading/deserializing blocks).

l0rinc commented at 6:48 PM on August 28, 2026:

Thanks, I like this approach. I moved BlockFetcher into src/node/blockfetcher.{h,cpp} in this push and added you as a co-author - although I initially forgot to add it to the Kernel build, and ReadBlock introduced a circular dependency. Since the extraction is useful independently of the index follow-up and makes the diff simpler, I also wired it into BaseIndex::Sync() locally to make sure the interface works. Your results make index prefetching worth doing, but I left the index wiring and configurable thread name out so we can finish the validation path in this PR first.

One detail for the follow-up is thread naming: ThreadPool adds a .xx suffix, so the current blkfltbscidx, coinstatsidx, and txospenderidx names would exceed the limit accepted by ThreadRename() before it adds b-. Since we just shortened these names in this release, maybe we should leave room for the worker suffix now, even if index prefetching remains a follow-up, to avoid renaming the threads again later (assuming each index will get their own ThreadPool, which would kind of defeat the purpose of handling shared resources).

And about the index parallelization work: this change is independent of batching index writes or parallelizing MuHash, so @furszy's ideas can still make sense on top. Before batching or adding more index concurrency, I think we should fix the concrete Stop()/Init() reader issue and account for pre-genesis sync and flush callbacks, stale-tip rewind after a disconnect, and blockfilter/coinstats crash-reorg recovery (I found several of these issues by repeatedly crashing during sync, and while some paths are theoretical and can wait, we should account for these states first).

There is also a separate follow-up for context-free validation: the prefetched block is owned by the worker until its future is consumed, so CheckBlock() can safely populate its memoization flags there, and I left a TODO at that point.

So I agree index block prefetching should be a follow-up, but before batching or parallelizing the indexes, I'd like to harden them to be more crash-safe.

andrewtoth commented at 5:13 PM on August 31, 2026:

I don't think there's a circular dependency. We don't need the ReadBlockFn (so we also don't need BlockFetcher to be wrapped in a std::unique_ptr):

<details><summary>Patch</summary>

diff --git a/src/node/blockfetcher.cpp b/src/node/blockfetcher.cpp
index cc8c65e182..0528622606 100644
--- a/src/node/blockfetcher.cpp
+++ b/src/node/blockfetcher.cpp
@@ -5,12 +5,11 @@
 #include <node/blockfetcher.h>
 
 #include <chain.h>
-#include <flatfile.h>
 #include <kernel/cs_main.h>
+#include <node/blockstorage.h>
 #include <primitives/block.h>
 #include <sync.h>
 #include <uint256.h>
-#include <util/expected.h>
 #include <util/threadpool.h>
 
 #include <cstddef>
@@ -31,9 +30,9 @@ std::shared_ptr<const CBlock> BlockFetcher::PopFollowup()
 bool BlockFetcher::Enqueue(const CBlockIndex& index)
 {
     if (m_pool.WorkersCount() == 0) m_pool.Start(WORKER_COUNT);
-    auto followup{m_pool.Submit([&read_block = m_read_block, hash = index.GetBlockHash(), pos = index.GetBlockPos()]() -> std::shared_ptr<const CBlock> {
+    auto followup{m_pool.Submit([&blockman = m_blockman, hash = index.GetBlockHash(), pos = index.GetBlockPos()]() -> std::shared_ptr<const CBlock> {
         auto block{std::make_shared<CBlock>()};
-        if (!read_block(*block, pos, hash)) return nullptr;
+        if (!blockman.ReadBlock(*block, pos, hash)) return nullptr;
         // TODO The block is owned by the worker until its future is consumed, so CheckBlock() may safely set its memoization flags.
         return block;
     })};
diff --git a/src/node/blockfetcher.h b/src/node/blockfetcher.h
index 88290a39af..40e489f328 100644
--- a/src/node/blockfetcher.h
+++ b/src/node/blockfetcher.h
@@ -5,33 +5,30 @@
 #ifndef BITCOIN_NODE_BLOCKFETCHER_H
 #define BITCOIN_NODE_BLOCKFETCHER_H
 
+#include <attributes.h>
 #include <kernel/cs_main.h>
 #include <sync.h>
 #include <util/threadpool.h>
 
 #include <cstdint>
 #include <deque>
-#include <functional>
 #include <future>
 #include <memory>
-#include <string>
-#include <utility>
 
 class CBlock;
 class CBlockIndex;
-struct FlatFilePos;
 class uint256;
 
 namespace node {
+class BlockManager;
+
 /** Supplies blocks to validation. Destruction waits for any queued reads. */
 class BlockFetcher
 {
-    using ReadBlockFn = std::function<bool(CBlock&, const FlatFilePos&, const uint256&)>;
-
     static constexpr uint32_t WORKER_COUNT{1};
     static constexpr uint32_t QUEUE_SIZE{2};
 
-    const ReadBlockFn m_read_block;
+    const BlockManager& m_blockman;
     ThreadPool m_pool{"blockread"};
     std::deque<std::future<std::shared_ptr<const CBlock>>> m_followups GUARDED_BY(::cs_main);
 
@@ -40,7 +37,7 @@ class BlockFetcher
     bool Enqueue(const CBlockIndex& index) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
 public:
-    explicit BlockFetcher(ReadBlockFn read_block) : m_read_block{std::move(read_block)} {}
+    explicit BlockFetcher(const BlockManager& blockman LIFETIMEBOUND) : m_blockman{blockman} {}
 
     void Clear() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
     std::shared_ptr<const CBlock> Load(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
diff --git a/src/validation.cpp b/src/validation.cpp
index abb2a612be..be006fdd0d 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -29,7 +29,6 @@
 #include <kernel/types.h>
 #include <kernel/warning.h>
 #include <logging/timer.h>
-#include <node/blockfetcher.h>
 #include <node/blockstorage.h>
 #include <node/utxo_snapshot.h>
 #include <policy/ephemeral_policy.h>
@@ -1876,17 +1875,13 @@ Chainstate::Chainstate(
     BlockManager& blockman,
     ChainstateManager& chainman,
     std::optional<uint256> from_snapshot_blockhash)
-    : m_block_fetcher{std::make_unique<node::BlockFetcher>([blockman = &blockman](CBlock& block, const FlatFilePos& pos, const uint256& hash) {
-          return blockman->ReadBlock(block, pos, hash);
-      })},
+    : m_block_fetcher{blockman},
       m_mempool(mempool),
       m_blockman(blockman),
       m_chainman(chainman),
       m_assumeutxo(from_snapshot_blockhash ? Assumeutxo::UNVALIDATED : Assumeutxo::VALIDATED),
       m_from_snapshot_blockhash(from_snapshot_blockhash) {}
 
-Chainstate::~Chainstate() = default;
-
 fs::path Chainstate::StoragePath() const
 {
     fs::path path{m_chainman.m_options.datadir / "chainstate"};
@@ -3222,8 +3217,8 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex&
     const CBlockIndex* pindexFork = m_chain.FindFork(index_most_work);
     const CBlockIndex* read_ahead_tip{provided_block ? index_most_work.pprev : &index_most_work}; // Avoid rereading the provided block
     if (pindexFork && pindexFork != pindexOldTip) {
-        m_block_fetcher->Clear();
-        if (read_ahead_tip) m_block_fetcher->FillQueue(*read_ahead_tip, pindexFork->nHeight + 1);
+        m_block_fetcher.Clear();
+        if (read_ahead_tip) m_block_fetcher.FillQueue(*read_ahead_tip, pindexFork->nHeight + 1);
     }
 
     // Disconnect active blocks which are no longer in the best chain.
@@ -3263,8 +3258,8 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex&
 
         // Connect new blocks.
         for (CBlockIndex* pindexConnect : vpindexToConnect | std::views::reverse) {
-            auto block_to_connect{provided_block && pindexConnect == &index_most_work ? provided_block : m_block_fetcher->Load(pindexConnect->GetBlockHash())};
-            if (read_ahead_tip) m_block_fetcher->FillQueue(*read_ahead_tip, pindexConnect->nHeight + 1);
+            auto block_to_connect{provided_block && pindexConnect == &index_most_work ? provided_block : m_block_fetcher.Load(pindexConnect->GetBlockHash())};
+            if (read_ahead_tip) m_block_fetcher.FillQueue(*read_ahead_tip, pindexConnect->nHeight + 1);
             if (!ConnectTip(state, pindexConnect, std::move(block_to_connect), connected_blocks, disconnectpool)) {
                 if (state.IsInvalid()) {
                     // The block violates a consensus rule.
diff --git a/src/validation.h b/src/validation.h
index 2c67d1aa32..2793ccb92b 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -18,6 +18,7 @@
 #include <kernel/chainparams.h>
 #include <kernel/chainstatemanager_opts.h>
 #include <kernel/cs_main.h> // IWYU pragma: export
+#include <node/blockfetcher.h>
 #include <node/blockstorage.h>
 #include <policy/feerate.h>
 #include <policy/packages.h>
@@ -63,7 +64,6 @@ namespace kernel {
 struct ChainstateRole;
 } // namespace kernel
 namespace node {
-class BlockFetcher;
 class SnapshotMetadata;
 } // namespace node
 namespace Consensus {
@@ -562,7 +562,7 @@ protected:
     Mutex m_chainstate_mutex;
 
     //! Reads blocks ahead during chain activation.
-    std::unique_ptr<node::BlockFetcher> m_block_fetcher;
+    node::BlockFetcher m_block_fetcher;
 
     //! Optional mempool that is kept in sync with the chain.
     //! Only the active chainstate has a mempool.
@@ -594,7 +594,6 @@ public:
         node::BlockManager& blockman,
         ChainstateManager& chainman,
         std::optional<uint256> from_snapshot_blockhash = std::nullopt);
-    ~Chainstate();
 
     //! Return path to chainstate leveldb directory.
     fs::path StoragePath() const;

</details>

l0rinc commented at 9:43 PM on September 1, 2026:

The full patch still introduces the exact cycle: node/blockfetcher -> node/blockstorage -> validation -> node/blockfetcher, can you please check it with lint-circular-dependencies.py?

andrewtoth commented at 4:56 PM on September 3, 2026:

Ahh I did not use the linter, I just compiled. Indeed, my patch fails lint. Thanks!

l0rinc force-pushed on Aug 28, 2026

DrahtBot added the label CI failed on Aug 28, 2026

DrahtBot commented at 6:06 PM on August 28, 2026: contributor

🚧 At least one of the CI tasks failed. Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/33196171834/job/98933744258 LLM reason (✨ experimental): CI failed because ctest hit a runtime symbol lookup error in test_kernel (libbitcoinkernel.so undefined symbol: node::BlockFetcher::FillQueue(...)).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

l0rinc force-pushed on Aug 28, 2026

DrahtBot removed the label CI failed on Aug 28, 2026

in doc/release-notes-36000.md:1 in aaf3283a12 outdated

   0 | @@ -0,0 +1,6 @@
   1 | +Performance Improvements

andrewtoth commented at 4:25 PM on August 31, 2026:

Does this warrant a release note? It is covered already by "performance improvements" in every release note.

l0rinc commented at 9:41 PM on September 1, 2026:

I expect this to land in a future release, so for now I'd keep it

in src/node/blockfetcher.cpp:1 in aaf3283a12

   0 | @@ -0,0 +1,60 @@
   1 | +// Copyright (c) 2026-present The Bitcoin Core developers

andrewtoth commented at 4:25 PM on August 31, 2026:

nit: we can remove the dates altogether. same for header file.

// Copyright (c) The Bitcoin Core developers

in src/node/blockfetcher.cpp:37 in aaf3283a12

  32 | +{
  33 | +    if (m_pool.WorkersCount() == 0) m_pool.Start(WORKER_COUNT);
  34 | +    auto followup{m_pool.Submit([&read_block = m_read_block, hash = index.GetBlockHash(), pos = index.GetBlockPos()]() -> std::shared_ptr<const CBlock> {
  35 | +        auto block{std::make_shared<CBlock>()};
  36 | +        if (!read_block(*block, pos, hash)) return nullptr;
  37 | +        // TODO The block is owned by the worker until its future is consumed, so CheckBlock() may safely set its memoization flags.

andrewtoth commented at 4:28 PM on August 31, 2026:

Not sure we should have this TODO here. I don't know if there is consensus for this yet. Changing the validation ordering is a different beast than just parallelizing reading a block from disk and deserializing.

in src/node/blockfetcher.h:32 in aaf3283a12

  27 | +class BlockFetcher
  28 | +{
  29 | +    using ReadBlockFn = std::function<bool(CBlock&, const FlatFilePos&, const uint256&)>;
  30 | +
  31 | +    static constexpr uint32_t WORKER_COUNT{1};
  32 | +    static constexpr uint32_t QUEUE_SIZE{2};

andrewtoth commented at 4:29 PM on August 31, 2026:

Could we pass these as default parameters to the constructor, and call ThreadPool::Start on construction? That way we can inject different values. Why do we wait to start the threadpool later? IMO it's cleaner/more RAII to start in constructor.

in src/node/blockfetcher.cpp:49 in aaf3283a12 outdated

  44 | +void BlockFetcher::Clear() { m_followups.clear(); }
  45 | +
  46 | +std::shared_ptr<const CBlock> BlockFetcher::Load(const uint256& hash)
  47 | +{
  48 | +    if (auto block{PopFollowup()}; block && block->GetHash() == hash) return block;
  49 | +    return nullptr;

andrewtoth commented at 4:31 PM on August 31, 2026:

We should call Clear here, since on a miss we have gotten out of sync of the fetching.

andrewtoth commented at 4:41 PM on August 31, 2026: contributor

Still working on out-of-order IBD benchmarks, but this is looking good. Left some minor suggestions.

l0rinc force-pushed on Sep 1, 2026

l0rinc commented at 9:53 PM on September 1, 2026: contributor

Thanks, addressed most of your concerns, rebased and updated the defaults to 2 workers and queue size of 4 (current SSD and especially HDD measurements indicate that to be the optimum).

l0rinc commented at 2:55 AM on September 2, 2026: contributor

Took about a week, but the measurements are in and 2 threads and a queue of 4 blocks (already the current values of the PR) seem to be the sweet spot:

If we add context-independent CheckBlock calls in a followup (to do partial validation before returning the blocks), the optimum shifts slightly to 4 threads and queue length of 8.

in src/node/blockfetcher.cpp:33 in 3547915bfb

  28 | +    return followup.get();
  29 | +}
  30 | +
  31 | +bool BlockFetcher::Enqueue(const CBlockIndex& index)
  32 | +{
  33 | +    auto followup{m_pool.Submit([&read_block = m_read_block, hash = index.GetBlockHash(), pos = index.GetBlockPos()]() -> std::shared_ptr<const CBlock> {

andrewtoth commented at 3:11 PM on September 3, 2026:

Since we have multiple worker threads now, maybe we can take advantage of the multi-Submit overload whenever we have an empty m_followups? That would awaken all threads at once. This would happen on every ActivateBestChainStep call, so could show some improvement?

Also, if we have multiple threads and queue depth, 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)? Or maybe a configuration of threads only and the queue depth is 2*threads? Just trying to think of ways to make it simple instead of adding all kinds of configuration options.

andrewtoth commented at 3:26 PM on September 25, 2026:

@l0rinc just wondering if you had any thoughts on these suggestions?

l0rinc commented at 1:20 AM on September 26, 2026:

I started investigating your suggestions, then got pulled into a few issues for this release and forgot to follow up here. I prototyped batching when the queue is empty, but the extra task-building code was hard to justify without a measured speedup - I can measure them later. But I did add -blockreadahead=<n> with a default of 2 readers and a queue capacity of 2*n blocks. Setting it to 0 disables read-ahead. Thanks for the reminder.

l0rinc commented at 12:25 AM on September 29, 2026:

I played with it a bit more and massaged it until I found a setup I liked. I forgot that we already have a batch overload that avoids locking and notifying for each entry - missing reads now use it, including refills of a partly drained queue. Let me know what you think, I’ll come back with the benchmarks tomorrow.

l0rinc commented at 4:51 PM on September 29, 2026:

Now that the code for multi-submit isn't more complicated than single-submit (which was the case in my original attempt), the 1% speedup is probably worth it (I should update the assumevalid in my benchmarks).

<details><summary>master vs single-submit vs multi-submit</summary>

for DBCACHE in 2000; do
  COMMITS="ed7dd7cf4e1561a97edf72eba29a67b14e28c717 7c4efd1174070eba967fb0803c2080500b77ee10 c5aafe25576d341451d679816ae0d1f8c8405c7e"
  STOP=968869; CC=gcc; CXX=g++
  BASE_DIR="/mnt/my_storage"; DATA_DIR="$BASE_DIR/BitcoinData"; LOG_DIR="$BASE_DIR/logs"
  (echo ""; for c in $COMMITS; do git fetch -q origin "$c" 2>/dev/null || true; git log -1 --pretty='%h %s' "$c" || exit 1; done) &&
  (echo "" && echo "$(date -I) | reindex-chainstate | ${STOP} blocks | dbcache ${DBCACHE} | $(hostname) | $(uname -m) | $(lscpu | grep 'Model name' | head -1 | cut -d: -f2 | xargs) | $(nproc) threads | $(free -h | awk '/^Mem:/{print $2}') RAM | $(lsblk -no ROTA $(df --output=source $BASE_DIR | tail -1) | grep -q 1 && echo HDD || echo SSD)"; echo "") &&
  hyperfine   --sort command   --runs 1   --export-json "$BASE_DIR/rdx-$(sed -E 's/([a-f0-9]{8})[a-f0-9]* ?/\1-/g;s/-$//'<<<"$COMMITS")-$STOP-$DBCACHE-$CC.json"   --parameter-list COMMIT ${COMMITS// /,}   --prepare "killall -9 bitcoind 2>/dev/null; rm -f ./build/bin/bitcoind; git clean -fxd; git reset --hard {COMMIT} && \
CC=$CC CXX=$CXX cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release && ninja -C build bitcoind -j1 && \
./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=$STOP -printtoconsole=0; sleep 20 && rm -f $DATA_DIR/debug.log && rm -rfd $DATA_DIR/chainstate $DATA_DIR/indexes"     --conclude "killall bitcoind || true; sleep 100; sync; echo 3 | sudo tee /proc/sys/vm/drop_caches; \
grep -q 'height=0' $DATA_DIR/debug.log && \
grep -q 'height=$STOP' $DATA_DIR/debug.log && \
grep 'Bitcoin Core version' $DATA_DIR/debug.log | grep -q \"\$(git rev-parse --short=12 {COMMIT})\" && \
cp $DATA_DIR/debug.log $LOG_DIR/debug-{COMMIT}-\$(date +%s).log"   "COMPILER=$CC ./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=$STOP -dbcache=$DBCACHE -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0"; done

ed7dd7cf4e Merge bitcoin/bitcoin#34371: wallet: allow importprunedfunds for spending transactions
7c4efd1174 doc: add block read-ahead release note
c5aafe2557 doc: add block read-ahead release note

2026-09-29 | reindex-chainstate | 968869 blocks | dbcache 2000 | ssd-ryzen | x86_64 | AMD Ryzen 7 3700X 8-Core Processor | 16 threads | 62Gi RAM | SSD

Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=968869 -dbcache=2000 -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0 (COMMIT = ed7dd7cf4e1561a97edf72eba29a67b14e28c717)
  Time (abs ≡):        12430.900 s               [User: 36689.125 s, System: 2521.533 s]

Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=968869 -dbcache=2000 -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0 (COMMIT = 7c4efd1174070eba967fb0803c2080500b77ee10)
  Time (abs ≡):        9641.134 s               [User: 37300.454 s, System: 2003.075 s]

Benchmark 3: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=968869 -dbcache=2000 -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0 (COMMIT = c5aafe25576d341451d679816ae0d1f8c8405c7e)
  Time (abs ≡):        9562.801 s               [User: 37275.991 s, System: 2185.462 s]

Relative speed comparison
        1.30          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=968869 -dbcache=2000 -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0 (COMMIT = ed7dd7cf4e1561a97edf72eba29a67b14e28c717)
        1.01          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=968869 -dbcache=2000 -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0 (COMMIT = 7c4efd1174070eba967fb0803c2080500b77ee10)
        1.00          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=968869 -dbcache=2000 -reindex-chainstate -assumevalid=00000000000000000000ccebd6d74d9194d8dcdc1d177c478e094bfad51ba5ac -blocksonly -disablewallet -connect=0 -listen=0 -dnsseed=0 -printtoconsole=0 (COMMIT = c5aafe25576d341451d679816ae0d1f8c8405c7e)

</details>

in doc/release-notes-36000.md:4 in 3547915bfb

   0 | @@ -0,0 +1,6 @@
   1 | +Performance Improvements
   2 | +------------------------
   3 | +
   4 | +- A background thread can now prefetch later blocks from disk while another

andrewtoth commented at 6:32 PM on September 8, 2026:

The release notes are stale. There are now multiple threads.

in src/node/blockfetcher.h:26 in 3547915bfb

  21 | +class CBlockIndex;
  22 | +struct FlatFilePos;
  23 | +class uint256;
  24 | +
  25 | +namespace node {
  26 | +/** Supplies blocks to validation. Destruction waits for any queued reads. */

andrewtoth commented at 6:37 PM on September 8, 2026:

This comment is not very helpful IMO. Why would anyone care about destruction waiting on queued reads? Something like this would be better.

/**
 * Reads and deserializes blocks in parallel starting from a requested block index.
 * Useful for improving performance when scanning blocks.
 **/

in src/validation.cpp:3212 in 3547915bfb

3209 |      AssertLockHeld(cs_main);
3210 |      if (m_mempool) AssertLockHeld(m_mempool->cs);
3211 |  
3212 |      const CBlockIndex* pindexOldTip = m_chain.Tip();
3213 |      const CBlockIndex* pindexFork = m_chain.FindFork(index_most_work);
3214 | +    const CBlockIndex* read_ahead_tip{provided_block ? index_most_work.pprev : &index_most_work}; // Avoid rereading the provided block

andrewtoth commented at 6:40 PM on September 8, 2026:

This comment is a bit cryptic.

    // If we have a provided block, we don't need to read it so we can stop reading ahead at the previous block.
    const CBlockIndex* read_ahead_tip{provided_block ? index_most_work.pprev : &index_most_work};

andrewtoth approved

andrewtoth commented at 6:53 PM on September 8, 2026: contributor

ACK 3547915bfb240f280188954c7c5b60ec90145ac5

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, especially for reindex-chainstate. This will also be good for the typical single block reorg during tip, and we can use it almost verbatim for speeding up our optional index syncing in a follow-up.

Workload	Machine	Base elapsed	PR elapsed	Less elapsed time
IBD	i7	5h 39m	4h 53m	13.4%
IBD	i5	7h 12m	6h 56m	3.8%
reindex-chainstate	i7	3h 44m	2h 24m	35.8%
reindex-chainstate	i5	4h 40m	4h 04m	12.7%

in src/validation.cpp:3259 in 3547915bfb outdated

3251 | @@ -3241,7 +3252,9 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex&
3252 |  
3253 |          // Connect new blocks.
3254 |          for (CBlockIndex* pindexConnect : vpindexToConnect | std::views::reverse) {
3255 | -            if (!ConnectTip(state, pindexConnect, pindexConnect == &index_most_work ? pblock : std::shared_ptr<const CBlock>(), connected_blocks, disconnectpool)) {
3256 | +            auto block_to_connect{provided_block && pindexConnect == &index_most_work ? provided_block : m_block_fetcher->Load(pindexConnect->GetBlockHash())};
3257 | +            if (read_ahead_tip) m_block_fetcher->FillQueue(*read_ahead_tip, pindexConnect->nHeight + 1);
3258 | +            if (!ConnectTip(state, pindexConnect, std::move(block_to_connect), connected_blocks, disconnectpool)) {
3259 |                  if (state.IsInvalid()) {
3260 |                      // The block violates a consensus rule.

w0xlt commented at 4:22 PM on September 16, 2026:

When a block was rejected, blocks read ahead for its branch could remain in memory indefinitely.

diff --git a/src/validation.cpp b/src/validation.cpp
index 86a2c8a2a4..0a14db9e9b 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3256,7 +3256,8 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex&
             if (read_ahead_tip) m_block_fetcher->FillQueue(*read_ahead_tip, pindexConnect->nHeight + 1);
             if (!ConnectTip(state, pindexConnect, std::move(block_to_connect), connected_blocks, disconnectpool)) {
                 if (state.IsInvalid()) {
-                    // The block violates a consensus rule.
+                    // The block violates a consensus rule. Discard any prefetched descendants.
+                    m_block_fetcher->Clear();
                     if (state.GetResult() != BlockValidationResult::BLOCK_MUTATED) {
                         InvalidChainFound(vpindexToConnect.front());
                     }

DrahtBot added the label Needs rebase on Sep 24, 2026

l0rinc force-pushed on Sep 24, 2026

l0rinc commented at 11:59 PM on September 24, 2026: contributor

Thanks for the reviews, rebased because of a conflict and addressed the documentation and queue cleanup comments: the fetcher and provided-block comments are clearer, the release note says “threads,” and an invalid block clears prefetched descendants. BENCH timings now include waits for prefetched blocks. I also clear the queue after connecting a caller-provided tip and disable reader threads in deterministic fuzz fixtures.

DrahtBot removed the label Needs rebase on Sep 25, 2026

l0rinc force-pushed on Sep 26, 2026

DrahtBot added the label CI failed on Sep 26, 2026

l0rinc commented at 1:26 AM on September 26, 2026: contributor

Pushed a few versions again, since 3547915, BENCH block-load timing includes waits for prefetched blocks, stale queued reads are cleared after invalid blocks and caller-provided tips, and -blockreadahead=<n> controls the readers and queue capacity (2*n, with 0 disabling read-ahead). I also restored helper and loop code changed in intermediate pushes, so this range-diff shows the net changes:

git range-diff --creation-factor=95 dc0395c585..3547915bfb ed7dd7cf4e..7c4efd1174

DrahtBot removed the label CI failed on Sep 26, 2026

in src/node/chainstatemanager_args.cpp:70 in 7c4efd1174 outdated

  65 | @@ -66,6 +66,12 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManage
  66 |          }
  67 |          opts.prevoutfetch_threads_num = std::min(*value, MAX_PREVOUTFETCH_THREADS);
  68 |      }
  69 | +    if (auto value{args.GetArg<int32_t>("-blockreadahead")}) {
  70 | +        if (*value < 0) {

andrewtoth commented at 1:10 AM on September 28, 2026:

We should bound this to a max value, no?

l0rinc commented at 12:23 AM on September 29, 2026:

I know we usually do that. I didn’t see the need here, but I don’t mind. Pushed a cap of 16 readers per chainstate.

andrewtoth commented at 3:24 AM on September 29, 2026:

Well without this guard the application could spawn millions of threads if misconfigured?

l0rinc commented at 3:32 AM on September 29, 2026:

Sure, but we don't usually guard against user error like -dbcache=-1 or similar - and I did apply it, I was just hesitant because of the reasons above.

in doc/release-notes-36000.md:6 in 7c4efd1174 outdated

   0 | @@ -0,0 +1,7 @@
   1 | +Performance Improvements
   2 | +------------------------
   3 | +
   4 | +- Background threads can now prefetch later blocks from disk while another
   5 | +  block is being connected, speeding up reindexing and initial block download.
   6 | +  Use `-blockreadahead=<n>` to choose the number of reader threads, or set it

andrewtoth commented at 1:11 AM on September 28, 2026:

Should we also add this to the reduce memory usage docs?

l0rinc commented at 12:23 AM on September 29, 2026:

It was already there, but I updated it in the latest push to cover the cap and queue capacity.

in src/validation.cpp:3279 in 7c4efd1174

3275 | @@ -3259,6 +3276,7 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex&
3276 |                      return false;
3277 |                  }
3278 |              } else {
3279 | +                if (is_provided_block) m_block_fetcher->Clear();

andrewtoth commented at 1:19 AM on September 28, 2026:

I don't see the purpose of this line. Seems like it will always be a no-op. I think it should be removed.

l0rinc commented at 12:23 AM on September 29, 2026:

I'm not sure whether it wasn't needed before, but FillQueue now handles cleanup when the read-ahead range ends, so it isn't needed anymore.

in src/validation.cpp:3022 in 7c4efd1174

3018 | @@ -3013,6 +3019,7 @@ bool Chainstate::ConnectTip(
3019 |      BlockValidationState& state,
3020 |      CBlockIndex* pindexNew,
3021 |      std::shared_ptr<const CBlock> block_to_connect,
3022 | +    SteadyClock::time_point load_start,

andrewtoth commented at 1:42 AM on September 28, 2026:

Instead of passing a time_point, could we pass const CBlockIndex* read_ahead_tip? Then we could do

if (!block_to_connect) {
    block_to_connect = m_block_fetcher->Load(pindexNew->GetBlockHash());
}
if (read_ahead_tip) {
    m_block_fetcher->FillQueue(*read_ahead_tip, pindexNew->nHeight + 1);
}

inside ConnectTip instead, and not have to adjust any of the timing variables?

l0rinc commented at 12:17 AM on September 29, 2026:

Thanks, this minimizes the diff, I like the outcome. Loading and refilling now happen inside ConnectTip, keeping the existing timing variables. FillQueue handles the guard internally, so its callers don't need one.

andrewtoth commented at 1:44 AM on September 28, 2026: contributor

Code review 7c4efd1174070eba967fb0803c2080500b77ee10

I measured reindex-chainstate again with this latest commit and it's a blistering 1h 48m.

Left a few suggestions on the latest revision.

l0rinc force-pushed on Sep 29, 2026

l0rinc commented at 12:36 AM on September 29, 2026: contributor

Added retry coverage and stronger reconnection checks, batched read-ahead submissions, unified thread naming with a 16-reader cap and a 2*n queue, and moved loading into ConnectTip to preserve the existing BENCH timers.

w0xlt commented at 8:06 PM on September 29, 2026: contributor

nit: feature_proxy.py disables prevout fetching with -prevoutfetchthreads=0 (added in f82043af50) to avoid starting a thread pool for each of its 7 nodes. With this PR, util.py sets blockreadahead=1 for every node, so each node now starts a reader thread that this test never uses. It runs on a clean chain and connects only the genesis block.

Suggestion:

        # This test launches many nodes; disable prevout prefetching and block read-ahead
        # so we don't spin up thread pools for each one.
        args = [a + ['-prevoutfetchthreads=0', '-blockreadahead=0'] for a in args]

in src/test/blockmanager_tests.cpp:256 in c5aafe2557 outdated

 251 | +    }, /*thread_count=*/1};
 252 | +
 253 | +    fetcher.FillQueue(&index, 0);
 254 | +    BOOST_CHECK(!fetcher.Load(hash));
 255 | +    fetcher.FillQueue(&index, 0);
 256 | +    BOOST_CHECK(Assert(fetcher.Load(hash))->GetHash() == hash);

w0xlt commented at 8:12 PM on September 29, 2026:

nit: 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. This file already uses BOOST_REQUIRE(block) for this at lines 169 and 197:

suggestion:

     const auto loaded{fetcher.Load(hash)};
     BOOST_REQUIRE(loaded);
     BOOST_CHECK_EQUAL(loaded->GetHash(), hash);

l0rinc commented at 10:38 PM on September 29, 2026:

I consider the loaded block a precondition for the hash comparison, so I used Assert() deliberately to keep it a simple one-liner.

w0xlt commented at 8:19 PM on September 29, 2026: contributor

Bikeshed: -blockreadahead=<n> reads like a block count, but it sets the number of reader threads, and the window is 2*n blocks. For example, someone setting -blockreadahead=4 to limit memory to 4 blocks would actually keep 8. Since options are hard to rename after a release, would -blockreadaheadthreads be clearer, matching -prevoutfetchthreads?

l0rinc force-pushed on Sep 29, 2026

l0rinc commented at 10:55 PM on September 29, 2026: contributor

Thanks, the leftover blockreadahead name was indeed confusing. I renamed it to be consistent with the prevout fetcher and added it to the memory savings options in the mentioned test.

DrahtBot added the label Needs rebase on Oct 8, 2026

l0rinc force-pushed on Oct 9, 2026

w0xlt commented at 6:54 AM on October 9, 2026: contributor

ACK 495d50ae0ca75b605495fe825f4034ffdea06544

DrahtBot requested review from andrewtoth on Oct 9, 2026

DrahtBot removed the label Needs rebase on Oct 9, 2026

in src/validation.h:561 in 495d50ae0c

 556 | @@ -556,6 +557,9 @@ class Chainstate
 557 |       */
 558 |      Mutex m_chainstate_mutex;
 559 |  
 560 | +    //! Reads blocks ahead during chain activation
 561 | +    std::unique_ptr<node::BlockFetcher> m_block_fetcher;

andrewtoth commented at 3:39 PM on October 9, 2026:

nit: this can now be util::NotNullUniquePtr<node::BlockFetcher>.

l0rinc commented at 12:19 PM on October 10, 2026:

Indeed, rebased & switched.

in src/node/blockfetcher.cpp:64 in 495d50ae0c

  59 | +        const auto* next{last_index->GetAncestor(next_height + i)};
  60 | +        if (!next || !(next->nStatus & BLOCK_HAVE_DATA)) break;
  61 | +        tasks.emplace_back([this, hash = next->GetBlockHash(), pos = next->GetBlockPos()] {
  62 | +            try {
  63 | +                if (auto block{std::make_shared<CBlock>()}; m_read_block(*block, pos, hash)) return block;
  64 | +            } catch (std::exception&) {} // Retry synchronously when needed

andrewtoth commented at 3:47 PM on October 9, 2026:

I also find this comment cryptic. I'm not sure it's useful.

            } catch (std::exception&) {}

l0rinc commented at 12:22 PM on October 10, 2026:

I over-compressed the comment, and I meant to explain why it's okay to swallow the exception. I kept the rationale but clarified it: ConnectTip() retries failed async read-ahead synchronously.

in test/functional/feature_reindex.py:24 in 78e0753aac

  19 | @@ -18,19 +20,43 @@
  20 |  )
  21 |  
  22 |  
  23 | +def cached_block_count(node, log_start):
  24 | +    """Blocks ConnectTip() did not have to read from disk since log_start."""

andrewtoth commented at 11:33 PM on October 9, 2026:

    """Returns the number of blocks ConnectTip() did not have to read from disk since log_start."""

in test/functional/feature_reindex.py:31 in 78e0753aac

  26 | +        debug_log.seek(log_start)
  27 | +        return debug_log.read().count('Using cached block')
  28 | +
  29 | +
  30 | +def blockread_msgs(count):
  31 | +    """Startup lines logged by the block read-ahead workers, one per worker thread."""

andrewtoth commented at 11:36 PM on October 9, 2026:

    """Returns the startup lines logged by the block read-ahead workers, one per worker thread."""

in test/functional/feature_reindex.py:47 in 78e0753aac

  44 | +            self.generatetoaddress(self.nodes[0], 3, self.nodes[0].get_deterministic_priv_key().address)
  45 |          blockcount = self.nodes[0].getblockcount()
  46 |          self.stop_nodes()
  47 |          extra_args = [["-reindex-chainstate" if justchainstate else "-reindex"]]
  48 | -        self.start_nodes(extra_args)
  49 | +        # Reindex connects multiple blocks in one ActivateBestChain() call, exercising read-ahead

andrewtoth commented at 11:38 PM on October 9, 2026:

I'm not sure what this comment is here for? I think it should be removed.

l0rinc commented at 12:23 PM on October 10, 2026:

Removed

in test/functional/feature_reindex.py:59 in 78e0753aac outdated

  56 | +        self.nodes[0].invalidateblock(block_hash)
  57 | +        log_start = self.nodes[0].debug_log_size(encoding='utf-8')
  58 | +        with self.nodes[0].assert_debug_log(expected_msgs=[], unexpected_msgs=blockread_msgs(2)):
  59 | +            self.nodes[0].reconsiderblock(block_hash)
  60 | +        assert_equal(cached_block_count(self.nodes[0], log_start), 0)  # TODO: Read-ahead should also supply later blocks when reconnecting
  61 | +        assert_equal(self.nodes[0].getblockcount(), blockcount)

andrewtoth commented at 11:55 PM on October 9, 2026:

Why do we repeat this line here? Seems like a redundant assertion that can be removed?

l0rinc commented at 12:24 PM on October 10, 2026:

This checks restoration after invalidate/reconsider, which the earlier assertion doesn't cover. We could also assert height 0 immediately after invalidation to make that transition explicit.

in test/functional/feature_reindex.py:119 in 495d50ae0c

 114 | +            (0, [], blockread_msgs(2) + ["Using cached block"]),
 115 | +        ):
 116 | +            self.log.info(f"Test reindex-chainstate with -blockfetchthreads={readers}")
 117 | +            with node.assert_debug_log(expected_msgs=expected_msgs, unexpected_msgs=unexpected_msgs):
 118 | +                self.restart_node(0, ["-reindex-chainstate", f"-blockfetchthreads={readers}"])
 119 | +            assert_equal(node.getblockcount(), blockcount)

andrewtoth commented at 12:10 AM on October 10, 2026:

Not sure why we assert this here either? This isn't what's under test here. I think we can remove this line and blockcount = node.getblockcount() above.

l0rinc commented at 12:31 PM on October 10, 2026:

Thanks, removed the height assertion and its saved value

in src/node/blockfetcher.h:39 in 495d50ae0c

  34 | +    std::deque<std::future<std::shared_ptr<const CBlock>>> m_followups GUARDED_BY(::cs_main);
  35 | +
  36 | +public:
  37 | +    BlockFetcher(ReadBlockFn read_block, int32_t thread_count);
  38 | +
  39 | +    //! Discard retained results without cancelling submitted reads

andrewtoth commented at 12:12 AM on October 10, 2026:

Maybe something like this is more clear:

    //! Clear queued reads. Queued block reads will no longer be returned via Load, but will still complete asynchronously.

l0rinc commented at 12:31 PM on October 10, 2026:

Thanks, clarified that clearing results doesn't cancel submitted reads and documented Load() and FillQueue().

in src/node/blockfetcher.cpp:68 in 495d50ae0c outdated

  63 | +                if (auto block{std::make_shared<CBlock>()}; m_read_block(*block, pos, hash)) return block;
  64 | +            } catch (std::exception&) {} // Retry synchronously when needed
  65 | +            return std::shared_ptr<CBlock>{};
  66 | +        });
  67 | +    }
  68 | +    if (auto followups{m_pool.Submit(std::move(tasks))}) std::ranges::move(*followups, std::back_inserter(m_followups));

andrewtoth commented at 12:13 AM on October 10, 2026:

I ran a reindex-chainstate with the latest code here, and it was 14 minutes slower at 2h 02m. Maybe it is worth reverting to single task submission, which would make this a bit simpler?

l0rinc commented at 12:30 PM on October 10, 2026:

My comparison favored the bulk version slightly. I'll run a few more tests a bit later and see if we need to revert this.

DrahtBot requested review from andrewtoth on Oct 10, 2026

test: characterize block connection cache use

Record cache use during reindexing, reconnection and a competing fork before introducing read-ahead.

99252a0a4a

refactor: extract provided block selection 68349d6664

validation: add synchronous block prefetch

Separate disk reads from block connection to prepare for overlapping I/O and validation.
The reader callback avoids a dependency cycle with block storage and validation.
Keep caller-provided blocks on the direct path and stop read-ahead at their parent to avoid reading them twice.
Discard results after invalid blocks or when the read-ahead range ends.
Speculative read failures fall back to the existing synchronous path.
Include loading time in the existing BENCH measurements.

Co-authored-by: Andrew Toth <andrewstoth@gmail.com>

bff4e6d8d1

validation: make block prefetch asynchronous

Use a worker pool with one retained read and one worker by default so disk I/O can overlap block connection.
`Chainstate` owns the fetcher and reuses the worker across activation calls.
Limit each chainstate to 16 readers, including callers that set kernel options directly.
Keep deterministic fuzz fixtures at zero workers so AFL can fork without inherited thread handles.

Co-authored-by: bitcoindev1337
Co-authored-by: Andrew Toth <andrewstoth@gmail.com>

62f3c2ab91

validation: prefetch blocks during reorgs

Queue the first sibling before disconnecting the old tip so its disk read can overlap `DisconnectTip()`.
Discard the previous candidate's results so the new branch can be queued immediately.

Co-authored-by: Andrew Toth <andrewstoth@gmail.com>

dd294788bd

validation: queue blocks for read-ahead

A single followup can leave the reader idle between block connections.
Keep a sliding window of two retained reads per thread, with two threads by default.
Measurements favor a window of four or eight reads, so use the smaller window.
Submit the missing reads together so the pool takes its queue lock once and notifies all workers.

Co-authored-by: Andrew Toth <andrewstoth@gmail.com>

b944508b8c

validation: configure block read-ahead threads

Expose `-blockfetchthreads=<n>` so systems with limited memory can reduce or disable read-ahead.
The read-ahead window retains up to two blocks per reader, and zero disables read-ahead.
Clamp higher values to the existing limit of 16 readers per chainstate, limiting the window to 32 blocks.

Co-authored-by: Andrew Toth <andrewstoth@gmail.com>

169be0b0f8

doc: add block read-ahead release note 81006fa18c

l0rinc force-pushed on Oct 10, 2026

l0rinc commented at 1:04 PM on October 10, 2026: contributor

Thanks @andrewtoth, applied most of your suggestions.

in src/node/blockfetcher.cpp:68 in 81006fa18c

  63 | +                if (auto block{std::make_shared<CBlock>()}; m_read_block(*block, pos, hash)) return block;
  64 | +            } catch (std::exception&) {} // ConnectTip() retries failed read-ahead synchronously
  65 | +            return std::shared_ptr<CBlock>{};
  66 | +        });
  67 | +    }
  68 | +    if (auto followups{m_pool.Submit(std::move(tasks))}) std::ranges::move(*followups, std::back_inserter(m_followups));

arejula27 commented at 8:24 PM on October 10, 2026:

nit: when tasks is empty, could we return early here?

arejula27 commented at 8:36 PM on October 10, 2026: contributor

Concept ACK

I was curious whether this could open a window for race conditions, so I checked the following cases and didn't find any:

Block writes: only blocks with BLOCK_HAVE_DATA are queued, and that flag is set after WriteBlock() closes the file, so a block that is still being written is never prefetched.
Pruning: files holding blocks above the tip are never pruned.
Indexes: they read blocks below the tip, while the fetcher reads above it.