Always flush block and undo when switching to new file #6948

1. 
   sipa commented at 11:21 PM on November 4, 2015: member

   Previously, the undo files weren't being flushed during a reindex because fKnown was set to true in FindBlockPos. That is the correct behaviour for block files as they aren't being touched, but undo files are touched.

   This changes the behaviour to always flush when switching to a new file (even for block files, though that isn't really necessary).

   Intended to fix #6923.

2. 
   sipa commented at 11:23 PM on November 4, 2015: member

   @jonasschnelli @laanwj You may want to test this.

3. sipa force-pushed on Nov 4, 2015
4. Always flush block and undo when switching to new file

   Previously, the undo weren't being flushed during a reindex because
   fKnown was set to true in FindBlockPos. That is the correct behaviour
   for block files as they aren't being touched, but undo files are
   touched.
   
   This changes the behaviour to always flush when switching to a new file
   (even for block files, though that isn't really necessary).

   22e780737d
5. sipa force-pushed on Nov 4, 2015
6. 
   laanwj commented at 8:20 AM on November 5, 2015: member

   I expect that this fixes the issue. #6923 seemed to be a case of a truncated undo file (there was a strangely-sized old undo file and a very small new one). Although I don't fully remember if it happened during -reindex. May well be the case.

   Will test.

7. laanwj added the label Block storage on Nov 5, 2015
8. morcos cross-referenced this on Nov 5, 2015 from issue Require nLastBlockFile to be the highest numbered file. by morcos
9. in src/main.cpp:None in 22e780737d

   2525 | @@ -2528,7 +2526,14 @@ bool FindBlockPos(CValidationState &state, CDiskBlockPos &pos, unsigned int nAdd
   2526 |          pos.nPos = vinfoBlockFile[nFile].nSize;
   2527 |      }
   2528 |  
   2529 | -    nLastBlockFile = nFile;
   2530 | +    if (nFile != nLastBlockFile) {
   2531 | +        if (!fKnown) {
   2532 | +            LogPrintf("Leaving block file %i: %s\n", nFile, vinfoBlockFile[nFile].ToString());
   2533 | +        }
   2534 | +        FlushBlockFile(!fKnown);
   

   ---

   ---

   

   morcos commented at 3:10 PM on November 5, 2015:

   So on a reindex this means undo files still won't be truncated I suppose?

10. 
    morcos commented at 3:21 PM on November 5, 2015: member

    utACK

    Would an alternative option be to at the end of the reindex loop, go through and flush all the undo files. It seems if you crash during a reindex, you don't care about the state of the undo files, its only if you were to crash after reindex before they were flushed that you would have a problem?

    Such alternative option would potentially have problems in the event of new blocks being written during reindex, but thats already a problem, see #6691.

11. 
    jgarzik commented at 3:44 PM on November 5, 2015: contributor

    tested ACK

12. 
    jonasschnelli commented at 3:44 PM on November 5, 2015: contributor

    Tested ACK.

13. gmaxwell added this to the milestone 0.11.0 on Nov 5, 2015
14. gmaxwell added this to the milestone 0.12.0 on Nov 5, 2015
15. gmaxwell removed this from the milestone 0.11.0 on Nov 5, 2015
16. gmaxwell added this to the milestone 0.11.0 on Nov 5, 2015
17. gmaxwell removed this from the milestone 0.12.0 on Nov 5, 2015
18. gmaxwell added this to the milestone 0.12.0 on Nov 5, 2015
19. gmaxwell removed this from the milestone 0.11.0 on Nov 5, 2015
20. 
    laanwj commented at 9:33 PM on November 5, 2015: member

    It seems if you crash during a reindex, you don't care about the state of the undo file

    I don't understand your reasoning here. The point of this PR is to avoid corruption in the undo files when there is a crash during reindex. It can then pick up where it left off next time the application is launched.

21. laanwj merged this on Nov 5, 2015
22. laanwj closed this on Nov 5, 2015

    ---

23. laanwj referenced this in commit 849a7e6453 on Nov 5, 2015
24. 
    paveljanik commented at 7:59 AM on November 6, 2015: contributor

    This change brought a new compiler warning:

    main.cpp:2554:15: warning: comparison of integers of different signs: 'unsigned int' and 'int' [-Wsign-compare]
        if (nFile != nLastBlockFile) {
            ~~~~~ ^  ~~~~~~~~~~~~~~
    1 warning generated.
    

25. sipa referenced this in commit 4e895b08da on Nov 6, 2015
26. harding cross-referenced this on Nov 8, 2015 from issue [Docs] First-draft release notes for 0.11.2RC1 by harding
27. paveljanik cross-referenced this on Nov 16, 2015 from issue Fixed integer comparison warning. by CodeShark
28. luke-jr referenced this in commit b7fcc14008 on Dec 8, 2015
29. furszy cross-referenced this on Apr 21, 2021 from issue [Block Storage] Always flush block and undo when switching to new file by furszy
30. furszy referenced this in commit 22872bb27b on Apr 24, 2021
31. bitcoin locked this on Sep 8, 2021

Contributors

sipalaanwjmorcosjgarzikjonasschnellipaveljanik

Milestone
0.12.0