From 373cf91531b84bfdd06fdf8abf4dca228029ce6b Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Mon, 23 Sep 2019 14:44:54 -0400 Subject: [PATCH] validation: indexing changes for assumeutxo When using an assumedvalid chainstate, only process validationinterface callbacks from the background chainstate within indexes. This ensures that all indexes are built in-order. Later, we can possibly designate indexes which can be built out of order and continue their operation during snapshot use. Once the background sync has completed, restart the indexes so that they continue to index the now-validated snapshot chainstate. --- src/index/base.cpp | 32 +++++++++++++++++++++++++++++--- src/index/base.h | 12 +++++++++--- src/init.cpp | 30 +++++++++++++++++++++++++----- src/validation.cpp | 18 ++++++++++++++++++ src/validation.h | 14 ++++++++++++++ 5 files changed, 95 insertions(+), 11 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 98a8bad102..8474d01c41 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -79,9 +79,15 @@ BaseIndex::~BaseIndex() bool BaseIndex::Init() { + AssertLockNotHeld(cs_main); + + // May need reset if index is being restarted. + m_interrupt.reset(); + // 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(); + m_chainstate = WITH_LOCK(::cs_main, + return &m_chain->context()->chainman->GetChainstateForIndexing()); // Register to validation interface before setting the 'm_synced' flag, so that // callbacks are not missed once m_synced is true. RegisterValidationInterface(this); @@ -92,7 +98,8 @@ bool BaseIndex::Init() } LOCK(cs_main); - CChain& active_chain = m_chainstate->m_chain; + CChain& index_chain = m_chainstate->m_chain; + if (locator.IsNull()) { SetBestBlockIndex(nullptr); } else { @@ -114,7 +121,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 = start_block == active_chain.Tip(); + m_synced = start_block == index_chain.Tip(); m_init = true; return true; } @@ -143,6 +150,8 @@ void BaseIndex::ThreadSync() std::chrono::steady_clock::time_point last_locator_write_time{0s}; while (true) { if (m_interrupt) { + LogPrintf("%s: m_interrupt set; exiting ThreadSync\n", GetName()); + SetBestBlockIndex(pindex); // No need to handle errors in Commit. If it fails, the error will be already be // logged. The best way to recover is to continue, as index cannot be corrupted by @@ -252,6 +261,17 @@ bool BaseIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_ti void BaseIndex::BlockConnected(ChainstateRole role, const std::shared_ptr& block, const CBlockIndex* pindex) { + // Ignore events from the assumed-valid chain; we will process its blocks + // (sequentially) after it is fully verified by the background chainstate. This + // is to avoid any out-of-order indexing. + // + // TODO at some point we could parameterize whether a particular index can be + // built out of order, but for now just do the conservative simple thing. + if (role == ChainstateRole::ASSUMEDVALID) { + return; + } + + // Ignore BlockConnected signals until we have fully indexed the chain. if (!m_synced) { return; } @@ -298,6 +318,12 @@ void BaseIndex::BlockConnected(ChainstateRole role, const std::shared_ptr(node.kernel->interrupt, chainman_opts, blockman_opts); ChainstateManager& chainman = *node.chainman; + // This is defined and set here instead of inline in validation.h to avoid a hard + // dependency between validation and index/base, since the latter is not in + // libbitcoinkernel. + chainman.restart_indexes = [&node]() { + LogPrintf("[snapshot] restarting indexes\n"); + + // Drain the validation interface queue to ensure that the old indexes + // don't have any pending work. + SyncWithValidationInterfaceQueue(); + + for (auto* index : node.indexes) { + index->Interrupt(); + index->Stop(); + if (!(index->Init() && index->StartBackgroundSync())) { + LogPrintf("[snapshot] WARNING failed to restart index %s on snapshot chain\n", index->GetName()); + } + } + }; + node::ChainstateLoadOptions options; options.mempool = Assert(node.mempool.get()); options.reindex = node::fReindex; @@ -1906,18 +1925,19 @@ bool StartIndexBackgroundSync(NodeContext& node) // indexes_start_block='nullptr' means "start from height 0". std::optional indexes_start_block; std::string older_index_name; - ChainstateManager& chainman = *Assert(node.chainman); + const Chainstate& chainstate = WITH_LOCK(::cs_main, return chainman.GetChainstateForIndexing()); + const CChain& index_chain = chainstate.m_chain; + 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 (!index_chain.Contains(pindex)) { + pindex = index_chain.FindFork(pindex); } if (!indexes_start_block || !pindex || pindex->nHeight < indexes_start_block.value()->nHeight) { @@ -1932,7 +1952,7 @@ bool StartIndexBackgroundSync(NodeContext& node) 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))) { + if (!chainman.m_blockman.CheckBlockDataAvailability(*index_chain.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)); } } diff --git a/src/validation.cpp b/src/validation.cpp index 8c657839e8..00da4a1c9e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3289,6 +3289,16 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< if (WITH_LOCK(::cs_main, return m_disabled)) { // Background chainstate has reached the snapshot base block, so exit. + + // Restart indexes to resume indexing for all blocks unique to the snapshot + // chain. This resumes indexing "in order" from where the indexing on the + // background validation chain left off. + // + // This cannot be done while holding cs_main (within + // MaybeCompleteSnapshotValidation) or a cs_main deadlock will occur. + if (m_chainman.restart_indexes) { + m_chainman.restart_indexes(); + } break; } @@ -5921,3 +5931,11 @@ bool ChainstateManager::ValidatedSnapshotCleanup() } return true; } + +Chainstate& ChainstateManager::GetChainstateForIndexing() +{ + // We can't always return `m_ibd_chainstate` because after background validation + // has completed, `m_snapshot_chainstate == m_active_chainstate`, but it can be + // indexed. + return (this->GetAll().size() > 1) ? *m_ibd_chainstate : *m_active_chainstate; +} diff --git a/src/validation.h b/src/validation.h index 38f57ed1b5..b46b55b650 100644 --- a/src/validation.h +++ b/src/validation.h @@ -905,6 +905,10 @@ public: explicit ChainstateManager(const util::SignalInterrupt& interrupt, Options options, node::BlockManager::Options blockman_options); + //! Function to restart active indexes; set dynamically to avoid a circular + //! dependency on `base/index.cpp`. + std::function restart_indexes = std::function(); + const CChainParams& GetParams() const { return m_options.chainparams; } const Consensus::Params& GetConsensus() const { return m_options.chainparams.GetConsensus(); } bool ShouldCheckBlockIndex() const { return *Assert(m_options.check_block_index); } @@ -1227,6 +1231,16 @@ public: //! @sa node/chainstate:LoadChainstate() bool ValidatedSnapshotCleanup() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + //! @returns the chainstate that indexes should consult when ensuring that an + //! index is synced with a chain where we can expect block index entries to have + //! BLOCK_HAVE_DATA beneath the tip. + //! + //! In other words, give us the chainstate for which we can reasonably expect + //! that all blocks beneath the tip have been indexed. In practice this means + //! when using an assumed-valid chainstate based upon a snapshot, return only the + //! fully validated chain. + Chainstate& GetChainstateForIndexing() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + ~ChainstateManager(); };