Check for missing undo files at startup, making the check somewhat more ... #4515
Conversation
|
What if the first encountered block with file N doesn't have undo data, but the second one does? You'd skip checking it. EDIT: fixed |
| @@ -2984,19 +2984,30 @@ bool static LoadBlockIndexDB() | |||
|
|
|||
| // Check presence of blk files | |||
| LogPrintf("Checking all blk files are present...\n"); | |||
| set<int> setBlkDataFiles; | |||
| map<int, bool> mapBlkDataFileReadable,mapBlkUndoFileReadable; | |||
There was a problem hiding this comment.
Why the map and bool? Just a set for both suffices.
There was a problem hiding this comment.
Because if we go the #4481 route, we'll have to expand the logic to better manage the cases when the files are missing without failing.
|
Closing this, as it has been rebased, refactored and pushed as part of #4481. |
|
I simplified this code. Reopening. |
| @@ -3025,6 +3007,50 @@ bool static LoadBlockIndexDB() | |||
| DateTimeStrFormat("%Y-%m-%d %H:%M:%S", chainActive.Tip()->GetBlockTime()), | |||
| Checkpoints::GuessVerificationProgress(chainActive.Tip())); | |||
|
|
|||
| // Check presence of essential data | |||
| LogPrintf("Checking essential data is accesible...\n"); | |||
There was a problem hiding this comment.
Now that this has been moved to the end of the function, consider factoring it out to a separate function (eg. CheckForBlockFiles()) and calling that separately. After all, this is not part of loading the block index DB.
|
Feedback addressed, and some bugs were fixed, and some things improved along the way. Would this be mergeable? |
|
Code style fixed to adhere to current standards. |
e58bbc5 to
bac3df7
Compare
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4515_bac3df7ed9c9627d54c9887702182adf9adf21df/ for binaries and test log. |
|
I guess this is too late for 0.10, but reopening this anyway as I don't expect this code to change (a lot) any further, and it feels like it would make sense merging this independently of #4701. |
bac3df7 to
899cbc7
Compare
| set<int> setRequiredDataFilesAreOpenable; | ||
| for (CBlockIndex* pindex = chainActive.Tip(); pindex && pindex->pprev; pindex = pindex->pprev) { | ||
| if (pindex->nStatus & BLOCK_HAVE_DATA && pindex->nStatus & BLOCK_HAVE_UNDO) { | ||
| if (!setRequiredDataFilesAreOpenable.count(pindex->nFile)) { |
There was a problem hiding this comment.
It's not clear to me why you switched from the two-pass solution to a single loop, but from a readability viewpoint I'd prefer something like:
std::map<int,uint32_t> mapBlkDataFiles;
BOOST_FOREACH(const PAIRTYPE(uint256, CBlockIndex*)& item, mapBlockIndex)
{
CBlockIndex* pindex = item.second;
mapBlkDataFiles[pindex->nFile] |= pindex->nStatus;
}
for (std::map<int,uint32_t>::iterator it = mapBlkDataFiles.begin(); it != mapBlkDataFiles.end(); it++)
{
if ((it->second & BLOCK_HAVE_DATA) && !BlockFileIsOpenable(it->first)) {
LogPrintf("Error: Required block file %i can't be opened.\n", it->first);
return false;
}
if ((it->second & BLOCK_HAVE_UNDO) && !UndoFileIsOpenable(it->first)) {
LogPrintf("Error: Required undo file %i can't be opened.\n", it->first);
return false;
}
}|
Looks like this is not necessary for the current autoprune (#5863), so I'm closing this. Let me know if I'm wrong. |
...efficient.
This is also an incremental step in making an index pruner for #4481