Merge pull request #6276 from zcash/revert-6231-headers-first-rebirth

Revert "Headers-first fix" to avoid a memory usage regression on IBD
This commit is contained in:
Greg Pfeil 2022-12-03 12:30:44 -07:00 committed by GitHub
commit e1b5bd26ea
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 13 additions and 41 deletions

View File

@ -4,3 +4,15 @@ release-notes at release time)
Notable changes
===============
Fixed
-----
This is a hotfix release that fixes a regression in memory usage during
Initial Block Download. The regression was indirectly caused by a change
to prioritize downloading headers (PR #6231), introduced in release 5.3.1.
It caused memory usage for new nodes to spike to roughly 11 GiB about an
hour after starting Initial Block Download.
The issue fixed by this release does not affect nodes that start from
a fully synced chain, or that had sufficient memory available to get
past the memory usage spike.

View File

@ -332,8 +332,6 @@ struct CNodeState {
CBlockIndex *pindexLastCommonBlock;
//! Whether we've started headers synchronization with this peer.
bool fSyncStarted;
//! When to potentially disconnect peer for stalling headers download
int64_t nHeadersSyncTimeout;
//! Since when we're stalling block download progress (in microseconds), or 0.
int64_t nStallingSince;
list<QueuedBlock> vBlocksInFlight;
@ -350,7 +348,6 @@ struct CNodeState {
hashLastUnknownBlock.SetNull();
pindexLastCommonBlock = NULL;
fSyncStarted = false;
nHeadersSyncTimeout = 0;
nStallingSince = 0;
nBlocksInFlight = 0;
nBlocksInFlightValidHeaders = 0;
@ -517,7 +514,7 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector<CBl
// Make sure pindexBestKnownBlock is up to date, we'll need it.
ProcessBlockAvailability(nodeid);
if (state->pindexBestKnownBlock == NULL || state->pindexBestKnownBlock->nChainWork < chainActive.Tip()->nChainWork || state->pindexBestKnownBlock->nChainWork < UintToArith256(Params().GetConsensus().nMinimumChainWork)) {
if (state->pindexBestKnownBlock == NULL || state->pindexBestKnownBlock->nChainWork < chainActive.Tip()->nChainWork) {
// This peer has nothing interesting.
return;
}
@ -7651,7 +7648,6 @@ bool SendMessages(const Consensus::Params& params, CNode* pto)
// Only actively request headers from a single peer, unless we're close to today.
if ((nSyncStarted == 0 && fFetch) || pindexBestHeader->GetBlockTime() > GetTime() - 24 * 60 * 60) {
state.fSyncStarted = true;
state.nHeadersSyncTimeout = GetTimeMicros() + HEADERS_DOWNLOAD_TIMEOUT_BASE + HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER * (EstimateNetHeight(params, pindexBestHeader->nHeight, pindexBestHeader->GetBlockTime()) - pindexBestHeader->nHeight);
nSyncStarted++;
const CBlockIndex *pindexStart = pindexBestHeader;
/* If possible, start at the block preceding the currently
@ -7837,38 +7833,6 @@ bool SendMessages(const Consensus::Params& params, CNode* pto)
pto->fDisconnect = true;
}
}
// Check for headers sync timeouts
if (state.fSyncStarted && state.nHeadersSyncTimeout < std::numeric_limits<int64_t>::max()) {
// Detect whether this is a stalling initial-headers-sync peer
if (pindexBestHeader->GetBlockTime() <= GetTime() - 24*60*60) {
if (nNow > state.nHeadersSyncTimeout && nSyncStarted == 1 && (nPreferredDownload - state.fPreferredDownload >= 1)) {
// Disconnect a (non-whitelisted) peer if it is our only sync peer,
// and we have others we could be using instead.
// Note: If all our peers are inbound, then we won't
// disconnect our sync peer for stalling; we have bigger
// problems if we can't get any outbound peers.
if (!pto->fWhitelisted) {
LogPrintf("Timeout downloading headers from peer=%d, disconnecting\n", pto->GetId());
pto->fDisconnect = true;
return true;
} else {
LogPrintf("Timeout downloading headers from whitelisted peer=%d, not disconnecting\n", pto->GetId());
// Reset the headers sync state so that we have a
// chance to try downloading from a different peer.
// Note: this will also result in at least one more
// getheaders message to be sent to
// this peer (eventually).
state.fSyncStarted = false;
nSyncStarted--;
state.nHeadersSyncTimeout = 0;
}
}
} else {
// After we've caught up once, reset the timeout so we can't trigger
// disconnect later.
state.nHeadersSyncTimeout = std::numeric_limits<int64_t>::max();
}
}
//
// Message: getdata (blocks)

View File

@ -98,10 +98,6 @@ static const int DEFAULT_SCRIPTCHECK_THREADS = 0;
static const int MAX_BLOCKS_IN_TRANSIT_PER_PEER = 16;
/** Timeout in seconds during which a peer must stall block download progress before being disconnected. */
static const unsigned int BLOCK_STALLING_TIMEOUT = 2;
/** Headers download timeout expressed in microseconds
* Timeout = base + per_header * (expected number of headers) */
static constexpr int64_t HEADERS_DOWNLOAD_TIMEOUT_BASE = 30 * 60 * 1000000; // 30 minutes
static constexpr int64_t HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER = 10000; // 10 ms/header
/** Number of headers sent in one getheaders result. We rely on the assumption that if a peer sends
* less than this number, we reached its tip. Changing this value is a protocol upgrade. */
static const unsigned int MAX_HEADERS_RESULTS = 160;