Page MenuHomePhabricator

D17894.diff
No OneTemporary

D17894.diff

diff --git a/chronik/chronik-cpp/chronik_bridge.cpp b/chronik/chronik-cpp/chronik_bridge.cpp
--- a/chronik/chronik-cpp/chronik_bridge.cpp
+++ b/chronik/chronik-cpp/chronik_bridge.cpp
@@ -204,8 +204,9 @@
ChronikBridge::lookup_block_index_by_height(int height) const {
// The boundary check is performed in the CChain::operator[](int nHeight)
// method, a nullptr is returned if height is out of bounds.
- const CBlockIndex *pindex =
- WITH_LOCK(cs_main, return m_node.chainman->ActiveChain()[height]);
+ const CBlockIndex *pindex = WITH_LOCK(
+ cs_main,
+ return m_node.chainman->GetChainstateForIndexing().m_chain[height]);
if (!pindex) {
throw block_index_not_found();
}
@@ -220,7 +221,8 @@
LOCK(cs_main);
std::vector<RawBlockHeader> headers;
for (int height = start; height <= end; height++) {
- const CBlockIndex *pindex = m_node.chainman->ActiveChain()[height];
+ const CBlockIndex *pindex =
+ m_node.chainman->GetChainstateForIndexing().m_chain[height];
if (!pindex) {
// We allow partial results or empty result.
// We can assume that if a block height does not exist the following
@@ -240,7 +242,8 @@
LOCK(cs_main);
std::vector<WrappedBlockHash> block_hashes;
for (int height = start; height <= end; height++) {
- const CBlockIndex *pindex = m_node.chainman->ActiveChain()[height];
+ const CBlockIndex *pindex =
+ m_node.chainman->GetChainstateForIndexing().m_chain[height];
if (!pindex) {
throw block_index_not_found();
}
@@ -308,7 +311,8 @@
const CBlockIndex &ChronikBridge::find_fork(const CBlockIndex &index) const {
const CBlockIndex *fork = WITH_LOCK(
cs_main,
- return m_node.chainman->ActiveChainstate().m_chain.FindFork(&index));
+ return m_node.chainman->GetChainstateForIndexing().m_chain.FindFork(
+ &index));
if (!fork) {
throw block_index_not_found();
}
@@ -322,7 +326,7 @@
coins_to_uncache.clear();
LOCK(cs_main);
CCoinsViewCache &coins_cache =
- m_node.chainman->ActiveChainstate().CoinsTip();
+ m_node.chainman->GetChainstateForIndexing().CoinsTip();
CCoinsViewMemPool coin_view(&coins_cache, *m_node.mempool);
for (TxInput &input : tx.inputs) {
TxId txid = TxId(chronik::util::ArrayToHash(input.prev_out.txid));
@@ -352,7 +356,7 @@
rust::Slice<const OutPoint> coins_to_uncache) const {
LOCK(cs_main);
CCoinsViewCache &coins_cache =
- m_node.chainman->ActiveChainstate().CoinsTip();
+ m_node.chainman->GetChainstateForIndexing().CoinsTip();
for (const OutPoint &outpoint : coins_to_uncache) {
TxId txid = TxId(chronik::util::ArrayToHash(outpoint.txid));
coins_cache.Uncache(COutPoint(txid, outpoint.out_idx));
diff --git a/chronik/chronik-cpp/chronik_validationinterface.cpp b/chronik/chronik-cpp/chronik_validationinterface.cpp
--- a/chronik/chronik-cpp/chronik_validationinterface.cpp
+++ b/chronik/chronik-cpp/chronik_validationinterface.cpp
@@ -49,6 +49,13 @@
void BlockConnected(ChainstateRole role,
const std::shared_ptr<const CBlock> &block,
const CBlockIndex *pindex) override {
+ // Ignore events from the assumed-valid chain; we will process its
+ // blocks (sequentially) after it is fully verified by the background
+ // chainstate.
+ if (role == ChainstateRole::ASSUMEDVALID) {
+ return;
+ }
+
// We can safely pass T& here as Rust guarantees us that no references
// can be kept after the below function call completed.
m_chronik->handle_block_connected(*block, *pindex);
diff --git a/src/index/base.h b/src/index/base.h
--- a/src/index/base.h
+++ b/src/index/base.h
@@ -15,6 +15,7 @@
class CBlock;
class CBlockIndex;
class Chainstate;
+class ChainstateManager;
struct IndexSummary {
std::string name;
@@ -27,6 +28,11 @@
* Base class for indices of blockchain data. This implements
* CValidationInterface and ensures blocks are indexed sequentially according
* to their position in the active chain.
+ *
+ * In the presence of multiple chainstates (i.e. if a UTXO snapshot is loaded),
+ * only the background "IBD" chainstate will be indexed to avoid building the
+ * index out of order. When the background chainstate completes validation, the
+ * index will be reinitialized and indexing will continue.
*/
class BaseIndex : public CValidationInterface {
protected:
@@ -122,9 +128,6 @@
virtual DB &GetDB() const = 0;
- /// Get the name of the index for display in logs.
- const std::string &GetName() const LIFETIMEBOUND { return m_name; }
-
/// Update the internal best block index as well as the prune lock.
void SetBestBlockIndex(const CBlockIndex *block);
@@ -133,6 +136,9 @@
/// Destructor interrupts sync thread if running and blocks until it exits.
virtual ~BaseIndex();
+ /// Get the name of the index for display in logs.
+ const std::string &GetName() const LIFETIMEBOUND { return m_name; }
+
/// Blocks the current thread until the index is caught up to the current
/// state of the block chain. This only blocks if the index has gotten in
/// sync once and only needs to process blocks in the ValidationInterface
diff --git a/src/index/base.cpp b/src/index/base.cpp
--- a/src/index/base.cpp
+++ b/src/index/base.cpp
@@ -73,9 +73,11 @@
}
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();
+ AssertLockNotHeld(cs_main);
+
+ // May need reset if index is being restarted.
+ m_interrupt.reset();
+
// Register to validation interface before setting the 'm_synced' flag, so
// that callbacks are not missed once m_synced is true.
RegisterValidationInterface(this);
@@ -86,7 +88,11 @@
}
LOCK(cs_main);
- CChain &active_chain = m_chainstate->m_chain;
+ // 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->GetChainstateForIndexing();
+ CChain &index_chain = m_chainstate->m_chain;
+
if (locator.IsNull()) {
SetBestBlockIndex(nullptr);
} else {
@@ -116,7 +122,7 @@
// 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;
}
@@ -145,6 +151,9 @@
int64_t last_locator_write_time = 0;
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
@@ -254,6 +263,17 @@
void BaseIndex::BlockConnected(ChainstateRole role,
const std::shared_ptr<const CBlock> &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;
}
@@ -305,6 +325,12 @@
void BaseIndex::ChainStateFlushed(ChainstateRole role,
const CBlockLocator &locator) {
+ // Ignore events from the assumed-valid chain; we will process its blocks
+ // (sequentially) after it is fully verified by the background chainstate.
+ if (role == ChainstateRole::ASSUMEDVALID) {
+ return;
+ }
+
if (!m_synced) {
return;
}
diff --git a/src/init.cpp b/src/init.cpp
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -2508,6 +2508,27 @@
std::make_unique<ChainstateManager>(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;
@@ -3045,8 +3066,11 @@
// 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);
+ 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) {
@@ -3056,11 +3080,10 @@
// 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(
BlockHash{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 ||
@@ -3082,7 +3105,7 @@
start_block = chainman.ActiveChain().Genesis();
}
if (!chainman.m_blockman.CheckBlockDataAvailability(
- *chainman.ActiveChain().Tip(), *Assert(start_block))) {
+ *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 "
diff --git a/src/validation.h b/src/validation.h
--- a/src/validation.h
+++ b/src/validation.h
@@ -1229,6 +1229,10 @@
explicit ChainstateManager(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<void()> restart_indexes = std::function<void()>();
+
const Config &GetConfig() const { return m_options.config; }
const CChainParams &GetParams() const {
@@ -1609,6 +1613,16 @@
//! @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);
+
/** Dump the recent block headers reception time to a file. */
bool DumpRecentHeadersTime(const fs::path &filePath) const
EXCLUSIVE_LOCKS_REQUIRED(GetMutex());
diff --git a/src/validation.cpp b/src/validation.cpp
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3648,6 +3648,17 @@
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;
}
@@ -7212,3 +7223,11 @@
}
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;
+}

File Metadata

Mime Type
text/plain
Expires
Tue, May 20, 23:34 (23 m, 38 s)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5866154
Default Alt Text
D17894.diff (13 KB)

Event Timeline