From f3cfdf2b511776211a0399510ced735e556d510d Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Mon, 23 Sep 2024 00:21:43 +0200 Subject: [PATCH] kernel: Move block tree db open to block manager Before this change the block tree db was needlessly re-opened during startup when loading a completed snapshot. Improve this by letting the block manager open it on construction. This also simplifies the test code a bit. The change was initially motivated to make it easier for users of the kernel library to instantiate a BlockManager that may be used to read data from disk without loading the block index into a cache. --- src/bitcoin-chainstate.cpp | 6 +++- src/init.cpp | 20 ++++++++++- src/kernel/blockmanager_opts.h | 2 ++ src/kernel/chainstatemanager_opts.h | 1 - src/node/blockmanager_args.cpp | 3 ++ src/node/blockstorage.cpp | 15 ++++++++- src/node/chainstate.cpp | 33 ++----------------- src/node/chainstate.h | 5 --- src/node/chainstatemanager_args.cpp | 1 - src/test/blockmanager_tests.cpp | 8 +++++ src/test/util/setup_common.cpp | 14 ++++---- .../validation_chainstatemanager_tests.cpp | 5 +++ 12 files changed, 64 insertions(+), 49 deletions(-) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 51c482607aa8f..e657f01871568 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -106,6 +106,7 @@ int main(int argc, char* argv[]) }; auto notifications = std::make_unique(); + kernel::CacheSizes cache_sizes{DEFAULT_KERNEL_CACHE}; // SETUP: Chainstate auto chainparams = CChainParams::Main(); @@ -119,11 +120,14 @@ int main(int argc, char* argv[]) .chainparams = chainman_opts.chainparams, .blocks_dir = abs_datadir / "blocks", .notifications = chainman_opts.notifications, + .block_tree_db_params = DBParams{ + .path = abs_datadir / "blocks" / "index", + .cache_bytes = cache_sizes.block_tree_db, + }, }; util::SignalInterrupt interrupt; ChainstateManager chainman{interrupt, chainman_opts, blockman_opts}; - kernel::CacheSizes cache_sizes{DEFAULT_KERNEL_CACHE}; node::ChainstateLoadOptions options; auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options); if (status != node::ChainstateLoadStatus::SUCCESS) { diff --git a/src/init.cpp b/src/init.cpp index 116823c36f079..2d61e395106b4 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1057,6 +1057,10 @@ bool AppInitParameterInteraction(const ArgsManager& args) .chainparams = chainman_opts_dummy.chainparams, .blocks_dir = args.GetBlocksDirPath(), .notifications = chainman_opts_dummy.notifications, + .block_tree_db_params = DBParams{ + .path = args.GetDataDirNet() / "blocks" / "index", + .cache_bytes = 0, + }, }; auto blockman_result{ApplyArgsManOptions(args, blockman_opts_dummy)}; if (!blockman_result) { @@ -1199,18 +1203,33 @@ static ChainstateLoadResult InitAndLoadChainstate( .signals = node.validation_signals.get(), }; Assert(ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction + BlockManager::Options blockman_opts{ .chainparams = chainman_opts.chainparams, .blocks_dir = args.GetBlocksDirPath(), .notifications = chainman_opts.notifications, + .block_tree_db_params = DBParams{ + .path = args.GetDataDirNet() / "blocks" / "index", + .cache_bytes = cache_sizes.block_tree_db, + .wipe_data = do_reindex, + }, }; Assert(ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction + + // Creating the chainstate manager internally creates a BlockManager, opens + // the blocks tree db, and wipes existing block files in case of a reindex. + // The coinsdb is opened at a later point on LoadChainstate. try { node.chainman = std::make_unique(*Assert(node.shutdown_signal), chainman_opts, blockman_opts); + } catch (dbwrapper_error& e) { + LogError("%s\n", e.what()); + return {ChainstateLoadStatus::FAILURE, _("Error opening block database")}; } catch (std::exception& e) { return {ChainstateLoadStatus::FAILURE_FATAL, Untranslated(strprintf("Failed to initialize ChainstateManager: %s", e.what()))}; } ChainstateManager& chainman = *node.chainman; + if (chainman.m_interrupt) return {ChainstateLoadStatus::INTERRUPTED, {}}; + // 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. @@ -1233,7 +1252,6 @@ static ChainstateLoadResult InitAndLoadChainstate( }; node::ChainstateLoadOptions options; options.mempool = Assert(node.mempool.get()); - options.wipe_block_tree_db = do_reindex; options.wipe_chainstate_db = do_reindex || do_reindex_chainstate; options.prune = chainman.m_blockman.IsPruneMode(); options.check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS); diff --git a/src/kernel/blockmanager_opts.h b/src/kernel/blockmanager_opts.h index 261ec3be5820b..5c7b24d7170a2 100644 --- a/src/kernel/blockmanager_opts.h +++ b/src/kernel/blockmanager_opts.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H #define BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H +#include #include #include @@ -27,6 +28,7 @@ struct BlockManagerOpts { bool fast_prune{false}; const fs::path blocks_dir; Notifications& notifications; + DBParams block_tree_db_params; }; } // namespace kernel diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h index 1b605f3d55dfc..15a8fbec61854 100644 --- a/src/kernel/chainstatemanager_opts.h +++ b/src/kernel/chainstatemanager_opts.h @@ -42,7 +42,6 @@ struct ChainstateManagerOpts { std::optional assumed_valid_block{}; //! If the tip is older than this, the node is considered to be in initial block download. std::chrono::seconds max_tip_age{DEFAULT_MAX_TIP_AGE}; - DBOptions block_tree_db{}; DBOptions coins_db{}; CoinsViewOptions coins_view{}; Notifications& notifications; diff --git a/src/node/blockmanager_args.cpp b/src/node/blockmanager_args.cpp index 0fc4e1646a1c1..5186b65546c64 100644 --- a/src/node/blockmanager_args.cpp +++ b/src/node/blockmanager_args.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -34,6 +35,8 @@ util::Result ApplyArgsManOptions(const ArgsManager& args, BlockManager::Op if (auto value{args.GetBoolArg("-fastprune")}) opts.fast_prune = *value; + ReadDatabaseArgs(args, opts.block_tree_db_params.options); + return {}; } } // namespace node diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 07878a560297c..f26b0e7380081 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -36,6 +36,7 @@ #include #include +#include #include #include #include @@ -1183,7 +1184,19 @@ BlockManager::BlockManager(const util::SignalInterrupt& interrupt, Options opts) m_opts{std::move(opts)}, m_block_file_seq{FlatFileSeq{m_opts.blocks_dir, "blk", m_opts.fast_prune ? 0x4000 /* 16kB */ : BLOCKFILE_CHUNK_SIZE}}, m_undo_file_seq{FlatFileSeq{m_opts.blocks_dir, "rev", UNDOFILE_CHUNK_SIZE}}, - m_interrupt{interrupt} {} + m_interrupt{interrupt} +{ + m_block_tree_db = std::make_unique(m_opts.block_tree_db_params); + + if (m_opts.block_tree_db_params.wipe_data) { + m_block_tree_db->WriteReindexing(true); + m_blockfiles_indexed = false; + // If we're reindexing in prune mode, wipe away unusable block files and all undo data files + if (m_prune_mode) { + CleanupBlockRevFiles(); + } + } +} class ImportingNow { diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 10b6e52c78b0c..c88bd5bad23ab 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -23,10 +23,7 @@ #include #include -#include #include -#include -#include #include using kernel::CacheSizes; @@ -36,34 +33,8 @@ namespace node { // to ChainstateManager::InitializeChainstate(). static ChainstateLoadResult CompleteChainstateInitialization( ChainstateManager& chainman, - const CacheSizes& cache_sizes, const ChainstateLoadOptions& options) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { - auto& pblocktree{chainman.m_blockman.m_block_tree_db}; - // new BlockTreeDB tries to delete the existing file, which - // fails if it's still open from the previous loop. Close it first: - pblocktree.reset(); - try { - pblocktree = std::make_unique(DBParams{ - .path = chainman.m_options.datadir / "blocks" / "index", - .cache_bytes = cache_sizes.block_tree_db, - .memory_only = options.block_tree_db_in_memory, - .wipe_data = options.wipe_block_tree_db, - .options = chainman.m_options.block_tree_db}); - } catch (dbwrapper_error& err) { - LogError("%s\n", err.what()); - return {ChainstateLoadStatus::FAILURE, _("Error opening block database")}; - } - - if (options.wipe_block_tree_db) { - pblocktree->WriteReindexing(true); - chainman.m_blockman.m_blockfiles_indexed = false; - //If we're reindexing in prune mode, wipe away unusable block files and all undo data files - if (options.prune) { - chainman.m_blockman.CleanupBlockRevFiles(); - } - } - if (chainman.m_interrupt) return {ChainstateLoadStatus::INTERRUPTED, {}}; // LoadBlockIndex will load m_have_pruned if we've ever removed a @@ -206,7 +177,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize } } - auto [init_status, init_error] = CompleteChainstateInitialization(chainman, cache_sizes, options); + auto [init_status, init_error] = CompleteChainstateInitialization(chainman, options); if (init_status != ChainstateLoadStatus::SUCCESS) { return {init_status, init_error}; } @@ -242,7 +213,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize // for the fully validated chainstate. chainman.ActiveChainstate().ClearBlockIndexCandidates(); - auto [init_status, init_error] = CompleteChainstateInitialization(chainman, cache_sizes, options); + auto [init_status, init_error] = CompleteChainstateInitialization(chainman, options); if (init_status != ChainstateLoadStatus::SUCCESS) { return {init_status, init_error}; } diff --git a/src/node/chainstate.h b/src/node/chainstate.h index ce414fa043434..843e8e09a66d5 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -22,12 +22,7 @@ namespace node { struct ChainstateLoadOptions { CTxMemPool* mempool{nullptr}; - bool block_tree_db_in_memory{false}; bool coins_db_in_memory{false}; - // Whether to wipe the block tree database when loading it. If set, this - // will also set a reindexing flag so any existing block data files will be - // scanned and added to the database. - bool wipe_block_tree_db{false}; // Whether to wipe the chainstate database when loading it. If set, this // will cause the chainstate database to be rebuilt starting from genesis. bool wipe_chainstate_db{false}; diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp index 0ac96c55149a7..db36d03fd5c4a 100644 --- a/src/node/chainstatemanager_args.cpp +++ b/src/node/chainstatemanager_args.cpp @@ -49,7 +49,6 @@ util::Result ApplyArgsManOptions(const ArgsManager& args, ChainstateManage if (auto value{args.GetIntArg("-maxtipage")}) opts.max_tip_age = std::chrono::seconds{*value}; - ReadDatabaseArgs(args, opts.block_tree_db); ReadDatabaseArgs(args, opts.coins_db); ReadCoinsViewArgs(args, opts.coins_view); diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index c2b95dd861ec6..ec91f2b276ede 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -33,6 +33,10 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) .chainparams = *params, .blocks_dir = m_args.GetBlocksDirPath(), .notifications = notifications, + .block_tree_db_params = DBParams{ + .path = m_args.GetDataDirNet() / "blocks" / "index", + .cache_bytes = 0, + }, }; BlockManager blockman{*Assert(m_node.shutdown_signal), blockman_opts}; // simulate adding a genesis block normally @@ -140,6 +144,10 @@ BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) .chainparams = Params(), .blocks_dir = m_args.GetBlocksDirPath(), .notifications = notifications, + .block_tree_db_params = DBParams{ + .path = m_args.GetDataDirNet() / "blocks" / "index", + .cache_bytes = 0, + }, }; BlockManager blockman{*Assert(m_node.shutdown_signal), blockman_opts}; diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 2cf25c4a2ad13..9fa06be3cad36 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -62,7 +62,6 @@ #include using namespace util::hex_literals; -using kernel::BlockTreeDB; using node::ApplyArgsManOptions; using node::BlockAssembler; using node::BlockManager; @@ -250,14 +249,15 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts) .chainparams = chainman_opts.chainparams, .blocks_dir = m_args.GetBlocksDirPath(), .notifications = chainman_opts.notifications, + .block_tree_db_params = DBParams{ + .path = m_args.GetDataDirNet() / "blocks" / "index", + .cache_bytes = m_kernel_cache_sizes.block_tree_db, + .memory_only = opts.block_tree_db_in_memory, + .wipe_data = m_args.GetBoolArg("-reindex", false), + }, }; m_node.chainman = std::make_unique(*Assert(m_node.shutdown_signal), chainman_opts, blockman_opts); LOCK(m_node.chainman->GetMutex()); - m_node.chainman->m_blockman.m_block_tree_db = std::make_unique(DBParams{ - .path = m_args.GetDataDirNet() / "blocks" / "index", - .cache_bytes = m_kernel_cache_sizes.block_tree_db, - .memory_only = true, - }); }; m_make_chainman(); } @@ -283,9 +283,7 @@ void ChainTestingSetup::LoadVerifyActivateChainstate() auto& chainman{*Assert(m_node.chainman)}; node::ChainstateLoadOptions options; options.mempool = Assert(m_node.mempool.get()); - options.block_tree_db_in_memory = m_block_tree_db_in_memory; options.coins_db_in_memory = m_coins_db_in_memory; - options.wipe_block_tree_db = m_args.GetBoolArg("-reindex", false); options.wipe_chainstate_db = m_args.GetBoolArg("-reindex", false) || m_args.GetBoolArg("-reindex-chainstate", false); options.prune = chainman.m_blockman.IsPruneMode(); options.check_blocks = m_args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS); diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 6c2a825e6477f..bf440ca639753 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -393,6 +393,11 @@ struct SnapshotTestSetup : TestChain100Setup { .chainparams = chainman_opts.chainparams, .blocks_dir = m_args.GetBlocksDirPath(), .notifications = chainman_opts.notifications, + .block_tree_db_params = DBParams{ + .path = chainman.m_options.datadir / "blocks" / "index", + .cache_bytes = m_kernel_cache_sizes.block_tree_db, + .memory_only = m_block_tree_db_in_memory, + }, }; // For robustness, ensure the old manager is destroyed before creating a // new one.