Merge 34ae04d775 into merged_master (Bitcoin PR bitcoin/bitcoin#21726)

This changed the node pruning logic and moved test/functional/feature_blockfilterindex_prune.py
to test/functional/feature_index_prune.py.

Please verify that:
1. I migrated the test correctly
2. The magic numbers in the test look fine

With respect to #2: I believe the magic numbers are wrong. I previously had to tweak them heavily
in commit 1278b31. I don't think I did it correctly then, and so I don't believe them to be correct now.

To summarize what this tweaking was: I changed the magic numbers in the test to work properly,
but I suspect that in changing them, I may have nullified what the test was testing.

It's very possible that the reason the test was failing was because of an underlying bug with the pruning
in elements which we have to fix, rather than just being an issue with the test itself.
This commit is contained in:
James Dorfman 2024-08-06 05:53:19 +00:00
commit 01e944c8aa
15 changed files with 265 additions and 115 deletions

View file

@ -19,7 +19,6 @@
#include <deploymentstatus.h>
#include <flatfile.h>
#include <hash.h>
#include <index/blockfilterindex.h>
#include <logging.h>
#include <logging/timer.h>
#include <mainchainrpc.h>
@ -112,6 +111,12 @@ const std::vector<std::string> CHECKLEVEL_DOC {
"level 4 tries to reconnect the blocks",
"each level includes the checks of the previous levels",
};
/** The number of blocks to keep below the deepest prune lock.
* There is nothing special about this number. It is higher than what we
* expect to see in regular mainnet reorgs, but not so high that it would
* noticeably interfere with the pruning mechanism.
* */
static constexpr int PRUNE_LOCK_BUFFER{10};
/**
* Mutex to guard access to validation specific variables, such as reading
@ -2564,12 +2569,24 @@ bool CChainState::FlushStateToDisk(
CoinsCacheSizeState cache_state = GetCoinsCacheSizeState();
LOCK(m_blockman.cs_LastBlockFile);
if (fPruneMode && (m_blockman.m_check_for_pruning || nManualPruneHeight > 0) && !fReindex) {
// make sure we don't prune above the blockfilterindexes bestblocks
// make sure we don't prune above any of the prune locks bestblocks
// pruning is height-based
int last_prune = m_chain.Height(); // last height we can prune
ForEachBlockFilterIndex([&](BlockFilterIndex& index) {
last_prune = std::max(1, std::min(last_prune, index.GetSummary().best_block_height));
});
int last_prune{m_chain.Height()}; // last height we can prune
std::optional<std::string> limiting_lock; // prune lock that actually was the limiting factor, only used for logging
for (const auto& prune_lock : m_blockman.m_prune_locks) {
if (prune_lock.second.height_first == std::numeric_limits<int>::max()) continue;
// Remove the buffer and one additional block here to get actual height that is outside of the buffer
const int lock_height{prune_lock.second.height_first - PRUNE_LOCK_BUFFER - 1};
last_prune = std::max(1, std::min(last_prune, lock_height));
if (last_prune == lock_height) {
limiting_lock = prune_lock.first;
}
}
if (limiting_lock) {
LogPrint(BCLog::PRUNE, "%s limited pruning to height %d\n", limiting_lock.value(), last_prune);
}
if (nManualPruneHeight > 0) {
LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune (manual)", BCLog::BENCH);
@ -2846,6 +2863,18 @@ bool CChainState::DisconnectTip(BlockValidationState& state, DisconnectedBlockTr
assert(flushed);
}
LogPrint(BCLog::BENCH, "- Disconnect block: %.2fms\n", (GetTimeMicros() - nStart) * MILLI);
{
// Prune locks that began at or after the tip should be moved backward so they get a chance to reorg
const int max_height_first{pindexDelete->nHeight - 1};
for (auto& prune_lock : m_blockman.m_prune_locks) {
if (prune_lock.second.height_first <= max_height_first) continue;
prune_lock.second.height_first = max_height_first;
LogPrint(BCLog::PRUNE, "%s prune lock moved back to %d\n", prune_lock.first, max_height_first);
}
}
// Write the chain state to disk, if necessary.
if (!FlushStateToDisk(state, FlushStateMode::IF_NEEDED)) {
return false;