Bitcoin Core currently implements five indexes. Two are required: the UTXO set (CCoinsViewDB, also often referred to as “coins db”) and the block index (BlockTreeDB). Three are optional: a transaction index (enabled with -txindex), a compact block filter index (enabled with -blockfilterindex=<type>), and a coinstats index (enabled with -coinstatsindex).
When running with -reindex, all indexes are wiped and rebuilt (generally from the block files on disk). This process can take quite a while, and it can be aborted by the user before it is finished.
Because the node needs to have an up to date UTXO set and block index, the reindexing state is persisted on disk. When a reindex is started, a flag is set, and it will only be unset when the reindex is finished. This way, when the node starts, it can detect that it should continue reindexing, even if the user didn’t provide the flag as a startup option.
This PR can make node startup more efficient by avoiding the wiping of the optional indexes when it is not necessary.
What is the behaviour change introduced by this PR. Can you reproduce it, and if so - how?
What are the two ways an index can process new blocks? How does this PR affect that logic?
What are the potential risks of not wiping the optional indexes when a reindex is continued? How are these addressed, if at all?
What is the difference between ChainstateLoadOptions::reindex and BlockManager::Options::reindex? Why do we need both?
Does 9de8b26 introduce any behaviour change? What is the relation between chainman.m_blockman.m_reindexing and blockman_opts.reindex? When are they the same, when are they different?
This PR fixes a bug introduced in b47bd95. What is the bug, and under which circumstances could it manifest?
The 0d04433 commit message states that “Log messages indicating when indexes and chainstate databases are loaded exist in other places.”. What are these places?
Are there any circumstances under which 9de8b26 will cause an optional index to be wiped, where prior to this commit it wouldn’t be wiped?
<stickies-v> ah, yay. welcome everyone! today we're looking at #30132, authored by TheCharlatan. the notes and questions are available on https://bitcoincore.reviews/30132
<monlovesmango> start bitcoind with reindex flag with additional optional indexes, restart bitcoind, check that optional index's progress isn't wiped out
<kevkevin> looks like we want to stop the reindex from being deleted if the user restarts their bitcoind without the reindex flag after one has been started
<kevkevin> looks like we want to use the reindex param provided by the user, but currently it is not clear if it will be true or false in options.reindex
<stickies-v> so we have this convenience feature for GUI users where if loading the chainstate fails, we ask them (in a popup box, so GUI only) if they'd like to try again with reindex
<stickies-v> but in b47bd959207e82555f07e028cc2246943d32d4c3, that behaviour was accidentally changed a bit - if the user responded yes to that, we would try again but without reindexing, and then we'd keep asking the user (ad infinitum) if they want to try again with reindex
<TheCharlatan> yes, being more concrete the problem is that it (the chainman) gets destroyed and recreated on each for loop iteration. So the users choice is immediately discarded.
<stickies-v> one way is to process them through the validation interface (after registering itself with `RegisterValidationInterface()`) on a per-block basis, or another is to do it in batch through a background sync (through `BaseIndex::StartBackgroundSync()`)
<kevkevin> looks like in BaseIndex::Init we use RegisterValidationInterface and it seems like background one (StartIndexBackgroundSync) is used in AppInitMain
<stickies-v> monlovesmango: it does indeed! although arriving not just in a "received from peer" sense, but importantly in this case also when processed while reindexing
<stickies-v> so this is the scenario: a user has started bitcoind with -reindex, aborted after half an hour (well before reindex has finished), and then started bitcoind again *without* the -reindex flag
<monlovesmango> i don't think the behavior between whether validation interface or background sync is used has changed..? i must be missing something. whether indexes are wiped or not on restart, the index is still out of sync
<stickies-v> reindex is a local process, i.e. we rebuild all of our indices again *from disk*, so generally there's no network activity involved (although we may have to request blocks from peers if they are missing on disk)
<stickies-v> a peer's chain tip is not relevant in any of the code we're looking at here (but the confusion is very understandable), that's handled in the p2p code