From bcbd7eb8d40fbbd0e58c61acef087d65f2047036 Mon Sep 17 00:00:00 2001 From: furszy Date: Sat, 25 Nov 2023 12:10:52 -0300 Subject: [PATCH 1/6] bench: basic block filter index initial sync Introduce benchmark for the block filter index sync. And makes synchronous 'Sync()' mechanism accessible. --- src/Makefile.bench.include | 1 + src/bench/index_blockfilter.cpp | 43 +++++++++++++++++++++++++++++++++ src/index/base.cpp | 4 +-- src/index/base.h | 16 ++++++------ 4 files changed, 54 insertions(+), 10 deletions(-) create mode 100644 src/bench/index_blockfilter.cpp diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 4d814bc5dc92b..7ba0111fa686f 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -34,6 +34,7 @@ bench_bench_bitcoin_SOURCES = \ bench/examples.cpp \ bench/gcs_filter.cpp \ bench/hashpadding.cpp \ + bench/index_blockfilter.cpp \ bench/load_external.cpp \ bench/lockedpool.cpp \ bench/logging.cpp \ diff --git a/src/bench/index_blockfilter.cpp b/src/bench/index_blockfilter.cpp new file mode 100644 index 0000000000000..5e0bfbfea6b3b --- /dev/null +++ b/src/bench/index_blockfilter.cpp @@ -0,0 +1,43 @@ +// Copyright (c) 2023-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include +#include +#include + +// Very simple block filter index sync benchmark, only using coinbase outputs. +static void BlockFilterIndexSync(benchmark::Bench& bench) +{ + const auto test_setup = MakeNoLogFileContext(); + + // Create more blocks + int CHAIN_SIZE = 600; + CPubKey pubkey{ParseHex("02ed26169896db86ced4cbb7b3ecef9859b5952825adbeab998fb5b307e54949c9")}; + CScript script = GetScriptForDestination(WitnessV0KeyHash(pubkey)); + std::vector noTxns; + for (int i = 0; i < CHAIN_SIZE - 100; i++) { + test_setup->CreateAndProcessBlock(noTxns, script); + SetMockTime(GetTime() + 1); + } + assert(WITH_LOCK(::cs_main, return test_setup->m_node.chainman->ActiveHeight() == CHAIN_SIZE)); + + bench.minEpochIterations(5).run([&] { + BlockFilterIndex filter_index(interfaces::MakeChain(test_setup->m_node), BlockFilterType::BASIC, + /*n_cache_size=*/0, /*f_memory=*/false, /*f_wipe=*/true); + assert(filter_index.Init()); + assert(!filter_index.BlockUntilSyncedToCurrentChain()); + filter_index.Sync(); + + IndexSummary summary = filter_index.GetSummary(); + assert(summary.synced); + assert(summary.best_block_hash == WITH_LOCK(::cs_main, return test_setup->m_node.chainman->ActiveTip()->GetBlockHash())); + }); +} + +BENCHMARK(BlockFilterIndexSync, benchmark::PriorityLevel::HIGH); diff --git a/src/index/base.cpp b/src/index/base.cpp index 036292cd8a11c..5953fe215004a 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -141,7 +141,7 @@ static const CBlockIndex* NextSyncBlock(const CBlockIndex* pindex_prev, CChain& return chain.Next(chain.FindFork(pindex_prev)); } -void BaseIndex::ThreadSync() +void BaseIndex::Sync() { const CBlockIndex* pindex = m_best_block_index.load(); if (!m_synced) { @@ -394,7 +394,7 @@ bool BaseIndex::StartBackgroundSync() { if (!m_init) throw std::logic_error("Error: Cannot start a non-initialized index"); - m_thread_sync = std::thread(&util::TraceThread, GetName(), [this] { ThreadSync(); }); + m_thread_sync = std::thread(&util::TraceThread, GetName(), [this] { Sync(); }); return true; } diff --git a/src/index/base.h b/src/index/base.h index 154061fb19120..0eb1d9ca3b229 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -78,13 +78,6 @@ class BaseIndex : public CValidationInterface std::thread m_thread_sync; CThreadInterrupt m_interrupt; - /// 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 - /// flag is set and the BlockConnected ValidationInterface callback takes - /// over and the sync thread exits. - void ThreadSync(); - /// Write the current index state (eg. chain block locator and subclass-specific items) to disk. /// /// Recommendations for error handling: @@ -152,9 +145,16 @@ class BaseIndex : public CValidationInterface /// validation interface so that it stays in sync with blockchain updates. [[nodiscard]] bool Init(); - /// Starts the initial sync process. + /// Starts the initial sync process on a background thread. [[nodiscard]] bool StartBackgroundSync(); + /// 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 + /// flag is set and the BlockConnected ValidationInterface callback takes + /// over and the sync thread exits. + void Sync(); + /// Stops the instance from staying in sync with blockchain updates. void Stop(); From 331f044e3b49223cedd16803d123c0da9d91d6a2 Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 24 Jan 2023 20:49:42 -0300 Subject: [PATCH 2/6] index: blockfilter, decouple Write into its own function --- src/index/blockfilterindex.cpp | 11 ++++++++--- src/index/blockfilterindex.h | 2 ++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index 65993e830e54e..1085b4da77dab 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -252,16 +252,21 @@ bool BlockFilterIndex::CustomAppend(const interfaces::BlockInfo& block) BlockFilter filter(m_filter_type, *Assert(block.data), block_undo); + return Write(filter, block.height, filter.ComputeHeader(prev_header)); +} + +bool BlockFilterIndex::Write(const BlockFilter& filter, uint32_t block_height, const uint256& filter_header) +{ size_t bytes_written = WriteFilterToDisk(m_next_filter_pos, filter); if (bytes_written == 0) return false; std::pair value; - value.first = block.hash; + value.first = filter.GetBlockHash(); value.second.hash = filter.GetHash(); - value.second.header = filter.ComputeHeader(prev_header); + value.second.header = filter_header; value.second.pos = m_next_filter_pos; - if (!m_db->Write(DBHeightKey(block.height), value)) { + if (!m_db->Write(DBHeightKey(block_height), value)) { return false; } diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h index 10a1cfd2ee0e5..1cfc21d00f4ff 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -44,6 +44,8 @@ class BlockFilterIndex final : public BaseIndex bool AllowPrune() const override { return true; } + bool Write(const BlockFilter& filter, uint32_t block_height, const uint256& filter_header); + protected: bool CustomInit(const std::optional& block) override; From a6756ecdb2f1ac960433412807aa377d1ee80d05 Mon Sep 17 00:00:00 2001 From: furszy Date: Mon, 30 Jan 2023 17:51:16 -0300 Subject: [PATCH 3/6] index: blockfilter, decouple header lookup into its own function --- src/index/blockfilterindex.cpp | 32 +++++++++++++++++++------------- src/index/blockfilterindex.h | 2 ++ 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index 1085b4da77dab..204e5d7e18b49 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -222,6 +222,22 @@ size_t BlockFilterIndex::WriteFilterToDisk(FlatFilePos& pos, const BlockFilter& return data_size; } +std::optional BlockFilterIndex::ReadFilterHeader(int height, const uint256& expected_block_hash) +{ + std::pair read_out; + if (!m_db->Read(DBHeightKey(height), read_out)) { + return std::nullopt; + } + + if (read_out.first != expected_block_hash) { + LogError("%s: previous block header belongs to unexpected block %s; expected %s\n", + __func__, read_out.first.ToString(), expected_block_hash.ToString()); + return std::nullopt; + } + + return read_out.second.header; +} + bool BlockFilterIndex::CustomAppend(const interfaces::BlockInfo& block) { CBlockUndo block_undo; @@ -235,19 +251,9 @@ bool BlockFilterIndex::CustomAppend(const interfaces::BlockInfo& block) return false; } - std::pair read_out; - if (!m_db->Read(DBHeightKey(block.height - 1), read_out)) { - return false; - } - - uint256 expected_block_hash = *Assert(block.prev_hash); - if (read_out.first != expected_block_hash) { - LogError("%s: previous block header belongs to unexpected block %s; expected %s\n", - __func__, read_out.first.ToString(), expected_block_hash.ToString()); - return false; - } - - prev_header = read_out.second.header; + auto op_prev_header = ReadFilterHeader(block.height - 1, *Assert(block.prev_hash)); + if (!op_prev_header) return false; + prev_header = *op_prev_header; } BlockFilter filter(m_filter_type, *Assert(block.data), block_undo); diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h index 1cfc21d00f4ff..01ba1025c87d5 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -46,6 +46,8 @@ class BlockFilterIndex final : public BaseIndex bool Write(const BlockFilter& filter, uint32_t block_height, const uint256& filter_header); + std::optional ReadFilterHeader(int height, const uint256& expected_block_hash); + protected: bool CustomInit(const std::optional& block) override; From f1469eb45469672046c5793b44863f606736c853 Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 23 Feb 2024 16:31:13 -0300 Subject: [PATCH 4/6] index: cache last block filter header Avoid disk read operations on every new processed block. --- src/index/blockfilterindex.cpp | 22 ++++++++++++++++------ src/index/blockfilterindex.h | 3 +++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index 204e5d7e18b49..41bdca9df562a 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -128,6 +128,16 @@ bool BlockFilterIndex::CustomInit(const std::optional& blo m_next_filter_pos.nFile = 0; m_next_filter_pos.nPos = 0; } + + if (block) { + auto op_last_header = ReadFilterHeader(block->height, block->hash); + if (!op_last_header) { + LogError("Cannot read last block filter header; index may be corrupted\n"); + return false; + } + m_last_header = *op_last_header; + } + return true; } @@ -241,7 +251,6 @@ std::optional BlockFilterIndex::ReadFilterHeader(int height, const uint bool BlockFilterIndex::CustomAppend(const interfaces::BlockInfo& block) { CBlockUndo block_undo; - uint256 prev_header; if (block.height > 0) { // pindex variable gives indexing code access to node internals. It @@ -250,15 +259,14 @@ bool BlockFilterIndex::CustomAppend(const interfaces::BlockInfo& block) if (!m_chainstate->m_blockman.UndoReadFromDisk(block_undo, *pindex)) { return false; } - - auto op_prev_header = ReadFilterHeader(block.height - 1, *Assert(block.prev_hash)); - if (!op_prev_header) return false; - prev_header = *op_prev_header; } BlockFilter filter(m_filter_type, *Assert(block.data), block_undo); - return Write(filter, block.height, filter.ComputeHeader(prev_header)); + const uint256& header = filter.ComputeHeader(m_last_header); + bool res = Write(filter, block.height, header); + if (res) m_last_header = header; // update last header + return res; } bool BlockFilterIndex::Write(const BlockFilter& filter, uint32_t block_height, const uint256& filter_header) @@ -326,6 +334,8 @@ bool BlockFilterIndex::CustomRewind(const interfaces::BlockKey& current_tip, con batch.Write(DB_FILTER_POS, m_next_filter_pos); if (!m_db->WriteBatch(batch)) return false; + // Update cached header + m_last_header = *Assert(ReadFilterHeader(new_tip.height, new_tip.hash)); return true; } diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h index 01ba1025c87d5..cdb9563fb8ec8 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -42,6 +42,9 @@ class BlockFilterIndex final : public BaseIndex /** cache of block hash to filter header, to avoid disk access when responding to getcfcheckpt. */ std::unordered_map m_headers_cache GUARDED_BY(m_cs_headers_cache); + // Last computed header to avoid disk reads on every new block. + uint256 m_last_header{}; + bool AllowPrune() const override { return true; } bool Write(const BlockFilter& filter, uint32_t block_height, const uint256& filter_header); From 0faafb57f8298547949cbc0044ee9e925ed887ba Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 16 Feb 2023 17:25:00 -0300 Subject: [PATCH 5/6] index: decrease ThreadSync cs_main contention Only NextSyncBlock requires cs_main lock. The other function calls like Commit or Rewind will lock or not cs_main internally when they need it. Avoiding keeping cs_main locked when Commit() or Rewind() write data to disk. --- src/index/base.cpp | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 5953fe215004a..b4bda2fca6ac8 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -159,23 +159,20 @@ void BaseIndex::Sync() return; } - { - LOCK(cs_main); - const CBlockIndex* pindex_next = NextSyncBlock(pindex, m_chainstate->m_chain); - if (!pindex_next) { - SetBestBlockIndex(pindex); - m_synced = true; - // No need to handle errors in Commit. See rationale above. - Commit(); - break; - } - if (pindex_next->pprev != pindex && !Rewind(pindex, pindex_next->pprev)) { - FatalErrorf("%s: Failed to rewind index %s to a previous chain tip", - __func__, GetName()); - return; - } - pindex = pindex_next; + const CBlockIndex* pindex_next = WITH_LOCK(cs_main, return NextSyncBlock(pindex, m_chainstate->m_chain)); + if (!pindex_next) { + SetBestBlockIndex(pindex); + m_synced = true; + // No need to handle errors in Commit. See rationale above. + Commit(); + break; } + if (pindex_next->pprev != pindex && !Rewind(pindex, pindex_next->pprev)) { + FatalErrorf("%s: Failed to rewind index %s to a previous chain tip", __func__, GetName()); + return; + } + pindex = pindex_next; + auto current_time{std::chrono::steady_clock::now()}; if (last_log_time + SYNC_LOG_INTERVAL < current_time) { From 99afb9d15a08d2f46739f4d2b66c63dbabd7a44e Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 28 Nov 2023 09:02:00 -0300 Subject: [PATCH 6/6] refactor: init, simplify index shutdown code --- src/init.cpp | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index b9a0bb732aa04..251ef0f8ff33f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -256,12 +256,8 @@ void Interrupt(NodeContext& node) InterruptMapPort(); if (node.connman) node.connman->Interrupt(); - if (g_txindex) { - g_txindex->Interrupt(); - } - ForEachBlockFilterIndex([](BlockFilterIndex& index) { index.Interrupt(); }); - if (g_coin_stats_index) { - g_coin_stats_index->Interrupt(); + for (auto* index : node.indexes) { + index->Interrupt(); } } @@ -337,16 +333,11 @@ void Shutdown(NodeContext& node) if (node.validation_signals) node.validation_signals->FlushBackgroundCallbacks(); // Stop and delete all indexes only after flushing background callbacks. - if (g_txindex) { - g_txindex->Stop(); - g_txindex.reset(); - } - if (g_coin_stats_index) { - g_coin_stats_index->Stop(); - g_coin_stats_index.reset(); - } - ForEachBlockFilterIndex([](BlockFilterIndex& index) { index.Stop(); }); + for (auto* index : node.indexes) index->Stop(); + if (g_txindex) g_txindex.reset(); + if (g_coin_stats_index) g_coin_stats_index.reset(); DestroyAllBlockFilterIndexes(); + node.indexes.clear(); // all instances are nullptr now // Any future callbacks will be dropped. This should absolutely be safe - if // missing a callback results in an unrecoverable situation, unclean shutdown