index: batch db writes during initial sync

furszy commented at 4:06 am on February 3, 2026: member

Decouples part of #26966.

Right now, index initial sync writes to disk for every block, which is not the best for HDD. This change batches those, so disk writes are less frequent during initial sync. This also cuts down cs_main contention by reducing the number of NextBlockSync calls (instead of calling it for every block, we will call it once per block window), making the node more responsive (IBD and validation) while indexes sync up. On top of that, it lays the groundwork for the bigger speedups, since part of the parallelization pre-work is already in place.

Just as a small summary:

Batch DB writes instead of flushing per block, which improves sync time on HDD due to the reduced number of disk write operations.
Reduce cs_main lock contention, which improves the overall node responsiveness (and primarily IBD) while the indexes threads are syncing.
Lays the groundwork for the real speedups, since part of #26966 parallelization pre-work is introduced here as well.

Note: Pending initial sync benchmark and further testing. I have only focused on decoupling and simplifying what was in #26966 first. Commits should be simple to digest.

DrahtBot added the label UTXO Db and Indexes on Feb 3, 2026

DrahtBot commented at 4:07 am on February 3, 2026: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	optout21
Concept ACK	l0rinc

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34440 (Refactor CChain methods to use references, tests by optout21)
#26966 (index: initial sync speedup, parallelize process 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.

LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

BlockFilterIndex filter_index(interfaces::MakeChain(m_node), BlockFilterType::BASIC, 1 « 20, /f_memory=/false) in src/test/blockfilter_index_tests.cpp

2026-02-10 01:23:16

furszy force-pushed on Feb 3, 2026

DrahtBot added the label CI failed on Feb 3, 2026

DrahtBot commented at 4:42 am on February 3, 2026: contributor

🚧 At least one of the CI tasks failed. Task iwyu: https://github.com/bitcoin/bitcoin/actions/runs/21616547248/job/62296358632 LLM reason (✨ experimental): IWYU (include-what-you-use) reported a failure in the test script, causing the CI to fail.

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.

in src/index/base.cpp:201 in 8368abaa8b outdated

197@@ -198,6 +198,22 @@ bool BaseIndex::ProcessBlock(const CBlockIndex* pindex, const CBlock* block_data
198     return true;
199 }
200 
201+bool BaseIndex::ProcessBlocks(const CBlockIndex* start, const CBlockIndex* end)

fjahr commented at 9:42 am on February 3, 2026:

Why keep ProcessBlock around? It should be simpler if we just have one function and it can be handed one or many blocks.

fjahr commented at 10:06 am on February 3, 2026:

Or, if you just want to keep it for encapsulation, consider making it private at least?

furszy commented at 3:30 pm on February 3, 2026:

Why keep ProcessBlock around? It should be simpler if we just have one function and it can be handed one or many blocks.

Because ProcessBlocks will be specialized in a follow-up commit (please see #26966) to enable additional speedups, such as processing blocks out-of-order for indexes that support it.

Also, I wouldn’t merge them because these two methods solve different problems. ProcessBlocks handles ordering, ProcessBlock handles a single unit of work. I think keeping them separate keeps the code clean and easy to follow while allowing us to introduce improvements in the future.

Or, if you just want to keep it for encapsulation, consider making it private at least?

~~Yeah, could do that.~~

just realized that ProcessBlocks is private already.

in src/index/base.h:32 in e7fa0d00e1 outdated

26@@ -27,6 +27,10 @@ class CBlockIndex;
27 class Chainstate;
28 
29 struct CBlockLocator;
30+
31+/** Range of blocks to process in batches */
32+static constexpr int INDEX_BATCH_SIZE = 500;

fjahr commented at 9:45 am on February 3, 2026:

Has this been optimized with benchmarks or are you planning to do that? This might be something differs between the indexes so it might make sense to let each index define their own value.

furszy commented at 3:13 pm on February 3, 2026:

Has this been optimized with benchmarks or are you planning to do that? This might be something differs between the indexes so it might make sense to let each index define their own value.

Let’s handle it in a focused follow-up so we don’t lose momentum here tuning a parameter.

I’d rather land the structural improvements first, since they unblock other improvements and allow us to continue working on the parallelization goal (which introduces the major sync speedup).

fjahr commented at 9:47 am on February 3, 2026: contributor

This relates to what you wrote here right? It would be helpful that you check if it solves the LevelDB file issue in coinstatsindex and I am also curious about your benchmark results because @l0rinc did not find that this was leading to a speed up.

l0rinc commented at 10:38 am on February 3, 2026: contributor

Concept ACK, I prefer this over #33306 - and will investigate if this solves that problem once my benchmarking servers free up.

because @l0rinc did not find that this was leading to a speed up.

I experimented with something similar in https://github.com/l0rinc/bitcoin/pull/37/changes, will compare it against this change. I wrote:

indexes: batch index writes: 34188s, 211M, 8 files It solved the fragmentation, but didn’t speed up anything. I still think this is a better direction than adding manual compactions.

Most likely since writing isn’t the bottleneck but MuHash calculations were.

furszy commented at 3:02 pm on February 3, 2026: member

This relates to what you wrote here right? It would be helpful that you check if it solves the LevelDB file issue in coinstatsindex and I am also curious about your benchmark results because @l0rinc did not find that this was leading to a speed up. @fjahr the comment was merely an excuse to decouple the DB writes batching code out of #26966 rather than me having a formed opinion in favor or against #33306. That’s why I didn’t mention it in the PR description. Maybe the changes complement each other.

To be crystal clear, just updated the PR description with further details on why this change worth alone, independently on #33306.

To summarize it here, the goal of the changes are:

Batch DB writes instead of flushing per block, which will improve sync time on HDD due to the reduced number of IO operations.
Reduce cs_main lock contention, which orthogonally improves the overall node responsiveness (and primarily IBD) while the indexes threads are syncing.
Lays the groundwork for the real speedups, since part of #26966 parallelization pre-work is introduced here as well.

furszy force-pushed on Feb 3, 2026

furszy commented at 8:26 pm on February 3, 2026: member

Updated per feedback from @hebasto (thanks!). Changed <cinttypes> include for <cstdint> to make IWYU happy.

hebasto commented at 8:38 pm on February 3, 2026: member

Updated per feedback from @hebasto (thanks!). Changed <cinttypes> include for <cstdint> to make IWYU happy.

~~I guess, this is a bug in IWYU caused by https://github.com/include-what-you-use/include-what-you-use/commit/44480a2de0d8ef039b13391997f274ea33750be9.~~

UPDATE: Fixed in #34498.

maflcko commented at 1:57 pm on February 4, 2026: member

Can you run all unit and functional tests on all commits? Or do they time out?

furszy force-pushed on Feb 4, 2026

furszy commented at 3:22 pm on February 4, 2026: member

Can you run all unit and functional tests on all commits? Or do they time out?

Bad squash, my bad. Thanks for the heads up. Fixed. Also rebased the branch to pick up the CI changes.

DrahtBot removed the label CI failed on Feb 4, 2026

Jhackman2019 commented at 2:54 am on February 8, 2026: none

Builds clean on ARM64 (Pi 5, Debian Bookworm). All index-related functional tests and blockfilter_index unit tests pass. :)

in src/index/txindex.cpp:48 in 8826900ddc

bvbfan commented at 8:31 am on February 8, 2026:

This function doesn’t exist anymore.

furszy commented at 4:55 pm on February 8, 2026:

Done, thanks.

optout21 commented at 10:22 pm on February 9, 2026:

A minor comment on the commit description: I feel an ambiguity or typo around “during”.

0Pass CDBBatch to subclasses during so writes can be accumulated and committed together instead of flushing per block.

furszy commented at 1:12 am on February 10, 2026:

Fixed.

furszy force-pushed on Feb 8, 2026

in src/index/base.cpp:246 in eed9ea5fc5 outdated

243+                if (block_batch.start_index) {
244+                    const int start_height = block_batch.start_index->nHeight;
245+                    const int tip_height = m_chainstate->m_chain.Height();
246+                    // Compute the last height in the batch without exceeding the chain tip
247+                    const int batch_end_height = std::min(start_height + m_num_blocks_batch - 1, tip_height);
248+                    block_batch.end_index = m_chainstate->m_chain[batch_end_height];

optout21 commented at 2:06 pm on February 9, 2026:

The start and end are set here, according to the desired size. However, block_batch.start_index can change below. Wouldn’t it be better/cleaner to set it just before the ProcessBlocks call? (I mean to keep the logic up to the deleted pindex = pindex_next; line, and put the bactch setup there.)

furszy commented at 4:39 pm on February 9, 2026:

The start and end are set here, according to the desired size. However, block_batch.start_index can change below.

It seems the code I had for this case got dropped in the previous bad squash. Good catch. See the missing line https://github.com/furszy/bitcoin-core/commit/54874bf43b2df6846061fbb844b7026f69e7bc74#diff-5251d93616c116c364d2d502ad2a59e901a3512194462fabacc9756f96fd8dddR274. Will re-add it. Thanks.

Wouldn’t it be better/cleaner to set it just before the ProcessBlocks call? (I mean to keep the logic up to the deleted pindex = pindex_next; line, and put the bactch setup there.)

That would not only require taking cs_main an extra time, but would also introduce race conditions. The extra locking comes from the chain lookup needed to determine the window’s end index. And, more importantly, if the tip changes between setting the start index and computing the end, we could end up on a fork at a lower height, resulting in an invalid window (start_index > end_index).

optout21 commented at 6:02 pm on February 9, 2026:

That would not only require taking cs_main an extra time, but would also introduce race conditions. The extra locking comes from the chain lookup needed to determine the window’s end index.

What I had in mind is setting both the start and end a bit later, after the pindex is finalized. I will try it out and provide a concrete code snippet a bit later.

furszy commented at 7:00 pm on February 9, 2026:

What I had in mind is setting both the start and end a bit later, after the pindex is finalized. I will try it out and provide a concrete code snippet a bit later.

I see the same two problems there, with an extra requirement of a reorg check and another rewind call. Which makes it even worse.

optout21 commented at 9:56 pm on February 9, 2026:

I acknowledge that it’s important to take the cs_main lock only once in the typical scenario. Also, the missing update of end was identified and fixed! Thx

optout21 commented at 2:29 pm on February 9, 2026: contributor

I am confused as to how this batching relates to the batching logic of the orginal code. Looking at the pre-change BaseIndex::Sync(), it seems that Commit/WriteBatch occured only every 30 seconds (or at the chain tip), not after each block. This change does a WriteBatch after every 500 blocks, which in fact may be even more frequent. The original commit-after-every-30-secs is still there, so I don’t see how this change makes less writes. Maybe I misunderstand the change.

0            if (!ProcessBlock(pindex)) return; // error logged internally
1            ...
2            if (current_time - last_locator_write_time >= SYNC_LOCATOR_WRITE_INTERVAL) {
3                SetBestBlockIndex(pindex);
4                last_locator_write_time = current_time;
5                // No need to handle errors in Commit. See rationale above.
6                Commit();
7            }

furszy commented at 6:48 pm on February 9, 2026: member

I am confused as to how this batching relates to the batching logic of the orginal code. Looking at the pre-change BaseIndex::Sync(), it seems that Commit/WriteBatch occured only every 30 seconds (or at the chain tip), not after each block. This change does a WriteBatch after every 500 blocks, which in fact may be even more frequent. The original commit-after-every-30-secs is still there, so I don’t see how this change makes less writes. Maybe I misunderstand the change.
0            if (!ProcessBlock(pindex)) return; // error logged internally
1            ...
2            if (current_time - last_locator_write_time >= SYNC_LOCATOR_WRITE_INTERVAL) {
3                SetBestBlockIndex(pindex);
4                last_locator_write_time = current_time;
5                // No need to handle errors in Commit. See rationale above.
6                Commit();
7            }

Commit() only persists the locator (i.e. “how far the index has synced”) so we can resume from that point after a restart. It does not flush or batch the per-block digested data.

The block data is written inside CustomAppend() which is called from ProcessBlock() per block, and it writes to the db immediately.

We’re not replacing locator update time nor merging it with something else here, we’re only batching something that previously wasn’t batched at all.

Also, using a block window instead of reusing the existing time-based window is intentional. Time-based flushing makes memory usage hard to predict, and difficult to test (see #32878), because the amount of buffered data depends on runtime speed, which does not necessarily correlate with available memory (batched data stays in memory until it’s flushed..). A fixed block window provides predictable memory usage and deterministic behavior, which is overall better and also integrates nicely with multiple workers sync (#26966). I would argue in favor of ditching the time window in the future, but.. that’s not something related to this PR.

furszy force-pushed on Feb 9, 2026

furszy commented at 8:00 pm on February 9, 2026: member

Updated per feedback, plus added test coverage for the introduced changes. Thanks @optout21.

optout21 commented at 9:52 pm on February 9, 2026: contributor

Thanks for the explanations, and apologies for asking trivial/irrelevant questions.

Commit() only persists the locator (i.e. “how far the index has synced”) so we can resume from that point after a restart. The block data is written inside CustomAppend() which is called from ProcessBlock() per block, and it writes to the db immediately. These are two important points, I can see now.

I was puzzled by the fact that there is a loop in Sync(), and another loop is introduced in ProcessBlocks(). But the two complement each other.

Another approach would be to keep the outer loop in Sync() block-by-block, store a CDBBatch variable, process one block, count the blocks in the batch, and flush the batch if batch size is reached (outside of ProcessBlock). This would be slightly less change (no secondary loop), but – if I’m correct – the cs_main lock would be still accessed at every block.

… I would argue in favor of ditching the time window in the future, but.. that’s not something related to this PR. Maybe Commit() could be called with the same granularity as batch writes, but that’s out of scope here.

furszy commented at 10:11 pm on February 9, 2026: member

Another approach would be to keep the outer loop in Sync() block-by-block, store a CDBBatch variable, process one block, count the blocks in the batch, and flush the batch if batch size is reached (outside of ProcessBlock). This would be slightly less change (no secondary loop), but – if I’m correct – the cs_main lock would be still accessed at every block.

Yes. That would still lock cs_main on every block, which harms the overall node responsiveness, and it is also not very helpful for the parallelization goal, which requires certain isolation between batches so worker threads can process them concurrently.

in src/index/base.cpp:276 in 360c43d7c5

272                 FatalErrorf("Failed to rewind %s to a previous chain tip", GetName());
273                 return;
274             }
275 
276-            // For now, process a single block at time
277             if (!ProcessBlocks(block_batch.start_index, block_batch.end_index)) return; // error logged internally

optout21 commented at 10:17 pm on February 9, 2026:

Code review shows that by this point start_index and end_index are not null, but there is several if’s to ensure that, it’s not trivial. I propose to document/ensure this invariant with an Assume before this line, especially having in mind that ProcessBlocks crashes if start or end is nullptr.

furszy commented at 1:15 am on February 10, 2026:

Probably better to just pass them by ref instead. Will do if have to re-touch. Otherwise it can be tackled in a quick follow-up too.

in src/index/base.cpp:212 in 0d980feb95

207+    }
208+
209+    // And process blocks in forward order: from start to end
210+    for (auto it = ordered_blocks.rbegin(); it != ordered_blocks.rend(); ++it) {
211+        if (m_interrupt) return false;
212+        if (!ProcessBlock(*it)) return false;

optout21 commented at 10:18 pm on February 9, 2026:

nit: the “// error logged internally” comment could be passed on here as well.

furszy commented at 1:12 am on February 10, 2026:

Done as suggested.

optout21 commented at 10:24 pm on February 9, 2026: contributor

crACK 3687ead892d207ddbdfec3f2ef5c086130414d9a

LGTM after clarifications and changes! Code review, no assessment on performance.

Prev: crACK d0f793ac61a0659f4f1e89f7943813244499bf7d

DrahtBot requested review from l0rinc on Feb 9, 2026

index: add method to process block ranges

No behavior change.

Introduce ProcessBlocks(start, end) to handle a range of blocks
in forward order. Currently used per-block, but this lays the
foundation for future batch processing and parallelization.

00eb958910

index: introduce BlockBatch struct for block range handling

No behavior change.

Lays the foundation for batch processing and future parallelization.

1e07ad5315

index: compute block batch window

Sets the end of a block batch, starting from the next
block to sync and stopping at either the configured
batch size or the chain tip.

d0ec1f869c

index: enable DB writes batching during sync

Pass CDBBatch to subclasses so writes can be accumulated
and committed together instead of flushed per block

b4009f5606

test: index, add initial sync batch writes coverage e86517cf94

furszy force-pushed on Feb 10, 2026

furszy commented at 1:19 am on February 10, 2026: member

nits addressed, thanks @optout21. Also, just for completeness, added test coverage for the “reorg during initial sync” scenario that was not covered before.

test: index, add coverage for initial sync reorgs

The general idea of the introduced test is to pause the index during initial sync
so that a fork can be introduced. Then, once unblocked, the test verifies that the
process correctly handles the reorg scenario by checking the resulting synced tip.

3687ead892

furszy force-pushed on Feb 10, 2026

DrahtBot added the label CI failed on Feb 10, 2026

DrahtBot removed the label CI failed on Feb 10, 2026

l0rinc commented at 11:15 am on February 10, 2026: contributor

I just finished measuring the size and file count for this PR (it was running previously for a week with the wrong tail -F grep I reused from #26966#pullrequestreview-2965376357)

 0COMMITS="47c9297172b0866d2b5d0de6b4fd35b798db3929 dd76491174b3a3b60c174822f75abb03fda3b1d4"; \
 1DATA_DIR="/mnt/my_storage/BitcoinData"; export DATA_DIR; \
 2wait_index() { tail -F ${DATA_DIR}/debug.log 2>/dev/null | grep -q -m1 'coinstatsindex is enabled'; killall bitcoind 2>/dev/null || true; sleep 2; }; export -f wait_index; \
 3for COMMIT in $COMMITS; do \
 4  killall bitcoind 2>/dev/null || true; rm -rf "$DATA_DIR/indexes" "$DATA_DIR/debug.log"; \
 5  git fetch -q origin "$COMMIT" >/dev/null 2>&1 || true && git checkout -q "$COMMIT" && git log -1 --pretty='%h %s' "$COMMIT"; \
 6  cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release >/dev/null 2>&1 && ninja -C build >/dev/null 2>&1; \
 7  ./build/bin/bitcoind -datadir="$DATA_DIR" -connect=0 -coinstatsindex=1 -printtoconsole=0 & wait_index; \
 8  du -h "$DATA_DIR/indexes/coinstatsindex/db"; \
 9  find "$DATA_DIR/indexes/coinstatsindex/db" -type f -name '*.ldb' | wc -l; \
10done

0[1]+  Done                    ./build/bin/bitcoind -datadir="$DATA_DIR" -connect=0 -coinstatsindex=1 -printtoconsole=0
147c9297172 Merge bitcoin/bitcoin#32420: mining, ipc: omit dummy extraNonce from coinbase
2[1] 3876795
3[1]+  Done                    ./build/bin/bitcoind -datadir="$DATA_DIR" -connect=0 -coinstatsindex=1 -printtoconsole=0
4216M    /mnt/my_storage/BitcoinData/indexes/coinstatsindex/db
5144

and

0dd76491174 index: enable DB writes batching during sync
1[1] 3885937
2[1]+  Done                    ./build/bin/bitcoind -datadir="$DATA_DIR" -connect=0 -coinstatsindex=1 -printtoconsole=0
3216M    /mnt/my_storage/BitcoinData/indexes/coinstatsindex/db
410

I haven’t measured the speeds yet but will do that next.

index: batch db writes during initial sync #34489

Reviews

Conflicts

LLM Linter (✨ experimental)