From fa721f1cab0de239a93f6bf70e3a8af26fddae8a Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 3 Jan 2023 13:12:45 +0100 Subject: Move ::nPruneTarget into BlockManager --- src/Makefile.am | 3 +++ src/bitcoin-chainstate.cpp | 2 +- src/init.cpp | 27 +++++++-------------- src/kernel/blockmanager_opts.h | 20 ++++++++++++++++ src/node/blockmanager_args.cpp | 32 +++++++++++++++++++++++++ src/node/blockmanager_args.h | 20 ++++++++++++++++ src/node/blockstorage.cpp | 15 ++++++------ src/node/blockstorage.h | 10 ++++++-- src/test/blockmanager_tests.cpp | 2 +- src/test/util/setup_common.cpp | 2 +- src/test/validation_chainstatemanager_tests.cpp | 6 ++--- src/validation.cpp | 4 +++- src/validation.h | 2 +- 13 files changed, 109 insertions(+), 36 deletions(-) create mode 100644 src/kernel/blockmanager_opts.h create mode 100644 src/node/blockmanager_args.cpp create mode 100644 src/node/blockmanager_args.h diff --git a/src/Makefile.am b/src/Makefile.am index a0c1670287..68ec92b27f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -174,6 +174,7 @@ BITCOIN_CORE_H = \ interfaces/ipc.h \ interfaces/node.h \ interfaces/wallet.h \ + kernel/blockmanager_opts.h \ kernel/chain.h \ kernel/chainstatemanager_opts.h \ kernel/checks.h \ @@ -200,6 +201,7 @@ BITCOIN_CORE_H = \ netbase.h \ netgroup.h \ netmessagemaker.h \ + node/blockmanager_args.h \ node/blockstorage.h \ node/caches.h \ node/chainstate.h \ @@ -387,6 +389,7 @@ libbitcoin_node_a_SOURCES = \ net.cpp \ net_processing.cpp \ netgroup.cpp \ + node/blockmanager_args.cpp \ node/blockstorage.cpp \ node/caches.cpp \ node/chainstate.cpp \ diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 423fa79c6f..136904a0c0 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -85,7 +85,7 @@ int main(int argc, char* argv[]) .datadir = gArgs.GetDataDirNet(), .adjusted_time_callback = NodeClock::now, }; - ChainstateManager chainman{chainman_opts}; + ChainstateManager chainman{chainman_opts, {}}; node::CacheSizes cache_sizes; cache_sizes.block_tree_db = 2 << 20; diff --git a/src/init.cpp b/src/init.cpp index 090a11a825..d68ba80b79 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -124,7 +125,6 @@ using node::ThreadImport; using node::VerifyLoadedChainstate; using node::fPruneMode; using node::fReindex; -using node::nPruneTarget; static constexpr bool DEFAULT_PROXYRANDOMIZE{true}; static constexpr bool DEFAULT_REST_ENABLE{false}; @@ -945,22 +945,6 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb init::SetLoggingCategories(args); init::SetLoggingLevel(args); - // block pruning; get the amount of disk space (in MiB) to allot for block & undo files - int64_t nPruneArg = args.GetIntArg("-prune", 0); - if (nPruneArg < 0) { - return InitError(_("Prune cannot be configured with a negative value.")); - } - nPruneTarget = (uint64_t) nPruneArg * 1024 * 1024; - if (nPruneArg == 1) { // manual pruning: -prune=1 - nPruneTarget = std::numeric_limits::max(); - fPruneMode = true; - } else if (nPruneTarget) { - if (nPruneTarget < MIN_DISK_SPACE_FOR_BLOCK_FILES) { - return InitError(strprintf(_("Prune configured below the minimum of %d MiB. Please use a higher number."), MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024)); - } - fPruneMode = true; - } - nConnectTimeout = args.GetIntArg("-timeout", DEFAULT_CONNECT_TIMEOUT); if (nConnectTimeout <= 0) { nConnectTimeout = DEFAULT_CONNECT_TIMEOUT; @@ -1051,6 +1035,10 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb if (const auto error{ApplyArgsManOptions(args, chainman_opts_dummy)}) { return InitError(*error); } + node::BlockManager::Options blockman_opts_dummy{}; + if (const auto error{ApplyArgsManOptions(args, blockman_opts_dummy)}) { + return InitError(*error); + } } return true; @@ -1450,6 +1438,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) }; Assert(!ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction + node::BlockManager::Options blockman_opts{}; + Assert(!ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction + // cache size calculations CacheSizes cache_sizes = CalculateCacheSizes(args, g_enabled_filter_types.size()); @@ -1485,7 +1476,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) for (bool fLoaded = false; !fLoaded && !ShutdownRequested();) { node.mempool = std::make_unique(mempool_opts); - node.chainman = std::make_unique(chainman_opts); + node.chainman = std::make_unique(chainman_opts, blockman_opts); ChainstateManager& chainman = *node.chainman; node::ChainstateLoadOptions options; diff --git a/src/kernel/blockmanager_opts.h b/src/kernel/blockmanager_opts.h new file mode 100644 index 0000000000..9dc93b6dd2 --- /dev/null +++ b/src/kernel/blockmanager_opts.h @@ -0,0 +1,20 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H +#define BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H + +namespace kernel { + +/** + * An options struct for `BlockManager`, more ergonomically referred to as + * `BlockManager::Options` due to the using-declaration in `BlockManager`. + */ +struct BlockManagerOpts { + uint64_t prune_target{0}; +}; + +} // namespace kernel + +#endif // BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H diff --git a/src/node/blockmanager_args.cpp b/src/node/blockmanager_args.cpp new file mode 100644 index 0000000000..cba44dafb9 --- /dev/null +++ b/src/node/blockmanager_args.cpp @@ -0,0 +1,32 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include + +namespace node { +std::optional ApplyArgsManOptions(const ArgsManager& args, BlockManager::Options& opts) +{ + // block pruning; get the amount of disk space (in MiB) to allot for block & undo files + int64_t nPruneArg{args.GetIntArg("-prune", opts.prune_target)}; + if (nPruneArg < 0) { + return _("Prune cannot be configured with a negative value."); + } + uint64_t nPruneTarget{uint64_t(nPruneArg) * 1024 * 1024}; + if (nPruneArg == 1) { // manual pruning: -prune=1 + nPruneTarget = std::numeric_limits::max(); + fPruneMode = true; + } else if (nPruneTarget) { + if (nPruneTarget < MIN_DISK_SPACE_FOR_BLOCK_FILES) { + return strprintf(_("Prune configured below the minimum of %d MiB. Please use a higher number."), MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024); + } + fPruneMode = true; + } + opts.prune_target = nPruneTarget; + + return std::nullopt; +} +} // namespace node diff --git a/src/node/blockmanager_args.h b/src/node/blockmanager_args.h new file mode 100644 index 0000000000..e657c6bb45 --- /dev/null +++ b/src/node/blockmanager_args.h @@ -0,0 +1,20 @@ + +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_NODE_BLOCKMANAGER_ARGS_H +#define BITCOIN_NODE_BLOCKMANAGER_ARGS_H + +#include + +#include + +class ArgsManager; +struct bilingual_str; + +namespace node { +std::optional ApplyArgsManOptions(const ArgsManager& args, BlockManager::Options& opts); +} // namespace node + +#endif // BITCOIN_NODE_BLOCKMANAGER_ARGS_H diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 255b73b347..5e364942d4 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -29,7 +29,6 @@ namespace node { std::atomic_bool fImporting(false); std::atomic_bool fReindex(false); bool fPruneMode = false; -uint64_t nPruneTarget = 0; bool CBlockIndexWorkComparator::operator()(const CBlockIndex* pa, const CBlockIndex* pb) const { @@ -178,7 +177,7 @@ void BlockManager::FindFilesToPruneManual(std::set& setFilesToPrune, int nM void BlockManager::FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, int prune_height, bool is_ibd) { LOCK2(cs_main, cs_LastBlockFile); - if (chain_tip_height < 0 || nPruneTarget == 0) { + if (chain_tip_height < 0 || GetPruneTarget() == 0) { return; } if ((uint64_t)chain_tip_height <= nPruneAfterHeight) { @@ -194,14 +193,14 @@ void BlockManager::FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPr uint64_t nBytesToPrune; int count = 0; - if (nCurrentUsage + nBuffer >= nPruneTarget) { + if (nCurrentUsage + nBuffer >= GetPruneTarget()) { // On a prune event, the chainstate DB is flushed. // To avoid excessive prune events negating the benefit of high dbcache // values, we should not prune too rapidly. // So when pruning in IBD, increase the buffer a bit to avoid a re-prune too soon. if (is_ibd) { // Since this is only relevant during IBD, we use a fixed 10% - nBuffer += nPruneTarget / 10; + nBuffer += GetPruneTarget() / 10; } for (int fileNumber = 0; fileNumber < m_last_blockfile; fileNumber++) { @@ -211,7 +210,7 @@ void BlockManager::FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPr continue; } - if (nCurrentUsage + nBuffer < nPruneTarget) { // are we below our target? + if (nCurrentUsage + nBuffer < GetPruneTarget()) { // are we below our target? break; } @@ -229,9 +228,9 @@ void BlockManager::FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPr } LogPrint(BCLog::PRUNE, "target=%dMiB actual=%dMiB diff=%dMiB max_prune_height=%d removed %d blk/rev pairs\n", - nPruneTarget/1024/1024, nCurrentUsage/1024/1024, - ((int64_t)nPruneTarget - (int64_t)nCurrentUsage)/1024/1024, - nLastBlockWeCanPrune, count); + GetPruneTarget() / 1024 / 1024, nCurrentUsage / 1024 / 1024, + (int64_t(GetPruneTarget()) - int64_t(nCurrentUsage)) / 1024 / 1024, + nLastBlockWeCanPrune, count); } void BlockManager::UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) { diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 09aff7f08b..d76d916a93 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -49,7 +50,6 @@ static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = CMessageHeader::MESSAG extern std::atomic_bool fImporting; extern std::atomic_bool fReindex; extern bool fPruneMode; -extern uint64_t nPruneTarget; // Because validation code takes pointers to the map's CBlockIndex objects, if // we ever switch to another associative container, we need to either use a @@ -138,7 +138,13 @@ private: */ std::unordered_map m_prune_locks GUARDED_BY(::cs_main); + const kernel::BlockManagerOpts m_opts; + public: + using Options = kernel::BlockManagerOpts; + + explicit BlockManager(Options opts) : m_opts{std::move(opts)} {}; + BlockMap m_block_index GUARDED_BY(cs_main); std::vector GetAllBlockIndices() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); @@ -184,7 +190,7 @@ public: [[nodiscard]] bool IsPruneMode() const { return fPruneMode; } /** Attempt to stay below this number of bytes of block files. */ - [[nodiscard]] uint64_t GetPruneTarget() const { return nPruneTarget; } + [[nodiscard]] uint64_t GetPruneTarget() const { return m_opts.prune_target; } [[nodiscard]] bool LoadingBlocks() const { diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index d5825d0d7e..2118f476cd 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -21,7 +21,7 @@ BOOST_FIXTURE_TEST_SUITE(blockmanager_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) { const auto params {CreateChainParams(ArgsManager{}, CBaseChainParams::MAIN)}; - BlockManager blockman {}; + BlockManager blockman{{}}; CChain chain {}; // simulate adding a genesis block normally BOOST_CHECK_EQUAL(blockman.SaveBlockToDisk(params->GenesisBlock(), 0, chain, *params, nullptr).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 4e0000cb3d..58593c9d5b 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -184,7 +184,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve .adjusted_time_callback = GetAdjustedTime, .check_block_index = true, }; - m_node.chainman = std::make_unique(chainman_opts); + m_node.chainman = std::make_unique(chainman_opts, node::BlockManager::Options{}); m_node.chainman->m_blockman.m_block_tree_db = std::make_unique(DBParams{ .path = m_args.GetDataDirNet() / "blocks" / "index", .cache_bytes = static_cast(m_cache_sizes.block_tree_db), diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 6fc9d0fa51..688aafdd46 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -381,7 +381,7 @@ struct SnapshotTestSetup : TestChain100Setup { // For robustness, ensure the old manager is destroyed before creating a // new one. m_node.chainman.reset(); - m_node.chainman.reset(new ChainstateManager(chainman_opts)); + m_node.chainman = std::make_unique(chainman_opts, node::BlockManager::Options{}); } return *Assert(m_node.chainman); } @@ -588,7 +588,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup // chainstate_snapshot should still exist. BOOST_CHECK(fs::exists(snapshot_chainstate_dir)); - // Test that simulating a shutdown (reseting ChainstateManager) and then performing + // Test that simulating a shutdown (resetting ChainstateManager) and then performing // chainstate reinitializing successfully cleans up the background-validation // chainstate data, and we end up with a single chainstate that is at tip. ChainstateManager& chainman_restarted = this->SimulateNodeRestart(); @@ -660,7 +660,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion_hash_mismatch, Sna fs::path snapshot_invalid_dir = gArgs.GetDataDirNet() / "chainstate_snapshot_INVALID"; BOOST_CHECK(fs::exists(snapshot_invalid_dir)); - // Test that simulating a shutdown (reseting ChainstateManager) and then performing + // Test that simulating a shutdown (resetting ChainstateManager) and then performing // chainstate reinitializing successfully loads only the fully-validated // chainstate data, and we end up with a single chainstate that is at tip. ChainstateManager& chainman_restarted = this->SimulateNodeRestart(); diff --git a/src/validation.cpp b/src/validation.cpp index f3c0401c0f..47355b0850 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5555,7 +5555,9 @@ static ChainstateManager::Options&& Flatten(ChainstateManager::Options&& opts) return std::move(opts); } -ChainstateManager::ChainstateManager(Options options) : m_options{Flatten(std::move(options))} {} +ChainstateManager::ChainstateManager(Options options, node::BlockManager::Options blockman_options) + : m_options{Flatten(std::move(options))}, + m_blockman{std::move(blockman_options)} {} ChainstateManager::~ChainstateManager() { diff --git a/src/validation.h b/src/validation.h index b0cef0d37b..8fac2f6091 100644 --- a/src/validation.h +++ b/src/validation.h @@ -955,7 +955,7 @@ private: public: using Options = kernel::ChainstateManagerOpts; - explicit ChainstateManager(Options options); + explicit ChainstateManager(Options options, node::BlockManager::Options blockman_options); const CChainParams& GetParams() const { return m_options.chainparams; } const Consensus::Params& GetConsensus() const { return m_options.chainparams.GetConsensus(); } -- cgit v1.2.3