From 84820561dcb2d156d1a1151a480fc1be6649cae4 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Sat, 3 May 2025 08:41:02 -0400 Subject: [PATCH 1/3] validation: periodically flush dbcache during reindex-chainstate Move the periodic flush inside the outer loop of ActivateBestChain. For very long activations, such as with reindex-chainstate, this calls periodic flushes so progress can be saved to disk. Co-Authored-By: l0rinc --- src/validation.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 5ad2ebdcd7e..94b9de3a49e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3567,6 +3567,11 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< m_chainman.MaybeRebalanceCaches(); } + // Write changes periodically to disk, after relay. + if (!FlushStateToDisk(state, FlushStateMode::PERIODIC)) { + return false; + } + if (WITH_LOCK(::cs_main, return m_disabled)) { // Background chainstate has reached the snapshot base block, so exit. @@ -3591,11 +3596,6 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< m_chainman.CheckBlockIndex(); - // Write changes periodically to disk, after relay. - if (!FlushStateToDisk(state, FlushStateMode::PERIODIC)) { - return false; - } - return true; } From 41479ed1d23ea752d0ce14c2cf5627f43bceb722 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Sun, 4 May 2025 13:34:29 -0400 Subject: [PATCH 2/3] test: add test for periodic flush inside ActivateBestChain --- src/test/chainstate_write_tests.cpp | 61 ++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/src/test/chainstate_write_tests.cpp b/src/test/chainstate_write_tests.cpp index ccca2f9be10..e1b82ebc121 100644 --- a/src/test/chainstate_write_tests.cpp +++ b/src/test/chainstate_write_tests.cpp @@ -8,6 +8,10 @@ #include +// Taken from validation.cpp +static constexpr auto DATABASE_WRITE_INTERVAL_MIN{50min}; +static constexpr auto DATABASE_WRITE_INTERVAL_MAX{70min}; + BOOST_AUTO_TEST_SUITE(chainstate_write_tests) BOOST_FIXTURE_TEST_CASE(chainstate_write_interval, TestingSetup) @@ -31,15 +35,68 @@ BOOST_FIXTURE_TEST_CASE(chainstate_write_interval, TestingSetup) BOOST_CHECK(!sub->m_did_flush); // The periodic flush interval is between 50 and 70 minutes (inclusive) - SetMockTime(GetTime() + 49min); + SetMockTime(GetTime() + DATABASE_WRITE_INTERVAL_MIN - 1min); chainstate.FlushStateToDisk(state_dummy, FlushStateMode::PERIODIC); m_node.validation_signals->SyncWithValidationInterfaceQueue(); BOOST_CHECK(!sub->m_did_flush); - SetMockTime(GetTime() + 70min); + SetMockTime(GetTime() + DATABASE_WRITE_INTERVAL_MAX); chainstate.FlushStateToDisk(state_dummy, FlushStateMode::PERIODIC); m_node.validation_signals->SyncWithValidationInterfaceQueue(); BOOST_CHECK(sub->m_did_flush); } +// Test that we do PERIODIC flushes inside ActivateBestChain. +// This is necessary for reindex-chainstate to be able to periodically flush +// before reaching chain tip. +BOOST_FIXTURE_TEST_CASE(write_during_multiblock_activation, TestChain100Setup) +{ + struct TestSubscriber final : CValidationInterface + { + const CBlockIndex* m_tip{nullptr}; + const CBlockIndex* m_flushed_at_block{nullptr}; + void ChainStateFlushed(ChainstateRole, const CBlockLocator&) override + { + m_flushed_at_block = m_tip; + } + void UpdatedBlockTip(const CBlockIndex* block_index, const CBlockIndex*, bool) override { + m_tip = block_index; + } + }; + + auto& chainstate{Assert(m_node.chainman)->ActiveChainstate()}; + BlockValidationState state_dummy{}; + + // Pop two blocks from the tip + const CBlockIndex* tip{chainstate.m_chain.Tip()}; + CBlockIndex* second_from_tip{tip->pprev}; + + { + LOCK2(m_node.chainman->GetMutex(), chainstate.MempoolMutex()); + chainstate.DisconnectTip(state_dummy, nullptr); + chainstate.DisconnectTip(state_dummy, nullptr); + } + + BOOST_CHECK_EQUAL(second_from_tip->pprev, chainstate.m_chain.Tip()); + + // Set m_next_write to current time + chainstate.FlushStateToDisk(state_dummy, FlushStateMode::ALWAYS); + m_node.validation_signals->SyncWithValidationInterfaceQueue(); + // The periodic flush interval is between 50 and 70 minutes (inclusive) + // The next call to a PERIODIC write will flush + SetMockTime(GetMockTime() + DATABASE_WRITE_INTERVAL_MAX); + + const auto sub{std::make_shared()}; + m_node.validation_signals->RegisterSharedValidationInterface(sub); + + // ActivateBestChain back to tip + chainstate.ActivateBestChain(state_dummy, nullptr); + BOOST_CHECK_EQUAL(tip, chainstate.m_chain.Tip()); + // Check that we flushed inside ActivateBestChain while we were at the + // second block from tip, since FlushStateToDisk is called with PERIODIC + // inside the outer loop. + m_node.validation_signals->SyncWithValidationInterfaceQueue(); + BOOST_CHECK_EQUAL(sub->m_flushed_at_block, second_from_tip); +} + BOOST_AUTO_TEST_SUITE_END() From c1e554d3e5834a140f2a53854018499a3bfe6822 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Sun, 4 May 2025 13:35:29 -0400 Subject: [PATCH 3/3] refactor: consolidate 3 separate locks into one block The main lock needs to be taken 3 separate times. It simplifies readability to take it once, and is more efficient. --- src/validation.cpp | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 94b9de3a49e..b3f6d637a98 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3560,19 +3560,24 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< } // release cs_main // When we reach this point, we switched to a new tip (stored in pindexNewTip). - if (exited_ibd) { - // If a background chainstate is in use, we may need to rebalance our - // allocation of caches once a chainstate exits initial block download. - LOCK(::cs_main); - m_chainman.MaybeRebalanceCaches(); + bool disabled{false}; + { + LOCK(m_chainman.GetMutex()); + if (exited_ibd) { + // If a background chainstate is in use, we may need to rebalance our + // allocation of caches once a chainstate exits initial block download. + m_chainman.MaybeRebalanceCaches(); + } + + // Write changes periodically to disk, after relay. + if (!FlushStateToDisk(state, FlushStateMode::PERIODIC)) { + return false; + } + + disabled = m_disabled; } - // Write changes periodically to disk, after relay. - if (!FlushStateToDisk(state, FlushStateMode::PERIODIC)) { - return false; - } - - if (WITH_LOCK(::cs_main, return m_disabled)) { + if (disabled) { // Background chainstate has reached the snapshot base block, so exit. // Restart indexes to resume indexing for all blocks unique to the snapshot