validation: delete pruned files after a failed flush #36735

pull pzafonte wants to merge 4 commits into bitcoin:master from pzafonte:prune-unlink-after-retry changing 5 files +95 −5

pzafonte commented at 10:09 PM on November 2, 2026: contributor

If a flush that prunes block files fails before the block index write, the files are already marked pruned in memory, but setFilesToPrune is a local in FlushStateToDisk(). The next successful flush writes the pruned file info and never deletes the blk/rev files. Only ScanAndUnlinkAlreadyPrunedFiles() deletes them, at the next startup.

This is number 7 of ryanofsky's [post-merge review of #35714](/bitcoin-bitcoin/35714/#pullrequestreview-5697890316). The block file flush failure from #35714 and the blocks directory CheckDiskSpace() failure both return before that write.

I worked on this because it lets me test my kernel pruning refactor (for #35627 and kernel-node/kernel-node#111) against the issue he describes. The refactor moves the flush, block index write and file deletion that a pruning flush runs from FlushStateToDisk() into WriteBlockStorage(), and no unit test ran that sequence in prune mode. I rebased my refactor onto this branch and they pass.

The first three commits are test the current behavior for a flush after a failed one writes what was skipped, but a failed prune flush still leaves its files on disk after a retry.

The last commit keeps the selected files in BlockManager and deletes them after the next block index write. If that write fails they are dropped with the dirty entries, which means a file is only deleted once it is marked pruned on disk. Pending files don't force a write. The next flush that writes deletes them.

The disk-space failure and automatic pruning take the same path but have no test. Neither does the FindNextBlockPos() line that drops a file from the pending set when a block is written to it, which can happen after an assumeutxo snapshot validates.

I will be opening follow-ups as drafts based on top of this shortly.

test: apply BlockManager args in test fixtures

Without this, -prune and -fastprune in extra_args have no effect.

24ad36d929

test: check flush retry after failure

Move the blk file aside instead of deleting it, so it can be restored.

2334d27a24

test: characterize a failed prune flush

The pruned blk/rev files are not deleted, even after a later flush
succeeds.

108d7ff03c

validation: delete pruned files on a later flush

Files selected for pruning were kept in a local, so a failed flush left
them on disk until the next startup.

Keep them in BlockManager and delete them after the next block index
write. Take them with the dirty entries, which WriteBlockIndexDB() drops
even if the write fails. Drop a file from the set once a block is
written to it.

Pending files wait for the next periodic, forced or shutdown flush
rather than forcing one, which would retry a failing write on every
block.

d224fb8721

DrahtBot added the label Validation on Nov 2, 2026
DrahtBot commented at 10:09 PM on November 2, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



External sites
- Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36735
- Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36735.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36735#author-ralph


Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.
in src/validation.cpp:2865 in d224fb8721
```
2859 | @@ -2859,17 +2860,20 @@ bool Chainstate::FlushStateToDisk(
2860 |                  }
2861 |              }
2862 |  
2863 | +            // Take the pruned files with the dirty entries, which WriteBlockIndexDB() drops
2864 | +            // even if it fails, so files are only deleted after the write that marks them pruned.
2865 | +            const auto files_to_unlink{std::exchange(m_blockman.m_files_to_unlink, {})};
```
Yudis-bit commented at 4:32 PM on November 3, 2026:

If a flush fails after PruneOneBlockFile() clears nSize, retrying pruneblockchain won't clean up the files. FindFilesToPruneManual() skips files with nSize == 0, so fFlushForPrune stays false. With FlushStateMode::NONE, should_write is false and the function returns early without ever unlinking them.

In chainstate_write_tests.cpp:211, the retry passes only because it calls FORCE_FLUSH instead of FlushStateMode::NONE, prune_height.
in src/node/blockstorage.cpp:899 in d224fb8721
```
 894 | @@ -895,6 +895,8 @@ FlatFilePos BlockManager::FindNextBlockPos(unsigned int nAddSize, unsigned int n
 895 |              m_blockfile_info.resize(nFile + 1);
 896 |          }
 897 |      }
 898 | +    // Don't delete a pruned file that is about to hold a new block.
 899 | +    m_files_to_unlink.erase(nFile);
```
Yudis-bit commented at 4:32 PM on November 3, 2026:

m_files_to_unlink.erase(nFile);

Is it actually possible for nFile to be in m_files_to_unlink here? m_last_blockfile contains the tip, and rolling over advances to a new file number.

Also, if an old file were somehow reused here, opening it with rb+ would overwrite the head without truncating the existing block data on disk.

Contributors

Labels

Linked (view graph)

#35627 kernel: can't work with pruned blocksdir #35714 validation: return block-file flush errors #36738 refactor: move block writes and pruning into BlockManager

validation: delete pruned files after a failed flush #36735

External sites

Reviews