diff options
author | Ryan Ofsky <ryan@ofsky.org> | 2023-07-10 11:53:47 -0400 |
---|---|---|
committer | Ryan Ofsky <ryan@ofsky.org> | 2023-07-10 11:56:11 -0400 |
commit | ef29d5d7e239b42269dd22ea94a709b5e4ceb5e5 (patch) | |
tree | bac8a6d82f7cd21c2fa1dac195b51cbf390ebc15 | |
parent | c464e67e0b71c397f6b3d3a920d98a1a2a296c7a (diff) | |
parent | ca91c244ef1ba7eac6643d66a5fc56d3a2a8b550 (diff) |
Merge bitcoin/bitcoin#27607: index: make startup more efficient
ca91c244ef1ba7eac6643d66a5fc56d3a2a8b550 index: verify blocks data existence only once (furszy)
fcbdaeef4d5a63e3e5b479c6fcad730eb86fb923 init: don't start indexes sync thread prematurely (furszy)
2ec89f1970935d27631bcd17b7923a79cdb1edbb refactor: simplify pruning violation check (furszy)
c82ef91eae38f385d408b095ebbc8a180ce52ebb make GetFirstStoredBlock assert that 'start_block' always has data (furszy)
430e7027a18870a296abb0bbd9332cbe40d8fdc0 refactor: index, decouple 'Init' from 'Start' (furszy)
225e213110602b4fd1d345167f5f92d26557f6c1 refactor: init indexes, decouple 'Start()' from the creation step (furszy)
2ebc7e68cc9d347807b646f601b27940c9590c89 doc: describe 'init load' thread actions (Martin Zumsande)
04575106b2529f495ce8110ddf7ed2247d4bc339 scripted-diff: rename 'loadblk' thread name to 'initload' (furszy)
ed4462cc78afd2065bbf5bd79728852b65b9ad7f init: start indexes sync earlier (furszy)
Pull request description:
Simplifies index startup code, eliminating the `g_indexes_ready_to_sync` variable,
deduplicating code and moving the prune violation check out of the `BaseIndex` class.
Also makes startup more efficient by running the prune violation check once for all indexes
instead of once for each index, and by delaying the prune violation check and moving it off
of the main thread so the node can start up faster and perform the block data availability
verification even when the '-reindex" or the "-reindex-chainstate" flags are enabled (which
hasn't being possible so far).
ACKs for top commit:
ryanofsky:
Code review ACK ca91c244ef1ba7eac6643d66a5fc56d3a2a8b550. Just rebase and suggested changes since last review (Start return check, and code simplification)
TheCharlatan:
re-ACK ca91c244ef1ba7eac6643d66a5fc56d3a2a8b550
Tree-SHA512: e9c98ce89aeb29e8d0f505f17b34aa54fe44efefbf017f4746e3b446ab4de25ade4f707254a0bbe4b99b69731b04a4067ce529eb7aa834ced196784b694cf7ce
-rw-r--r-- | doc/developer-notes.md | 5 | ||||
-rw-r--r-- | src/bitcoin-chainstate.cpp | 2 | ||||
-rw-r--r-- | src/index/base.cpp | 75 | ||||
-rw-r--r-- | src/index/base.h | 15 | ||||
-rw-r--r-- | src/init.cpp | 81 | ||||
-rw-r--r-- | src/init.h | 3 | ||||
-rw-r--r-- | src/node/blockstorage.cpp | 24 | ||||
-rw-r--r-- | src/node/blockstorage.h | 14 | ||||
-rw-r--r-- | src/node/chainstate.cpp | 2 | ||||
-rw-r--r-- | src/node/context.h | 4 | ||||
-rw-r--r-- | src/rpc/blockchain.cpp | 7 | ||||
-rw-r--r-- | src/test/blockfilter_index_tests.cpp | 3 | ||||
-rw-r--r-- | src/test/blockmanager_tests.cpp | 39 | ||||
-rw-r--r-- | src/test/coinstatsindex_tests.cpp | 9 | ||||
-rw-r--r-- | src/test/txindex_tests.cpp | 3 | ||||
-rw-r--r-- | src/test/util/setup_common.cpp | 1 | ||||
-rw-r--r-- | src/validation.h | 2 | ||||
-rwxr-xr-x | test/functional/feature_index_prune.py | 4 | ||||
-rwxr-xr-x | test/functional/feature_init.py | 2 | ||||
-rwxr-xr-x | test/functional/test_framework/test_node.py | 2 |
20 files changed, 189 insertions, 108 deletions
diff --git a/doc/developer-notes.md b/doc/developer-notes.md index fca72914a3..80353bcdd2 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -621,8 +621,9 @@ Threads : Started from `main()` in `bitcoind.cpp`. Responsible for starting up and shutting down the application. -- [ThreadImport (`b-loadblk`)](https://doxygen.bitcoincore.org/namespacenode.html#ab4305679079866f0f420f7dbf278381d) - : Loads blocks from `blk*.dat` files or `-loadblock=<file>` on startup. +- [Init load (`b-initload`)](https://doxygen.bitcoincore.org/namespacenode.html#ab4305679079866f0f420f7dbf278381d) + : Performs various loading tasks that are part of init but shouldn't block the node from being started: external block import, + reindex, reindex-chainstate, main chain activation, spawn indexes background sync threads and mempool load. - [CCheckQueue::Loop (`b-scriptch.x`)](https://doxygen.bitcoincore.org/class_c_check_queue.html#a6e7fa51d3a25e7cb65446d4b50e6a987) : Parallel script validation threads for transactions in blocks. diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index c36233207e..45fd239e20 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -287,7 +287,7 @@ epilogue: // Without this precise shutdown sequence, there will be a lot of nullptr // dereferencing and UB. scheduler.stop(); - if (chainman.m_load_block.joinable()) chainman.m_load_block.join(); + if (chainman.m_thread_load.joinable()) chainman.m_thread_load.join(); StopScriptCheckWorkerThreads(); GetMainSignals().FlushBackgroundCallbacks(); diff --git a/src/index/base.cpp b/src/index/base.cpp index cf07cae286..55fb154d99 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -23,8 +23,6 @@ #include <string> #include <utility> -using node::g_indexes_ready_to_sync; - constexpr uint8_t DB_BEST_BLOCK{'B'}; constexpr auto SYNC_LOG_INTERVAL{30s}; @@ -81,6 +79,13 @@ BaseIndex::~BaseIndex() bool BaseIndex::Init() { + // m_chainstate member gives indexing code access to node internals. It is + // removed in followup https://github.com/bitcoin/bitcoin/pull/24230 + m_chainstate = &m_chain->context()->chainman->ActiveChainstate(); + // Register to validation interface before setting the 'm_synced' flag, so that + // callbacks are not missed once m_synced is true. + RegisterValidationInterface(this); + CBlockLocator locator; if (!GetDB().ReadBestBlock(locator)) { locator.SetNull(); @@ -100,45 +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) { - bool prune_violation = false; - if (!start_block) { - // index is not built yet - // make sure we have all block data back to the genesis - prune_violation = m_chainstate->m_blockman.GetFirstStoredBlock(*active_chain.Tip()) != active_chain.Genesis(); - } - // in case the index has a best block set and is not fully synced - // check if we have the required blocks to continue building the index - else { - const CBlockIndex* block_to_test = start_block; - if (!active_chain.Contains(block_to_test)) { - // if the bestblock is not part of the mainchain, find the fork - // and make sure we have all data down to the fork - block_to_test = active_chain.FindFork(block_to_test); - } - const CBlockIndex* block = active_chain.Tip(); - prune_violation = true; - // check backwards from the tip if we have all block data until we reach the indexes bestblock - while (block_to_test && block && (block->nStatus & BLOCK_HAVE_DATA)) { - if (block_to_test == block) { - prune_violation = false; - break; - } - // block->pprev must exist at this point, since block_to_test is part of the chain - // and thus must be encountered when going backwards from the tip - assert(block->pprev); - block = block->pprev; - } - } - if (prune_violation) { - 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; } @@ -146,7 +114,8 @@ 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; } @@ -168,12 +137,6 @@ static const CBlockIndex* NextSyncBlock(const CBlockIndex* pindex_prev, CChain& void BaseIndex::ThreadSync() { - // Wait for a possible reindex-chainstate to finish until continuing - // with the index sync - while (!g_indexes_ready_to_sync) { - if (!m_interrupt.sleep_for(std::chrono::milliseconds(500))) return; - } - const CBlockIndex* pindex = m_best_block_index.load(); if (!m_synced) { std::chrono::steady_clock::time_point last_log_time{0s}; @@ -401,15 +364,9 @@ void BaseIndex::Interrupt() m_interrupt(); } -bool BaseIndex::Start() +bool BaseIndex::StartBackgroundSync() { - // m_chainstate member gives indexing code access to node internals. It is - // removed in followup https://github.com/bitcoin/bitcoin/pull/24230 - m_chainstate = &m_chain->context()->chainman->ActiveChainstate(); - // Need to register this ValidationInterface before running Init(), so that - // callbacks are not missed if Init sets m_synced to true. - RegisterValidationInterface(this); - if (!Init()) return false; + if (!m_init) throw std::logic_error("Error: Cannot start a non-initialized index"); m_thread_sync = std::thread(&util::TraceThread, GetName(), [this] { ThreadSync(); }); return true; @@ -429,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 8affee90f8..9b2a41dc92 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; }; /** @@ -54,6 +55,8 @@ protected: }; private: + /// Whether the index has been initialized or not. + std::atomic<bool> m_init{false}; /// Whether the index is in sync with the main chain. The flag is flipped /// from false to true once, after which point this starts processing /// ValidationInterface notifications to stay in sync. @@ -69,9 +72,6 @@ private: std::thread m_thread_sync; CThreadInterrupt m_interrupt; - /// Read best block locator and check that data needed to sync has not been pruned. - bool Init(); - /// Sync the index with the block index starting from the current best block. /// Intended to be run in its own thread, m_thread_sync, and can be /// interrupted with m_interrupt. Once the index gets in sync, the m_synced @@ -142,9 +142,12 @@ public: void Interrupt(); - /// Start initializes the sync state and registers the instance as a - /// ValidationInterface so that it stays in sync with blockchain updates. - [[nodiscard]] bool Start(); + /// Initializes the sync state and registers the instance to the + /// validation interface so that it stays in sync with blockchain updates. + [[nodiscard]] bool Init(); + + /// Starts the initial sync process. + [[nodiscard]] bool StartBackgroundSync(); /// Stops the instance from staying in sync with blockchain updates. void Stop(); diff --git a/src/init.cpp b/src/init.cpp index 988976028d..e96485b3aa 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -125,13 +125,12 @@ 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; using node::NodeContext; using node::ShouldPersistMempool; -using node::ThreadImport; +using node::ImportBlocks; using node::VerifyLoadedChainstate; static constexpr bool DEFAULT_PROXYRANDOMIZE{true}; @@ -268,7 +267,7 @@ void Shutdown(NodeContext& node) // After everything has been shut down, but before things get flushed, stop the // CScheduler/checkqueue, scheduler and load block thread. if (node.scheduler) node.scheduler->stop(); - if (node.chainman && node.chainman->m_load_block.joinable()) node.chainman->m_load_block.join(); + if (node.chainman && node.chainman->m_thread_load.joinable()) node.chainman->m_thread_load.join(); StopScriptCheckWorkerThreads(); // After the threads that potentially access these pointers have been stopped, @@ -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 ThreadImport 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) { @@ -1554,25 +1551,22 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } g_txindex = std::make_unique<TxIndex>(interfaces::MakeChain(node), cache_sizes.tx_index, false, fReindex); - if (!g_txindex->Start()) { - return false; - } + node.indexes.emplace_back(g_txindex.get()); } for (const auto& filter_type : g_enabled_filter_types) { InitBlockFilterIndex([&]{ return interfaces::MakeChain(node); }, filter_type, cache_sizes.filter_index, false, fReindex); - if (!GetBlockFilterIndex(filter_type)->Start()) { - return false; - } + node.indexes.emplace_back(GetBlockFilterIndex(filter_type)); } if (args.GetBoolArg("-coinstatsindex", DEFAULT_COINSTATSINDEX)) { g_coin_stats_index = std::make_unique<CoinStatsIndex>(interfaces::MakeChain(node), /*cache_size=*/0, false, fReindex); - if (!g_coin_stats_index->Start()) { - return false; - } + node.indexes.emplace_back(g_coin_stats_index.get()); } + // Init indexes + for (auto index : node.indexes) if (!index->Init()) return false; + // ********************************************************* Step 9: load wallet for (const auto& client : node.chain_clients) { if (!client->load()) { @@ -1656,15 +1650,24 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) vImportFiles.push_back(fs::PathFromString(strFile)); } - chainman.m_load_block = std::thread(&util::TraceThread, "loadblk", [=, &chainman, &args] { - ThreadImport(chainman, vImportFiles, ShouldPersistMempool(args) ? MempoolPath(args) : fs::path{}); + chainman.m_thread_load = std::thread(&util::TraceThread, "initload", [=, &chainman, &args, &node] { + // Import blocks + ImportBlocks(chainman, vImportFiles); + // Start indexes initial sync + if (!StartIndexBackgroundSync(node)) { + bilingual_str err_str = _("Failed to start indexes, shutting down.."); + chainman.GetNotifications().fatalError(err_str.original, err_str); + return; + } + // Load mempool from disk + chainman.ActiveChainstate().LoadMempool(ShouldPersistMempool(args) ? MempoolPath(args) : fs::path{}); }); // Wait for genesis block to be processed { WAIT_LOCK(g_genesis_wait_mutex, lock); // We previously could hang here if StartShutdown() is called prior to - // ThreadImport getting started, so instead we just wait on a timer to + // ImportBlocks getting started, so instead we just wait on a timer to // check ShutdownRequested() regularly. while (!fHaveGenesis && !ShutdownRequested()) { g_genesis_wait_cv.wait_for(lock, std::chrono::milliseconds(500)); @@ -1875,3 +1878,47 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) return true; } + +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<const CBlockIndex*> 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/init.h b/src/init.h index 4dcf3fc51e..f27d6120ef 100644 --- a/src/init.h +++ b/src/init.h @@ -73,4 +73,7 @@ bool AppInitMain(node::NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip */ void SetupServerArgs(ArgsManager& argsman); +/** Validates requirements to run the indexes and spawns each index initial sync thread */ +bool StartIndexBackgroundSync(node::NodeContext& node); + #endif // BITCOIN_INIT_H diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 191b6dc2f5..b3ef29a445 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 { @@ -403,16 +402,31 @@ bool BlockManager::IsBlockPruned(const CBlockIndex* pblockindex) return (m_have_pruned && !(pblockindex->nStatus & BLOCK_HAVE_DATA) && pblockindex->nTx > 0); } -const CBlockIndex* BlockManager::GetFirstStoredBlock(const CBlockIndex& start_block) +const CBlockIndex* BlockManager::GetFirstStoredBlock(const CBlockIndex& upper_block, const CBlockIndex* lower_block) { AssertLockHeld(::cs_main); - const CBlockIndex* last_block = &start_block; + const CBlockIndex* last_block = &upper_block; + assert(last_block->nStatus & BLOCK_HAVE_DATA); // 'upper_block' must have data while (last_block->pprev && (last_block->pprev->nStatus & BLOCK_HAVE_DATA)) { + if (lower_block) { + // Return if we reached the lower_block + if (last_block == lower_block) return lower_block; + // if range was surpassed, means that 'lower_block' is not part of the 'upper_block' chain + // and so far this is not allowed. + assert(last_block->nHeight >= lower_block->nHeight); + } last_block = last_block->pprev; } + assert(last_block != nullptr); return last_block; } +bool BlockManager::CheckBlockDataAvailability(const CBlockIndex& upper_block, const CBlockIndex& lower_block) +{ + if (!(upper_block.nStatus & BLOCK_HAVE_DATA)) return false; + return GetFirstStoredBlock(upper_block, &lower_block) == &lower_block; +} + // If we're using -prune with -reindex, then delete block files that will be ignored by the // reindex. Since reindexing works by starting at block file 0 and looping until a blockfile // is missing, do the same here to delete any later block files after a gap. Also delete all @@ -868,7 +882,7 @@ public: } }; -void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles, const fs::path& mempool_path) +void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFiles) { ScheduleBatchPriority(); @@ -939,7 +953,5 @@ void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFile return; } } // End scope of ImportingNow - chainman.ActiveChainstate().LoadMempool(mempool_path); - g_indexes_ready_to_sync = true; } } // namespace node diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 36b8aa4806..0b8b6de6ad 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 @@ -222,8 +221,15 @@ public: //! Returns last CBlockIndex* that is a checkpoint const CBlockIndex* GetLastCheckpoint(const CCheckpointData& data) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - //! Find the first block that is not pruned - const CBlockIndex* GetFirstStoredBlock(const CBlockIndex& start_block LIFETIMEBOUND) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + //! Check if all blocks in the [upper_block, lower_block] range have data available. + //! The caller is responsible for ensuring that lower_block is an ancestor of upper_block + //! (part of the same chain). + bool CheckBlockDataAvailability(const CBlockIndex& upper_block LIFETIMEBOUND, const CBlockIndex& lower_block LIFETIMEBOUND) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + + //! Find the first stored ancestor of start_block immediately after the last + //! pruned ancestor. Return value will never be null. Caller is responsible + //! for ensuring that start_block has data is not pruned. + const CBlockIndex* GetFirstStoredBlock(const CBlockIndex& start_block LIFETIMEBOUND, const CBlockIndex* lower_block=nullptr) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** True if any block files have ever been pruned. */ bool m_have_pruned = false; @@ -255,7 +261,7 @@ public: void CleanupBlockRevFiles() const; }; -void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles, const fs::path& mempool_path); +void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFiles); } // namespace node #endif // BITCOIN_NODE_BLOCKSTORAGE_H diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 3900d2e620..255d8be0ec 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -82,7 +82,7 @@ static ChainstateLoadResult CompleteChainstateInitialization( // At this point blocktree args are consistent with what's on disk. // If we're not mid-reindex (based on disk + args), add a genesis block on disk // (otherwise we use the one already on disk). - // This is called again in ThreadImport after the reindex completes. + // This is called again in ImportBlocks after the reindex completes. if (!fReindex && !chainman.ActiveChainstate().LoadGenesisBlock()) { return {ChainstateLoadStatus::FAILURE, _("Error initializing block database")}; } diff --git a/src/node/context.h b/src/node/context.h index 91b68fa5bb..dae900ff7f 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -15,8 +15,9 @@ #include <vector> class ArgsManager; -class BanMan; class AddrMan; +class BanMan; +class BaseIndex; class CBlockPolicyEstimator; class CConnman; class CScheduler; @@ -58,6 +59,7 @@ struct NodeContext { std::unique_ptr<ChainstateManager> chainman; std::unique_ptr<BanMan> banman; ArgsManager* args{nullptr}; // Currently a raw pointer because the memory is not managed by this struct + std::vector<BaseIndex*> indexes; // raw pointers because memory is not managed by this struct std::unique_ptr<interfaces::Chain> chain; //! List of all chain clients (wallet processes or other client) connected to node. std::vector<std::unique_ptr<interfaces::ChainClient>> chain_clients; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index ee3237638e..717a119b56 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -812,9 +812,7 @@ static RPCHelpMan pruneblockchain() PruneBlockFilesManual(active_chainstate, height); const CBlockIndex& block{*CHECK_NONFATAL(active_chain.Tip())}; - const CBlockIndex* last_block{active_chainstate.m_blockman.GetFirstStoredBlock(block)}; - - return static_cast<int64_t>(last_block->nHeight - 1); + return block.nStatus & BLOCK_HAVE_DATA ? active_chainstate.m_blockman.GetFirstStoredBlock(block)->nHeight - 1 : block.nHeight; }, }; } @@ -1267,7 +1265,8 @@ RPCHelpMan getblockchaininfo() obj.pushKV("size_on_disk", chainman.m_blockman.CalculateCurrentUsage()); obj.pushKV("pruned", chainman.m_blockman.IsPruneMode()); if (chainman.m_blockman.IsPruneMode()) { - obj.pushKV("pruneheight", chainman.m_blockman.GetFirstStoredBlock(tip)->nHeight); + bool has_tip_data = tip.nStatus & BLOCK_HAVE_DATA; + obj.pushKV("pruneheight", has_tip_data ? chainman.m_blockman.GetFirstStoredBlock(tip)->nHeight : tip.nHeight + 1); const bool automatic_pruning{chainman.m_blockman.GetPruneTarget() != BlockManager::PRUNE_TARGET_MANUAL}; obj.pushKV("automatic_pruning", automatic_pruning); diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index 24af51cce1..9bd5c7c2b6 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -113,6 +113,7 @@ bool BuildChainTestingSetup::BuildChain(const CBlockIndex* pindex, BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) { BlockFilterIndex filter_index(interfaces::MakeChain(m_node), BlockFilterType::BASIC, 1 << 20, true); + BOOST_REQUIRE(filter_index.Init()); uint256 last_header; @@ -139,7 +140,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) // BlockUntilSyncedToCurrentChain should return false before index is started. BOOST_CHECK(!filter_index.BlockUntilSyncedToCurrentChain()); - BOOST_REQUIRE(filter_index.Start()); + BOOST_REQUIRE(filter_index.StartBackgroundSync()); // Allow filter index to catch up with the block index. IndexWaitSynced(filter_index); diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index 2ab2fa55f0..e4ed861b12 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -90,4 +90,43 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_scan_unlink_already_pruned_files, TestChain BOOST_CHECK(!AutoFile(blockman.OpenBlockFile(new_pos, true)).IsNull()); } +BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup) +{ + // The goal of the function is to return the first not pruned block in the range [upper_block, lower_block]. + LOCK(::cs_main); + auto& chainman = m_node.chainman; + auto& blockman = chainman->m_blockman; + const CBlockIndex& tip = *chainman->ActiveTip(); + + // Function to prune all blocks from 'last_pruned_block' down to the genesis block + const auto& func_prune_blocks = [&](CBlockIndex* last_pruned_block) + { + LOCK(::cs_main); + CBlockIndex* it = last_pruned_block; + while (it != nullptr && it->nStatus & BLOCK_HAVE_DATA) { + it->nStatus &= ~BLOCK_HAVE_DATA; + it = it->pprev; + } + }; + + // 1) Return genesis block when all blocks are available + BOOST_CHECK_EQUAL(blockman.GetFirstStoredBlock(tip), chainman->ActiveChain()[0]); + BOOST_CHECK(blockman.CheckBlockDataAvailability(tip, *chainman->ActiveChain()[0])); + + // 2) Check lower_block when all blocks are available + CBlockIndex* lower_block = chainman->ActiveChain()[tip.nHeight / 2]; + BOOST_CHECK(blockman.CheckBlockDataAvailability(tip, *lower_block)); + + // Prune half of the blocks + int height_to_prune = tip.nHeight / 2; + CBlockIndex* first_available_block = chainman->ActiveChain()[height_to_prune + 1]; + CBlockIndex* last_pruned_block = first_available_block->pprev; + func_prune_blocks(last_pruned_block); + + // 3) The last block not pruned is in-between upper-block and the genesis block + BOOST_CHECK_EQUAL(blockman.GetFirstStoredBlock(tip), first_available_block); + BOOST_CHECK(blockman.CheckBlockDataAvailability(tip, *first_available_block)); + BOOST_CHECK(!blockman.CheckBlockDataAvailability(tip, *last_pruned_block)); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp index 9dc88ea671..74d6d7231a 100644 --- a/src/test/coinstatsindex_tests.cpp +++ b/src/test/coinstatsindex_tests.cpp @@ -18,6 +18,7 @@ BOOST_AUTO_TEST_SUITE(coinstatsindex_tests) BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup) { CoinStatsIndex coin_stats_index{interfaces::MakeChain(m_node), 1 << 20, true}; + BOOST_REQUIRE(coin_stats_index.Init()); const CBlockIndex* block_index; { @@ -32,7 +33,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup) // is started. BOOST_CHECK(!coin_stats_index.BlockUntilSyncedToCurrentChain()); - BOOST_REQUIRE(coin_stats_index.Start()); + BOOST_REQUIRE(coin_stats_index.StartBackgroundSync()); IndexWaitSynced(coin_stats_index); @@ -83,7 +84,8 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup) const CChainParams& params = Params(); { CoinStatsIndex index{interfaces::MakeChain(m_node), 1 << 20}; - BOOST_REQUIRE(index.Start()); + BOOST_REQUIRE(index.Init()); + BOOST_REQUIRE(index.StartBackgroundSync()); IndexWaitSynced(index); std::shared_ptr<const CBlock> new_block; CBlockIndex* new_block_index = nullptr; @@ -109,8 +111,9 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup) { CoinStatsIndex index{interfaces::MakeChain(m_node), 1 << 20}; + BOOST_REQUIRE(index.Init()); // Make sure the index can be loaded. - BOOST_REQUIRE(index.Start()); + BOOST_REQUIRE(index.StartBackgroundSync()); index.Stop(); } } diff --git a/src/test/txindex_tests.cpp b/src/test/txindex_tests.cpp index 2677502ef0..cb80dbed69 100644 --- a/src/test/txindex_tests.cpp +++ b/src/test/txindex_tests.cpp @@ -17,6 +17,7 @@ BOOST_AUTO_TEST_SUITE(txindex_tests) BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup) { TxIndex txindex(interfaces::MakeChain(m_node), 1 << 20, true); + BOOST_REQUIRE(txindex.Init()); CTransactionRef tx_disk; uint256 block_hash; @@ -29,7 +30,7 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup) // BlockUntilSyncedToCurrentChain should return false before txindex is started. BOOST_CHECK(!txindex.BlockUntilSyncedToCurrentChain()); - BOOST_REQUIRE(txindex.Start()); + BOOST_REQUIRE(txindex.StartBackgroundSync()); // Allow tx index to catch up with the block index. IndexWaitSynced(txindex); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 3e2f0ab88d..d8f30bdc6e 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() diff --git a/src/validation.h b/src/validation.h index 66bc036e66..49860f2d6a 100644 --- a/src/validation.h +++ b/src/validation.h @@ -987,7 +987,7 @@ public: const util::SignalInterrupt& m_interrupt; const Options m_options; - std::thread m_load_block; + std::thread m_thread_load; //! A single BlockManager instance is shared across each constructed //! chainstate to avoid duplicating block metadata. node::BlockManager m_blockman; diff --git a/test/functional/feature_index_prune.py b/test/functional/feature_index_prune.py index 77a056346a..d6e802b399 100755 --- a/test/functional/feature_index_prune.py +++ b/test/functional/feature_index_prune.py @@ -3,6 +3,7 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test indices in conjunction with prune.""" +import os from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -127,8 +128,9 @@ class FeatureIndexPruneTest(BitcoinTestFramework): self.log.info("make sure we get an init error when starting the nodes again with the indices") filter_msg = "Error: basic block filter index best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)" stats_msg = "Error: coinstatsindex best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)" + end_msg = f"{os.linesep}Error: Failed to start indexes, shutting down.." for i, msg in enumerate([filter_msg, stats_msg, filter_msg]): - self.nodes[i].assert_start_raises_init_error(extra_args=self.extra_args[i], expected_msg=msg) + self.nodes[i].assert_start_raises_init_error(extra_args=self.extra_args[i], expected_msg=msg+end_msg) self.log.info("make sure the nodes start again with the indices and an additional -reindex arg") for i in range(3): diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py index 7af67730bd..64ca312b84 100755 --- a/test/functional/feature_init.py +++ b/test/functional/feature_init.py @@ -71,7 +71,7 @@ class InitStressTest(BitcoinTestFramework): b'init message: Starting network threads', b'net thread start', b'addcon thread start', - b'loadblk thread start', + b'initload thread start', b'txindex thread start', b'block filter index thread start', b'coinstatsindex thread start', diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 5111d88e15..e5e3061def 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -250,7 +250,7 @@ class TestNode(): # Wait for the node to finish reindex, block import, and # loading the mempool. Usually importing happens fast or # even "immediate" when the node is started. However, there - # is no guarantee and sometimes ThreadImport might finish + # is no guarantee and sometimes ImportBlocks might finish # later. This is going to cause intermittent test failures, # because generally the tests assume the node is fully # ready after being started. |