From c6ca2b85a3e6e73674e210aee4ed69c4af2848e4 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 8 Aug 2024 09:56:53 -0400 Subject: [PATCH] validation: do not wipe utxo cache for stats/scans/snapshots Since #28280, the cost of a non-wiping sync of the UTXO cache is only proportional to the number of dirty entries, rather than proportional to the size of the entire cache. Because of that, there is no reason to perform a wiping flush in case the contents of the cache is still useful. Split the FlushStateMode::ALWAYS mode into a FORCE_SYNC (non-wiping) and a FORCE_FLUSH (wiping), and then use the former in scantxoutset, gettxoutsetinfo, snapshot creation. Co-authored-by: l0rinc Co-authored-by: cedwies <141683552+cedwies@users.noreply.github.com> --- contrib/tracing/log_utxocache_flush.py | 1 + doc/tracing.md | 2 +- src/rpc/blockchain.cpp | 6 +++--- src/test/fuzz/utxo_snapshot.cpp | 2 +- src/test/fuzz/utxo_total_supply.cpp | 10 +++++----- src/validation.cpp | 6 +++--- src/validation.h | 5 +++-- test/functional/interface_usdt_utxocache.py | 5 +++-- 8 files changed, 20 insertions(+), 17 deletions(-) diff --git a/contrib/tracing/log_utxocache_flush.py b/contrib/tracing/log_utxocache_flush.py index bcbc79dfb30..fd35e7f69c7 100755 --- a/contrib/tracing/log_utxocache_flush.py +++ b/contrib/tracing/log_utxocache_flush.py @@ -46,6 +46,7 @@ FLUSH_MODES = [ 'IF_NEEDED', 'PERIODIC', 'FORCE_FLUSH', + 'FORCE_SYNC', ] diff --git a/doc/tracing.md b/doc/tracing.md index e0e96b044eb..f2599708f94 100644 --- a/doc/tracing.md +++ b/doc/tracing.md @@ -186,7 +186,7 @@ Is called *after* the in-memory UTXO cache is flushed. Arguments passed: 1. Time it took to flush the cache microseconds as `int64` 2. Flush state mode as `uint32`. It's an enumerator class with values - `0` (`NONE`), `1` (`IF_NEEDED`), `2` (`PERIODIC`), `3` (`FORCE_FLUSH`), + `0` (`NONE`), `1` (`IF_NEEDED`), `2` (`PERIODIC`), `3` (`FORCE_FLUSH`), `4` (`FORCE_SYNC`) 3. Cache size (number of coins) before the flush as `uint64` 4. Cache memory usage in bytes as `uint64` 5. If pruning caused the flush as `bool` diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 763de836890..4783f37ae59 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1043,7 +1043,7 @@ static RPCHelpMan gettxoutsetinfo() NodeContext& node = EnsureAnyNodeContext(request.context); ChainstateManager& chainman = EnsureChainman(node); Chainstate& active_chainstate = chainman.ActiveChainstate(); - active_chainstate.ForceFlushStateToDisk(); + active_chainstate.ForceFlushStateToDisk(/*wipe_cache=*/false); CCoinsView* coins_view; BlockManager* blockman; @@ -2383,7 +2383,7 @@ static RPCHelpMan scantxoutset() ChainstateManager& chainman = EnsureChainman(node); LOCK(cs_main); Chainstate& active_chainstate = chainman.ActiveChainstate(); - active_chainstate.ForceFlushStateToDisk(); + active_chainstate.ForceFlushStateToDisk(/*wipe_cache=*/false); pcursor = CHECK_NONFATAL(active_chainstate.CoinsDB().Cursor()); tip = CHECK_NONFATAL(active_chainstate.m_chain.Tip()); } @@ -3200,7 +3200,7 @@ PrepareUTXOSnapshot( // AssertLockHeld(::cs_main); - chainstate.ForceFlushStateToDisk(); + chainstate.ForceFlushStateToDisk(/*wipe_cache=*/false); maybe_stats = GetUTXOStats(&chainstate.CoinsDB(), chainstate.m_blockman, CoinStatsHashType::HASH_SERIALIZED, interruption_point); if (!maybe_stats) { diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index e6ae29a873f..f3ab3554656 100644 --- a/src/test/fuzz/utxo_snapshot.cpp +++ b/src/test/fuzz/utxo_snapshot.cpp @@ -57,7 +57,7 @@ void sanity_check_snapshot() // Connect the chain to the tmp chainman and sanity check the chainparams snapshot values. LOCK(cs_main); auto& cs{node.chainman->ActiveChainstate()}; - cs.ForceFlushStateToDisk(); + cs.ForceFlushStateToDisk(/*wipe_cache=*/false); const auto stats{*Assert(kernel::ComputeUTXOStats(kernel::CoinStatsHashType::HASH_SERIALIZED, &cs.CoinsDB(), node.chainman->m_blockman))}; const auto cp_au_data{*Assert(node.chainman->GetParams().AssumeutxoForHeight(2 * COINBASE_MATURITY))}; Assert(stats.nHeight == cp_au_data.height); diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp index d27ca3470b4..e23fe74492b 100644 --- a/src/test/fuzz/utxo_total_supply.cpp +++ b/src/test/fuzz/utxo_total_supply.cpp @@ -87,9 +87,9 @@ FUZZ_TARGET(utxo_total_supply) tx.vin.emplace_back(txo.first); tx.vout.emplace_back(txo.second.nValue, txo.second.scriptPubKey); // "Forward" coin with no fee }; - const auto UpdateUtxoStats = [&]() { + const auto UpdateUtxoStats = [&](bool wipe_cache) { LOCK(chainman.GetMutex()); - chainman.ActiveChainstate().ForceFlushStateToDisk(); + chainman.ActiveChainstate().ForceFlushStateToDisk(wipe_cache); utxo_stats = std::move( *Assert(kernel::ComputeUTXOStats(kernel::CoinStatsHashType::NONE, &chainman.ActiveChainstate().CoinsDB(), chainman.m_blockman, {}))); // Check that miner can't print more money than they are allowed to @@ -99,7 +99,7 @@ FUZZ_TARGET(utxo_total_supply) // Update internal state to chain tip StoreLastTxo(); - UpdateUtxoStats(); + UpdateUtxoStats(/*wipe_cache=*/fuzzed_data_provider.ConsumeBool()); assert(ActiveHeight() == 0); // Get at which height we duplicate the coinbase // Assuming that the fuzzer will mine relatively short chains (less than 200 blocks), we want the duplicate coinbase to be not too high. @@ -124,7 +124,7 @@ FUZZ_TARGET(utxo_total_supply) circulation += GetBlockSubsidy(ActiveHeight(), Params().GetConsensus()); assert(ActiveHeight() == 1); - UpdateUtxoStats(); + UpdateUtxoStats(/*wipe_cache=*/fuzzed_data_provider.ConsumeBool()); current_block = PrepareNextBlock(); StoreLastTxo(); @@ -163,7 +163,7 @@ FUZZ_TARGET(utxo_total_supply) circulation += GetBlockSubsidy(ActiveHeight(), Params().GetConsensus()); } - UpdateUtxoStats(); + UpdateUtxoStats(/*wipe_cache=*/fuzzed_data_provider.ConsumeBool()); if (!was_valid) { // utxo stats must not change diff --git a/src/validation.cpp b/src/validation.cpp index c8d326e5802..f0ac366134e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2797,7 +2797,7 @@ bool Chainstate::FlushStateToDisk( bool fPeriodicWrite = mode == FlushStateMode::PERIODIC && nNow >= m_next_write; const auto empty_cache{(mode == FlushStateMode::FORCE_FLUSH) || fCacheLarge || fCacheCritical}; // Combine all conditions that result in a write to disk. - bool should_write = empty_cache || fPeriodicWrite || fFlushForPrune; + bool should_write = (mode == FlushStateMode::FORCE_SYNC) || empty_cache || fPeriodicWrite || fFlushForPrune; // Write blocks, block index and best chain related state to disk. if (should_write) { LogDebug(BCLog::COINDB, "Writing chainstate to disk: flush mode=%s, prune=%d, large=%d, critical=%d, periodic=%d", @@ -2871,10 +2871,10 @@ bool Chainstate::FlushStateToDisk( return true; } -void Chainstate::ForceFlushStateToDisk() +void Chainstate::ForceFlushStateToDisk(bool wipe_cache) { BlockValidationState state; - if (!this->FlushStateToDisk(state, FlushStateMode::FORCE_FLUSH)) { + if (!this->FlushStateToDisk(state, wipe_cache ? FlushStateMode::FORCE_FLUSH : FlushStateMode::FORCE_SYNC)) { LogWarning("Failed to force flush state (%s)", state.ToString()); } } diff --git a/src/validation.h b/src/validation.h index fb96de4e2eb..b666fe6b0a1 100644 --- a/src/validation.h +++ b/src/validation.h @@ -457,12 +457,13 @@ enum DisconnectResult class ConnectTrace; /** @see Chainstate::FlushStateToDisk */ -inline constexpr std::array FlushStateModeNames{"NONE", "IF_NEEDED", "PERIODIC", "FORCE_FLUSH"}; +inline constexpr std::array FlushStateModeNames{"NONE", "IF_NEEDED", "PERIODIC", "FORCE_FLUSH", "FORCE_SYNC"}; enum class FlushStateMode: uint8_t { NONE, IF_NEEDED, PERIODIC, FORCE_FLUSH, + FORCE_SYNC, }; /** @@ -736,7 +737,7 @@ public: int nManualPruneHeight = 0); //! Flush all changes to disk. - void ForceFlushStateToDisk(); + void ForceFlushStateToDisk(bool wipe_cache = true); //! Prune blockfiles from the disk if necessary and then flush chainstate changes //! if we pruned. diff --git a/test/functional/interface_usdt_utxocache.py b/test/functional/interface_usdt_utxocache.py index bbdb8b9a6b6..da90790d70d 100755 --- a/test/functional/interface_usdt_utxocache.py +++ b/test/functional/interface_usdt_utxocache.py @@ -110,6 +110,7 @@ FLUSHMODE_NAME = { 1: "IF_NEEDED", 2: "PERIODIC", 3: "FORCE_FLUSH", + 4: "FORCE_SYNC", } @@ -385,7 +386,7 @@ class UTXOCacheTracepointTest(BitcoinTestFramework): bpf["utxocache_flush"].open_perf_buffer(handle_utxocache_flush) self.log.info("stop the node to flush the UTXO cache") - UTXOS_IN_CACHE = 2 # might need to be changed if the earlier tests are modified + UTXOS_IN_CACHE = 3 # might need to be changed if the earlier tests are modified # A node shutdown causes two flushes. One that flushes UTXOS_IN_CACHE # UTXOs and one that flushes 0 UTXOs. Normally the 0-UTXO-flush is the # second flush, however it can happen that the order changes. @@ -415,7 +416,7 @@ class UTXOCacheTracepointTest(BitcoinTestFramework): bpf["utxocache_flush"].open_perf_buffer(handle_utxocache_flush) self.log.info("prune blockchain to trigger a flush for pruning") - expected_flushes.append({"mode": "NONE", "for_prune": True, "size": 0}) + expected_flushes.append({"mode": "NONE", "for_prune": True, "size": BLOCKS_TO_MINE}) self.nodes[0].pruneblockchain(315) bpf.perf_buffer_poll(timeout=500)