Skip to content

Commit 38cc045

Browse files
ryanofskyryanofsky
committed
blockman: Replace m_reindexing with m_blockfiles_indexed
This is a just a mechanical change, renaming and inverting the meaning of the indexing variable. "m_blockfiles_indexed" is a more straightforward name for this variable because this variable just indicates whether or not <datadir>/blocks/blk?????.dat files have been indexed in the <datadir>/blocks/index LevelDB database. The name "m_reindexing" was more confusing, it could be true even if -reindex was not specified, and false when it was specified. Also, the previous name unnecessarily required thinking about the whole reindexing process just to understand simple checks in validation code about whether blocks were indexed. Motivation for this change is to follow up on previous commit, moving away from having multiple variables called "reindex" internally, and instead naming variables individually after what they do and represent.
1 parent 9c643e7 commit 38cc045

File tree

6 files changed

+22
-23
lines changed

6 files changed

+22
-23
lines changed

‎src/bitcoin-chainstate.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ int main(int argc, char* argv[])
151151
{
152152
LOCK(chainman.GetMutex());
153153
std::cout
154-
<< "\t" << "Reindexing: " << std::boolalpha << chainman.m_blockman.m_reindexing.load() << std::noboolalpha << std::endl
154+
<< "\t" << "Reindexing: " << std::boolalpha << !chainman.m_blockman.m_blockfiles_indexed.load() << std::noboolalpha << std::endl
155155
<< "\t" << "Snapshot Active: " << std::boolalpha << chainman.IsSnapshotActive() << std::noboolalpha << std::endl
156156
<< "\t" << "Active Height: " << chainman.ActiveHeight() << std::endl
157157
<< "\t" << "Active IBD: " << std::boolalpha << chainman.IsInitialBlockDownload() << std::noboolalpha << std::endl;

‎src/init.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1669,7 +1669,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
16691669
// if pruning, perform the initial blockstore prune
16701670
// after any wallet rescanning has taken place.
16711671
if (chainman.m_blockman.IsPruneMode()) {
1672-
if (!chainman.m_blockman.m_reindexing) {
1672+
if (chainman.m_blockman.m_blockfiles_indexed) {
16731673
LOCK(cs_main);
16741674
for (Chainstate* chainstate : chainman.GetAll()) {
16751675
uiInterface.InitMessage(_("Pruning blockstore…").translated);

‎src/node/blockstorage.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,7 @@ bool BlockManager::LoadBlockIndexDB(const std::optional<uint256>& snapshot_block
551551
// Check whether we need to continue reindexing
552552
bool fReindexing = false;
553553
m_block_tree_db->ReadReindexing(fReindexing);
554-
if (fReindexing) m_reindexing = true;
554+
if (fReindexing) m_blockfiles_indexed = false;
555555

556556
return true;
557557
}
@@ -1182,7 +1182,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFile
11821182
ImportingNow imp{chainman.m_blockman.m_importing};
11831183

11841184
// -reindex
1185-
if (chainman.m_blockman.m_reindexing) {
1185+
if (!chainman.m_blockman.m_blockfiles_indexed) {
11861186
int nFile = 0;
11871187
// Map of disk positions for blocks with unknown parent (only used for reindex);
11881188
// parent hash -> child disk position, multiple children can have the same parent.
@@ -1205,7 +1205,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFile
12051205
nFile++;
12061206
}
12071207
WITH_LOCK(::cs_main, chainman.m_blockman.m_block_tree_db->WriteReindexing(false));
1208-
chainman.m_blockman.m_reindexing = false;
1208+
chainman.m_blockman.m_blockfiles_indexed = true;
12091209
LogPrintf("Reindexing finished\n");
12101210
// To avoid ending up in a situation without genesis block, re-try initializing (no-op if reindexing worked):
12111211
chainman.ActiveChainstate().LoadGenesisBlock();

‎src/node/blockstorage.h‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -273,11 +273,12 @@ class BlockManager
273273
std::atomic<bool> m_importing{false};
274274

275275
/**
276-
* Tracks if a reindex is currently in progress. Set to true when a reindex
277-
* is requested and false when reindexing completes. Its value is persisted
278-
* in the BlockTreeDB across restarts.
276+
* Whether blockfiles have been added to the block tree database. Normally
277+
* true, but set to false when a reindex is requested and the database is
278+
* wiped. The value is persisted in the database across restarts and will be
279+
* false until reindexing completes.
279280
*/
280-
std::atomic_bool m_reindexing{false};
281+
std::atomic_bool m_blockfiles_indexed{true};
281282

282283
BlockMap m_block_index GUARDED_BY(cs_main);
283284

@@ -358,7 +359,7 @@ class BlockManager
358359
[[nodiscard]] uint64_t GetPruneTarget() const { return m_opts.prune_target; }
359360
static constexpr auto PRUNE_TARGET_MANUAL{std::numeric_limits<uint64_t>::max()};
360361

361-
[[nodiscard]] bool LoadingBlocks() const { return m_importing || m_reindexing; }
362+
[[nodiscard]] bool LoadingBlocks() const { return m_importing || !m_blockfiles_indexed; }
362363

363364
/** Calculate the amount of disk space the block & undo files currently use */
364365
uint64_t CalculateCurrentUsage();

‎src/node/chainstate.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ static ChainstateLoadResult CompleteChainstateInitialization(
5050

5151
if (options.wipe_block_tree_db) {
5252
pblocktree->WriteReindexing(true);
53-
chainman.m_blockman.m_reindexing = true;
53+
chainman.m_blockman.m_blockfiles_indexed = false;
5454
//If we're reindexing in prune mode, wipe away unusable block files and all undo data files
5555
if (options.prune) {
5656
chainman.m_blockman.CleanupBlockRevFiles();
@@ -61,8 +61,6 @@ static ChainstateLoadResult CompleteChainstateInitialization(
6161

6262
// LoadBlockIndex will load m_have_pruned if we've ever removed a
6363
// block file from disk.
64-
// Note that it also sets m_reindexing based on the disk flag!
65-
// From here on, m_reindexing and options.reindex values may be different!
6664
if (!chainman.LoadBlockIndex()) {
6765
if (chainman.m_interrupt) return {ChainstateLoadStatus::INTERRUPTED, {}};
6866
return {ChainstateLoadStatus::FAILURE, _("Error loading block database")};
@@ -85,7 +83,7 @@ static ChainstateLoadResult CompleteChainstateInitialization(
8583
// If we're not mid-reindex (based on disk + args), add a genesis block on disk
8684
// (otherwise we use the one already on disk).
8785
// This is called again in ImportBlocks after the reindex completes.
88-
if (!chainman.m_blockman.m_reindexing && !chainman.ActiveChainstate().LoadGenesisBlock()) {
86+
if (chainman.m_blockman.m_blockfiles_indexed && !chainman.ActiveChainstate().LoadGenesisBlock()) {
8987
return {ChainstateLoadStatus::FAILURE, _("Error initializing block database")};
9088
}
9189

‎src/validation.cpp‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2641,7 +2641,7 @@ bool Chainstate::FlushStateToDisk(
26412641

26422642
CoinsCacheSizeState cache_state = GetCoinsCacheSizeState();
26432643
LOCK(m_blockman.cs_LastBlockFile);
2644-
if (m_blockman.IsPruneMode() && (m_blockman.m_check_for_pruning || nManualPruneHeight > 0) && !m_chainman.m_blockman.m_reindexing) {
2644+
if (m_blockman.IsPruneMode() && (m_blockman.m_check_for_pruning || nManualPruneHeight > 0) && m_chainman.m_blockman.m_blockfiles_indexed) {
26452645
// make sure we don't prune above any of the prune locks bestblocks
26462646
// pruning is height-based
26472647
int last_prune{m_chain.Height()}; // last height we can prune
@@ -3254,10 +3254,10 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex*
32543254
return true;
32553255
}
32563256

3257-
static SynchronizationState GetSynchronizationState(bool init, bool reindexing)
3257+
static SynchronizationState GetSynchronizationState(bool init, bool blockfiles_indexed)
32583258
{
32593259
if (!init) return SynchronizationState::POST_INIT;
3260-
if (reindexing) return SynchronizationState::INIT_REINDEX;
3260+
if (!blockfiles_indexed) return SynchronizationState::INIT_REINDEX;
32613261
return SynchronizationState::INIT_DOWNLOAD;
32623262
}
32633263

@@ -3279,7 +3279,7 @@ static bool NotifyHeaderTip(ChainstateManager& chainman) LOCKS_EXCLUDED(cs_main)
32793279
}
32803280
// Send block tip changed notifications without cs_main
32813281
if (fNotify) {
3282-
chainman.GetNotifications().headerTip(GetSynchronizationState(fInitialBlockDownload, chainman.m_blockman.m_reindexing), pindexHeader->nHeight, pindexHeader->nTime, false);
3282+
chainman.GetNotifications().headerTip(GetSynchronizationState(fInitialBlockDownload, chainman.m_blockman.m_blockfiles_indexed), pindexHeader->nHeight, pindexHeader->nTime, false);
32833283
}
32843284
return fNotify;
32853285
}
@@ -3398,7 +3398,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr<
33983398
}
33993399

34003400
// Always notify the UI if a new block tip was connected
3401-
if (kernel::IsInterrupted(m_chainman.GetNotifications().blockTip(GetSynchronizationState(still_in_ibd, m_chainman.m_blockman.m_reindexing), *pindexNewTip))) {
3401+
if (kernel::IsInterrupted(m_chainman.GetNotifications().blockTip(GetSynchronizationState(still_in_ibd, m_chainman.m_blockman.m_blockfiles_indexed), *pindexNewTip))) {
34023402
// Just breaking and returning success for now. This could
34033403
// be changed to bubble up the kernel::Interrupted value to
34043404
// the caller so the caller could distinguish between
@@ -3624,7 +3624,7 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde
36243624
// parameter indicating the source of the tip change so hooks can
36253625
// distinguish user-initiated invalidateblock changes from other
36263626
// changes.
3627-
(void)m_chainman.GetNotifications().blockTip(GetSynchronizationState(m_chainman.IsInitialBlockDownload(), m_chainman.m_blockman.m_reindexing), *to_mark_failed->pprev);
3627+
(void)m_chainman.GetNotifications().blockTip(GetSynchronizationState(m_chainman.IsInitialBlockDownload(), m_chainman.m_blockman.m_blockfiles_indexed), *to_mark_failed->pprev);
36283628
}
36293629
return true;
36303630
}
@@ -4263,7 +4263,7 @@ void ChainstateManager::ReportHeadersPresync(const arith_uint256& work, int64_t
42634263
m_last_presync_update = now;
42644264
}
42654265
bool initial_download = IsInitialBlockDownload();
4266-
GetNotifications().headerTip(GetSynchronizationState(initial_download, m_blockman.m_reindexing), height, timestamp, /*presync=*/true);
4266+
GetNotifications().headerTip(GetSynchronizationState(initial_download, m_blockman.m_blockfiles_indexed), height, timestamp, /*presync=*/true);
42674267
if (initial_download) {
42684268
int64_t blocks_left{(NodeClock::now() - NodeSeconds{std::chrono::seconds{timestamp}}) / GetConsensus().PowTargetSpacing()};
42694269
blocks_left = std::max<int64_t>(0, blocks_left);
@@ -4790,7 +4790,7 @@ bool ChainstateManager::LoadBlockIndex()
47904790
{
47914791
AssertLockHeld(cs_main);
47924792
// Load block index from databases
4793-
if (!m_blockman.m_reindexing) {
4793+
if (m_blockman.m_blockfiles_indexed) {
47944794
bool ret{m_blockman.LoadBlockIndexDB(SnapshotBlockhash())};
47954795
if (!ret) return false;
47964796

@@ -4961,7 +4961,7 @@ void ChainstateManager::LoadExternalBlockFile(
49614961
}
49624962
}
49634963

4964-
if (m_blockman.IsPruneMode() && !m_blockman.m_reindexing && pblock) {
4964+
if (m_blockman.IsPruneMode() && m_blockman.m_blockfiles_indexed && pblock) {
49654965
// must update the tip for pruning to work while importing with -loadblock.
49664966
// this is a tradeoff to conserve disk space at the expense of time
49674967
// spent updating the tip to be able to prune.

0 commit comments

Comments
 (0)