From ca91c244ef1ba7eac6643d66a5fc56d3a2a8b550 Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 16 May 2023 18:35:35 -0300 Subject: [PATCH] index: verify blocks data existence only once At present, during init, we traverse the chain (once per index) to confirm that all necessary blocks to sync each index up to the current tip are present. To make the process more efficient, we can fetch the oldest block from the indexers and perform the chain data existence check from that point only once. This also moves the pruning violation check to the end of the 'loadinit' thread, which is where the reindex, block loading and chain activation processes happen. Making the node's startup process faster, allowing us to remove the global g_indexes_ready_to_sync flag, and enabling the execution of the pruning violation verification even when the reindex or reindex-chainstate flags are enabled (which has being skipped so far). --- src/index/base.cpp | 38 ++++++++----------------------- src/index/base.h | 1 + src/init.cpp | 41 +++++++++++++++++++++++++++++++--- src/node/blockstorage.cpp | 3 --- src/node/blockstorage.h | 1 - src/test/util/setup_common.cpp | 1 - 6 files changed, 48 insertions(+), 37 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 1babfacb157..55fb154d99c 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -23,8 +23,6 @@ #include #include -using node::g_indexes_ready_to_sync; - constexpr uint8_t DB_BEST_BLOCK{'B'}; constexpr auto SYNC_LOG_INTERVAL{30s}; @@ -107,32 +105,8 @@ bool BaseIndex::Init() SetBestBlockIndex(locator_index); } - // Skip pruning check if indexes are not ready to sync (because reindex-chainstate has wiped the chain). - const CBlockIndex* start_block = m_best_block_index.load(); - bool synced = start_block == active_chain.Tip(); - if (!synced && g_indexes_ready_to_sync) { - const CBlockIndex* block_to_test = start_block ? start_block : active_chain.Genesis(); - - // Assert block_to_test is not null here. It can't be null because the - // genesis block can't be null here. The genesis block will be null - // during this BaseIndex::Init() call if the node is being started for - // the first time, or if -reindex is used. But in both of these cases - // m_best_block_index is also null so this branch is not reached. - assert(block_to_test); - - if (!active_chain.Contains(block_to_test)) { - // if the bestblock is not part of the mainchain, find the fork - // so we can make sure we have all data down to the fork - block_to_test = active_chain.FindFork(block_to_test); - } - - // make sure we have all block data back to the start block - if (!m_chainstate->m_blockman.CheckBlockDataAvailability(*active_chain.Tip(), *Assert(block_to_test))) { - return InitError(strprintf(Untranslated("%s best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)"), GetName())); - } - } - // Child init + const CBlockIndex* start_block = m_best_block_index.load(); if (!CustomInit(start_block ? std::make_optional(interfaces::BlockKey{start_block->GetBlockHash(), start_block->nHeight}) : std::nullopt)) { return false; } @@ -140,7 +114,7 @@ bool BaseIndex::Init() // Note: this will latch to true immediately if the user starts up with an empty // datadir and an index enabled. If this is the case, indexation will happen solely // via `BlockConnected` signals until, possibly, the next restart. - m_synced = synced; + m_synced = start_block == active_chain.Tip(); m_init = true; return true; } @@ -412,7 +386,13 @@ IndexSummary BaseIndex::GetSummary() const IndexSummary summary{}; summary.name = GetName(); summary.synced = m_synced; - summary.best_block_height = m_best_block_index ? m_best_block_index.load()->nHeight : 0; + if (const auto& pindex = m_best_block_index.load()) { + summary.best_block_height = pindex->nHeight; + summary.best_block_hash = pindex->GetBlockHash(); + } else { + summary.best_block_height = 0; + summary.best_block_hash = m_chain->getBlockHash(0); + } return summary; } diff --git a/src/index/base.h b/src/index/base.h index 8b986fe8c42..9b2a41dc92b 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -23,6 +23,7 @@ struct IndexSummary { std::string name; bool synced{false}; int best_block_height{0}; + uint256 best_block_hash; }; /** diff --git a/src/init.cpp b/src/init.cpp index 389654fe4f8..e96485b3aac 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -125,7 +125,6 @@ using node::CalculateCacheSizes; using node::DEFAULT_PERSIST_MEMPOOL; using node::DEFAULT_PRINTPRIORITY; using node::fReindex; -using node::g_indexes_ready_to_sync; using node::KernelNotifications; using node::LoadChainstate; using node::MempoolPath; @@ -1545,8 +1544,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 8: start indexers - // If reindex-chainstate was specified, delay syncing indexes until ImportBlocks has reindexed the chain - if (!fReindexChainState) g_indexes_ready_to_sync = true; if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) { auto result{WITH_LOCK(cs_main, return CheckLegacyTxindex(*Assert(chainman.m_blockman.m_block_tree_db)))}; if (!result) { @@ -1884,6 +1881,44 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) bool StartIndexBackgroundSync(NodeContext& node) { + // Find the oldest block among all indexes. + // This block is used to verify that we have the required blocks' data stored on disk, + // starting from that point up to the current tip. + // indexes_start_block='nullptr' means "start from height 0". + std::optional indexes_start_block; + std::string older_index_name; + + ChainstateManager& chainman = *Assert(node.chainman); + for (auto index : node.indexes) { + const IndexSummary& summary = index->GetSummary(); + if (summary.synced) continue; + + // Get the last common block between the index best block and the active chain + LOCK(::cs_main); + const CChain& active_chain = chainman.ActiveChain(); + const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(summary.best_block_hash); + if (!active_chain.Contains(pindex)) { + pindex = active_chain.FindFork(pindex); + } + + if (!indexes_start_block || !pindex || pindex->nHeight < indexes_start_block.value()->nHeight) { + indexes_start_block = pindex; + older_index_name = summary.name; + if (!pindex) break; // Starting from genesis so no need to look for earlier block. + } + }; + + // Verify all blocks needed to sync to current tip are present. + if (indexes_start_block) { + LOCK(::cs_main); + const CBlockIndex* start_block = *indexes_start_block; + if (!start_block) start_block = chainman.ActiveChain().Genesis(); + if (!chainman.m_blockman.CheckBlockDataAvailability(*chainman.ActiveChain().Tip(), *Assert(start_block))) { + return InitError(strprintf(Untranslated("%s best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)"), older_index_name)); + } + } + + // Start threads for (auto index : node.indexes) if (!index->StartBackgroundSync()) return false; return true; } diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index e3c6b062b2e..b3ef29a4453 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -26,7 +26,6 @@ namespace node { std::atomic_bool fReindex(false); -std::atomic_bool g_indexes_ready_to_sync{false}; bool CBlockIndexWorkComparator::operator()(const CBlockIndex* pa, const CBlockIndex* pb) const { @@ -954,7 +953,5 @@ void ImportBlocks(ChainstateManager& chainman, std::vector vImportFile return; } } // End scope of ImportingNow - - g_indexes_ready_to_sync = true; } } // namespace node diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 07ae223cbf9..0b8b6de6ad6 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -50,7 +50,6 @@ static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = CMessageHeader::MESSAGE_START_SIZE + sizeof(unsigned int); extern std::atomic_bool fReindex; -extern std::atomic_bool g_indexes_ready_to_sync; // Because validation code takes pointers to the map's CBlockIndex objects, if // we ever switch to another associative container, we need to either use a diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 3e2f0ab88d5..d8f30bdc6e6 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -157,7 +157,6 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto noui_connect(); noui_connected = true; } - node::g_indexes_ready_to_sync = true; } BasicTestingSetup::~BasicTestingSetup()