Skip to content

Improve elements memory usage #1190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Jan 26, 2023
Merged

Improve elements memory usage #1190

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
68eb086
Refactor CBlockIndex to make proof and dynafed fields optional.
gwillen Nov 8, 2022
7c55557
Add a flag for header-trimming; clear P2P network feature bits when e…
gwillen Nov 8, 2022
eff6be3
Trim headers when loading block index from disk.
gwillen Nov 8, 2022
4f0da3c
Trim headers when flushing to disk, and reduce flush interval from 1h…
gwillen Nov 8, 2022
f5c7995
When people ask us for headers we do not have (over REST or RPC), do …
gwillen Nov 10, 2022
7e6fe4a
Don't let header syncing get too far ahead of block syncing
gwillen Nov 15, 2022
a2db974
Fix false-positive lint
gwillen Nov 15, 2022
763d298
Fix deserialization issues in certain functional tests
gwillen Nov 16, 2022
394e59d
Politely and safely refuse getheaders requests for headers we have tr…
gwillen Nov 23, 2022
fba37b7
Move enough-headers check up, rejecting excess headers when received.
gwillen Dec 7, 2022
29345e9
pgreco's startup crash fix
gwillen Dec 7, 2022
b9b7f65
Apply suggestions from code review
gwillen Jan 4, 2023
8cad8cd
Further fixes from code review
gwillen Jan 5, 2023
e8475e7
Fix stuck IBD when discarding unwanted headers from peers.
gwillen Jan 25, 2023
e622156
Update documentation
gwillen Jan 25, 2023
15dbe64
Remove stray comment
gwillen Jan 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 55 additions & 11 deletions src/txdb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,12 +307,47 @@ bool CBlockTreeDB::WritePAKList(const std::vector<std::vector<unsigned char> >&
return Write(std::make_pair(DB_PAK, uint256S("1")), offline_list) && Write(std::make_pair(DB_PAK, uint256S("2")), online_list) && Write(std::make_pair(DB_PAK, uint256S("3")), reject);
}

bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function<CBlockIndex*(const uint256&)> insertBlockIndex)
bool CBlockTreeDB::WalkBlockIndexGutsForMaxHeight(int* nHeight) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not guaranteed to be the max height, correct? This is our best guess of the max height, from a random sample of 10k blocks?

Copy link
Collaborator

@jamesdorfman jamesdorfman Jan 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could that be placed into the docstring for this method, since it is public?

Copy link
Collaborator

@jamesdorfman jamesdorfman Jan 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, forgive my ignorance about the bitcoin / elements codebase, but is there no way to get the max height here? Certainly that is stored somewhere, since certain RPCs produce it.

Is it just that accessing it here would require some major refactors which are overkill for this PR? Or... do we actually have to load the block index first in order to get the max height?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to load the block index first to get the actual max height, yeah, it's a catch-22 here. This is one of the very first stages of startup. And the headers in the index are stored on disk in hash order, not height order, so we have no idea what the max height is until after we've loaded all of them. (But since hash order is random with respect to height, loading a few gives us a really good guess.)

Also note that this really only tells us the max header height, we don't learn what blocks we actually have until even later in the startup process.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. That makes sense, thanks,

What do you think about writing that "this is not guaranteed to be the max height, it's just from a random sample" in the method's doctoring?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added (hopefully I did the docstring right, there aren't a lot of them around.)

std::unique_ptr<CDBIterator> pcursor(NewIterator());
*nHeight = 0;
int i = 0;
pcursor->Seek(std::make_pair(DB_BLOCK_INDEX, uint256()));
while (pcursor->Valid()) {
if (ShutdownRequested()) return false;
std::pair<uint8_t, uint256> key;
if (pcursor->GetKey(key) && key.first == DB_BLOCK_INDEX) {
i++;
if (i > 10'000) {
// Under the (accurate) assumption hat the headers on disk are effectively in random height order,
// we have a good-enough (conservative) estimate of the max height very quickly, and don't need to
// waste more time. Shortcutting like this will cause us to keep a few extra headers, which is fine.
break;
}
CDiskBlockIndex diskindex;
if (pcursor->GetValue(diskindex)) {
if (diskindex.nHeight > *nHeight) {
*nHeight = diskindex.nHeight;
}
pcursor->Next();
} else {
return error("%s: failed to read value", __func__);
}
} else {
break;
}
}
return true;
}

bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function<CBlockIndex*(const uint256&)> insertBlockIndex, int trim_below_height)
{
std::unique_ptr<CDBIterator> pcursor(NewIterator());

pcursor->Seek(std::make_pair(DB_BLOCK_INDEX, uint256()));

int n_untrimmed = 0;
int n_total = 0;

// Load m_block_index
while (pcursor->Valid()) {
if (ShutdownRequested()) return false;
Expand All @@ -332,19 +367,27 @@ bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams,
pindexNew->nTime = diskindex.nTime;
pindexNew->nBits = diskindex.nBits;
pindexNew->nNonce = diskindex.nNonce;
pindexNew->proof = diskindex.proof;
pindexNew->nStatus = diskindex.nStatus;
pindexNew->nTx = diskindex.nTx;
pindexNew->m_dynafed_params = diskindex.m_dynafed_params;
pindexNew->m_signblock_witness = diskindex.m_signblock_witness;

const uint256 block_hash = pindexNew->GetBlockHash();
// Only validate one of every 1000 block header for sanity check
if (pindexNew->nHeight % 1000 == 0 &&
block_hash != consensusParams.hashGenesisBlock &&
!CheckProof(pindexNew->GetBlockHeader(), consensusParams)) {
return error("%s: CheckProof: %s, %s", __func__, block_hash.ToString(), pindexNew->ToString());

n_total++;
if (diskindex.nHeight >= trim_below_height) {
n_untrimmed++;
pindexNew->proof = diskindex.proof;
pindexNew->m_dynafed_params = diskindex.m_dynafed_params;
pindexNew->m_signblock_witness = diskindex.m_signblock_witness;

const uint256 block_hash = pindexNew->GetBlockHash();
// Only validate one of every 1000 block header for sanity check
if (pindexNew->nHeight % 1000 == 0 &&
block_hash != consensusParams.hashGenesisBlock &&
!CheckProof(pindexNew->GetBlockHeader(), consensusParams)) {
return error("%s: CheckProof: %s, %s", __func__, block_hash.ToString(), pindexNew->ToString());
}
} else {
pindexNew->m_trimmed = true;
}

pcursor->Next();
} else {
return error("%s: failed to read value", __func__);
Expand All @@ -354,6 +397,7 @@ bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams,
}
}

LogPrintf("LoadBlockIndexGuts: loaded %d total / %d untrimmed (fully in-memory) headers\n", n_total, n_untrimmed);
return true;
}

Expand Down
3 changes: 2 additions & 1 deletion src/txdb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,9 @@ class CBlockTreeDB : public CDBWrapper
void ReadReindexing(bool &fReindexing);
bool WriteFlag(const std::string &name, bool fValue);
bool ReadFlag(const std::string &name, bool &fValue);
bool LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function<CBlockIndex*(const uint256&)> insertBlockIndex);
bool LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function<CBlockIndex*(const uint256&)> insertBlockIndex, int trim_below_height);
// ELEMENTS:
bool WalkBlockIndexGutsForMaxHeight(int* nHeight);
bool ReadPAKList(std::vector<std::vector<unsigned char> >& offline_list, std::vector<std::vector<unsigned char> >& online_list, bool& reject);
bool WritePAKList(const std::vector<std::vector<unsigned char> >& offline_list, const std::vector<std::vector<unsigned char> >& online_list, bool reject);
};
Expand Down
14 changes: 13 additions & 1 deletion src/validation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4110,7 +4110,19 @@ bool BlockManager::LoadBlockIndex(
CBlockTreeDB& blocktree,
std::set<CBlockIndex*, CBlockIndexWorkComparator>& block_index_candidates)
{
if (!blocktree.LoadBlockIndexGuts(consensus_params, [this](const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return this->InsertBlockIndex(hash); }))
int trim_below_height = 0;
if (fTrimHeaders) {
int max_height = 0;
if (!blocktree.WalkBlockIndexGutsForMaxHeight(&max_height)) {
LogPrintf("LoadBlockIndex: Somehow failed to WalkBlockIndexGutsForMaxHeight.\n");
return false;
}

int must_keep_headers = (consensus_params.total_valid_epochs + 2) * consensus_params.dynamic_epoch_length;
int extra_headers_buffer = consensus_params.dynamic_epoch_length * 2; // XXX arbitrary
trim_below_height = max_height - must_keep_headers - extra_headers_buffer;
}
if (!blocktree.LoadBlockIndexGuts(consensus_params, [this](const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return this->InsertBlockIndex(hash); }, trim_below_height))
return false;

// Calculate nChainWork
Expand Down