In commit "test: characterize writes after flush failure" (07d23eb75527b8dd6571a33b3bc304815764bc07)
Optional, could be a follow-up: it would be nice if the test also checked what happens on the next flush after this one fails.
With this PR, when the block file flush fails, FlushStateToDisk() returns without writing the block index or the coins. That data stays in memory, so if the problem clears up before a later FlushStateToDisk() call (say, the open failed because the node ran out of file descriptors), that call writes it. The last chance is normally the flush in Shutdown(). If the problem doesn't clear up, the node stops at the last successful flush and catches up after a restart. The first case isn't tested right now.
Moving the block file aside instead of deleting it lets the test put it back, flush again, and check that everything catches up to new_tip:
--- a/src/test/chainstate_write_tests.cpp
+++ b/src/test/chainstate_write_tests.cpp
@@ -114,5 +114,4 @@ BOOST_FIXTURE_TEST_CASE(write_during_multiblock_activation, TestChain100Setup)
BOOST_FIXTURE_TEST_CASE(chainstate_flush_failure_boundary, TestChain100Setup)
{
- auto inject_file_open_failure{[](const fs::path& file) { Assert(fs::remove(file)); Assert(fs::create_directory(file)); }};
auto& chainstate{Assert(m_node.chainman)->ActiveChainstate()};
BlockValidationState state{};
@@ -124,5 +123,9 @@ BOOST_FIXTURE_TEST_CASE(chainstate_flush_failure_boundary, TestChain100Setup)
auto* new_tip{WITH_LOCK(::cs_main, return chainstate.m_chain.Tip())};
BOOST_REQUIRE_NE(old_flushed, new_tip);
- inject_file_open_failure(WITH_LOCK(::cs_main, return chainstate.m_blockman.GetBlockPosFilename(new_tip->GetBlockPos())));
+ // Move the block file aside and put a directory in its place so the flush fails to open it.
+ const fs::path blk_file{WITH_LOCK(::cs_main, return chainstate.m_blockman.GetBlockPosFilename(new_tip->GetBlockPos()))};
+ const fs::path blk_backup{blk_file + ".bak"};
+ fs::rename(blk_file, blk_backup);
+ Assert(fs::create_directory(blk_file));
ASSERT_DEBUG_LOG("Flushing block file to disk failed");
@@ -135,4 +138,14 @@ BOOST_FIXTURE_TEST_CASE(chainstate_flush_failure_boundary, TestChain100Setup)
BOOST_REQUIRE(WITH_LOCK(::cs_main, return chainstate.m_blockman.m_block_tree_db->ReadBlockFileInfo(new_tip->nFile, file_info)));
BOOST_CHECK_EQUAL(file_info.nHeightLast, old_flushed->nHeight);
+
+ // Once the block file is back, a later flush succeeds and catches up.
+ Assert(fs::remove(blk_file));
+ fs::rename(blk_backup, blk_file);
+ state = {};
+ BOOST_CHECK(chainstate.FlushStateToDisk(state, FlushStateMode::FORCE_FLUSH));
+ BOOST_CHECK_EQUAL(WITH_LOCK(::cs_main, return chainstate.GetLastFlushedBlock()), new_tip);
+ BOOST_CHECK(WITH_LOCK(::cs_main, return chainstate.CoinsDB().GetBestBlock()) == new_tip->GetBlockHash());
+ BOOST_REQUIRE(WITH_LOCK(::cs_main, return chainstate.m_blockman.m_block_tree_db->ReadBlockFileInfo(new_tip->nFile, file_info)));
+ BOOST_CHECK_EQUAL(file_info.nHeightLast, new_tip->nHeight);
}
The diff is against the PR head. The retry checks pass with or without the fix (without it, the failing flush already wrote everything), so they could also go in the first commit.
Update: this is item 5 in the post-merge follow-ups list: #35714#pullrequestreview-5697890316
