From 0cdddeb2240d1f33c8b2dd28bb0c9d84d9420e3d 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 BlockManager constructor Make the block db open RAII style by calling it in the BlockManager constructor. 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/init.cpp | 9 +++++++++ src/node/blockstorage.cpp | 15 ++++++++++++++- src/node/blockstorage.h | 2 -- src/node/chainstate.cpp | 23 ----------------------- src/test/util/setup_common.cpp | 2 -- 5 files changed, 23 insertions(+), 28 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index a306449dbd..3fed590dfb 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1215,12 +1215,21 @@ static ChainstateLoadResult InitAndLoadChainstate( }, }; 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", 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. diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 07878a5602..f26b0e7380 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/blockstorage.h b/src/node/blockstorage.h index 9db80c26c5..ac6db0558d 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -268,8 +268,6 @@ public: using Options = kernel::BlockManagerOpts; explicit BlockManager(const util::SignalInterrupt& interrupt, Options opts); - auto MakeBlockTreeDb() { return std::make_unique(m_opts.block_tree_db_params); } - auto OptsWipeBlockTreeDb() { return m_opts.block_tree_db_params.wipe_data; } const util::SignalInterrupt& m_interrupt; std::atomic m_importing{false}; diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 4fc7a851cd..c88bd5bad2 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; @@ -38,26 +35,6 @@ static ChainstateLoadResult CompleteChainstateInitialization( ChainstateManager& chainman, 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 = chainman.m_blockman.MakeBlockTreeDb(); - } catch (dbwrapper_error& err) { - LogError("%s\n", err.what()); - return {ChainstateLoadStatus::FAILURE, _("Error opening block database")}; - } - - if (chainman.m_blockman.OptsWipeBlockTreeDb()) { - 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 diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 83b78d7b5a..30db579f0b 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -257,8 +257,6 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts) }, }; 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 = m_node.chainman->m_blockman.MakeBlockTreeDb(); }; m_make_chainman(); }