Check for missing undo files at startup, making the check somewhat more ...

rdponticelli commented at 7:48 pm on July 12, 2014: contributor

…efficient.

This is also an incremental step in making an index pruner for #4481

sipa commented at 8:12 pm on July 12, 2014: member

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

in src/main.cpp: in d000f7f0c7 outdated

2983@@ -2984,19 +2984,30 @@ bool static LoadBlockIndexDB()
2984 
2985     // Check presence of blk files
2986     LogPrintf("Checking all blk files are present...\n");
2987-    set<int> setBlkDataFiles;
2988+    map<int, bool> mapBlkDataFileReadable,mapBlkUndoFileReadable;

sipa commented at 6:04 pm on July 13, 2014:

Why the map and bool? Just a set for both suffices.

rdponticelli commented at 6:09 pm on July 13, 2014:

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.

rdponticelli commented at 12:17 pm on July 16, 2014: contributor

Closing this, as it has been rebased, refactored and pushed as part of #4481.

rdponticelli closed this on Jul 16, 2014

rdponticelli commented at 10:13 pm on July 17, 2014: contributor

I simplified this code. Reopening.

rdponticelli reopened this on Jul 17, 2014

laanwj added the label UTXO Db and Indexes on Jul 31, 2014

in src/main.cpp: in 18fe307de7 outdated

3006@@ -3025,6 +3007,50 @@ bool static LoadBlockIndexDB()
3007         DateTimeStrFormat("%Y-%m-%d %H:%M:%S", chainActive.Tip()->GetBlockTime()),
3008         Checkpoints::GuessVerificationProgress(chainActive.Tip()));
3009 
3010+    // Check presence of essential data
3011+    LogPrintf("Checking essential data is accesible...\n");

laanwj commented at 3:54 pm on August 4, 2014:

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.

in src/main.cpp: in 8b8d083039 outdated

2984+}
2985 
2986+bool CheckBlockFiles()
2987+{
2988+    // Check presence of essential data
2989+    LogPrintf("Checking essential data is accesible...\n");

Diapolo commented at 8:32 am on August 5, 2014:

Idea: Checking required block data is available...?

in src/main.cpp: in 8b8d083039 outdated

3021+        else
3022+        {
3023+            // Block 0 doesn't have undo data.
3024+            if (pindex->nHeight)
3025+            {
3026+                LogPrintf("Missing undo for block: %i\n", pindex->nHeight);

Diapolo commented at 8:32 am on August 5, 2014:

I would prepend an Error: and also log which file exactly is missing (also a suggestion for normal block files).

rdponticelli commented at 9:37 pm on August 5, 2014:

“Error:” prepended, but notice that this condition doesn’t mean a file is missing, but that we don’t have the bit HAVE_UNDO set in the index for this block. If the file were missing, it would be logged by the function called in line 3015. So, there’s no need to log a file name here, IMHO.

in src/main.cpp: in 43795dff47 outdated

3021+        else
3022+        {
3023+            // Block 0 doesn't have undo data.
3024+            if (pindex->nHeight)
3025+            {
3026+                LogPrintf("Error: Missing undo for block: %i\n", pindex->nHeight);

Diapolo commented at 6:06 am on August 6, 2014:

Nit: undo data also fits better in here, but no need to add, if this is going to be merged soon.

rdponticelli commented at 5:52 pm on August 6, 2014: contributor

Feedback addressed, and some bugs were fixed, and some things improved along the way. Would this be mergeable?

rdponticelli commented at 2:10 pm on August 14, 2014: contributor

Code style fixed to adhere to current standards.

rdponticelli force-pushed on Aug 26, 2014

BitcoinPullTester commented at 1:39 pm on August 26, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4515_bac3df7ed9c9627d54c9887702182adf9adf21df/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

rdponticelli closed this on Sep 18, 2014

Refactor startup checks onto a separate function. 7694ec98a9

Split the logic to check if a block file can be opened to a separate function. 19f870d867

Add undo data to checks to be sure all required info can be opened. 30ff651bc2

Scan active chain instead of map of blocks. 899cbc76b2

rdponticelli commented at 1:38 pm on November 20, 2014: contributor

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.

rdponticelli reopened this on Nov 20, 2014

rdponticelli force-pushed on Nov 20, 2014

in src/main.cpp: in 899cbc76b2

2897+    // Check presence of blk files
2898+    LogPrintf("Checking all blk files are present...\n");
2899+    set<int> setRequiredDataFilesAreOpenable;
2900+    for (CBlockIndex* pindex = chainActive.Tip(); pindex && pindex->pprev; pindex = pindex->pprev) {
2901+        if (pindex->nStatus & BLOCK_HAVE_DATA && pindex->nStatus & BLOCK_HAVE_UNDO) {
2902+            if (!setRequiredDataFilesAreOpenable.count(pindex->nFile)) {

laanwj commented at 1:20 pm on March 18, 2015:

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:

 0std::map<int,uint32_t> mapBlkDataFiles;
 1BOOST_FOREACH(const PAIRTYPE(uint256, CBlockIndex*)& item, mapBlockIndex)
 2{
 3    CBlockIndex* pindex = item.second;
 4    mapBlkDataFiles[pindex->nFile] |= pindex->nStatus;
 5}
 6for (std::map<int,uint32_t>::iterator it = mapBlkDataFiles.begin(); it != mapBlkDataFiles.end(); it++)
 7{
 8    if ((it->second & BLOCK_HAVE_DATA) && !BlockFileIsOpenable(it->first)) {
 9        LogPrintf("Error: Required block file %i can't be opened.\n", it->first);
10        return false;
11    }
12    if ((it->second & BLOCK_HAVE_UNDO) && !UndoFileIsOpenable(it->first)) {
13        LogPrintf("Error: Required undo file %i can't be opened.\n", it->first);
14        return false;
15    }
16}

in src/main.cpp: in 899cbc76b2

2872@@ -2891,6 +2873,48 @@ bool static LoadBlockIndexDB()
2873     return true;
2874 }
2875 
2876+bool BlockFileIsOpenable(int nFile)

laanwj commented at 1:21 pm on March 18, 2015:

These functions can be static

in src/main.h: in 899cbc76b2

172@@ -173,6 +173,8 @@ bool LoadExternalBlockFile(FILE* fileIn, CDiskBlockPos *dbp = NULL);
173 bool InitBlockIndex();
174 /** Load the block tree and coins database from disk */
175 bool LoadBlockIndex();
176+/** Check all required block files are present */

laanwj commented at 1:23 pm on March 18, 2015:

0/** Check all required block and undo files are present */

laanwj commented at 5:04 pm on March 24, 2015: member

Looks like this is not necessary for the current autoprune (#5863), so I’m closing this. Let me know if I’m wrong.

laanwj closed this on Mar 24, 2015

MarcoFalke locked this on Sep 8, 2021

Check for missing undo files at startup, making the check somewhat more … #4515