Reset pblocktree before deleting LevelDB file #12401

1. 
   Sjors commented at 6:55 pm on February 9, 2018: member

   #11043 repaced:

   0delete pblocktree;
   1pblocktree = new CBlockTreeDB(nBlockTreeDBCache, false, fReset);
   

   With:

   0pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, false, fReset));
   

   This is problematic because new CBlockTreeDB tries to delete the existing file, which will fail with LOCK: already held by process if it’s still open. That’s the case for QT.

   When QT finds a problem with the index it will ask the user if they want to reindex. At that point it has already opened blocks/index. It then runs this while loop again with fReset = 1, resulting in the above error.

   This change makes that error go away, presumably because reset() without an argument closes the file.

2. 
   Sjors commented at 6:57 pm on February 9, 2018: member

   @laanwj if I’m correct then this regression was introduced after v0.15.1 and so we might need a new rc.
3. Sjors force-pushed on Feb 9, 2018
4. in src/init.cpp:1428 in 7bd20cf466 outdated

   1424@@ -1425,6 +1425,9 @@ bool AppInitMain()
   1425                 pcoinsTip.reset();
   1426                 pcoinsdbview.reset();
   1427                 pcoinscatcher.reset();
   1428+                if (fReset) {
   

   ---

   ---

   

   sipa commented at 7:26 pm on February 9, 2018:

   I don’t think the conditional is necessary. Also, perhaps add a comment that the reset is necessary to avoid temporarily having two CBlockTreeDB objects?

   ---

   

   Sjors commented at 10:48 am on February 10, 2018:

   I put the conditional there because under normal circumstances pblocktree.reset() is useless. However I don’t know what it does under the hood; maybe the performance overhead is negligible.
5. MarcoFalke added this to the milestone 0.16.0 on Feb 9, 2018
6. jonasschnelli added the label UTXO Db and Indexes on Feb 10, 2018
7. jonasschnelli added the label Block storage on Feb 10, 2018
8. jonasschnelli removed the label UTXO Db and Indexes on Feb 10, 2018
9. 
   Sjors commented at 10:50 am on February 10, 2018: member

   ~Should I change pcoinsTip, pcoinsdbview and pcoinscatcher in the same way?~ It’s not entirely clear to me what reset() actually does.
10. 
    Sjors commented at 10:52 am on February 10, 2018: member

    Also paging @practicalswift.
11. 
    practicalswift commented at 3:19 pm on February 10, 2018: contributor

    Thanks for the ping and thanks for finding+fixing this issue.

    utACK 7bd20cf466dfa5b2cccef00d1cd05bfbb8634f0d modulo the unnecessary conditional.

12. Sjors force-pushed on Feb 10, 2018
13. 
    Sjors commented at 4:29 pm on February 10, 2018: member

    Conditional removed.
14. 
    MarcoFalke commented at 4:42 pm on February 10, 2018: member

    utACK 8b986dc5696db1d623465cb13d036bac722762e6
15. MarcoFalke added the label Needs backport on Feb 10, 2018
16. 
    practicalswift commented at 9:45 am on February 11, 2018: contributor

    utACK 8b986dc5696db1d623465cb13d036bac722762e6
17. 
    laanwj commented at 10:35 am on February 11, 2018: member

    utACK
18. in src/init.cpp:1430 in 8b986dc569 outdated

    1424@@ -1425,6 +1425,7 @@ bool AppInitMain()
    1425                 pcoinsTip.reset();
    1426                 pcoinsdbview.reset();
    1427                 pcoinscatcher.reset();
    1428+                pblocktree.reset();
    

    ---

    ---

    

    promag commented at 10:44 am on February 11, 2018:

    Add comment here?

    ---

    

    Sjors commented at 11:14 am on February 11, 2018:

    Added comment.
19. 
    promag commented at 10:44 am on February 11, 2018: member

    utACK 8b986dc.
20. Reset pblocktree before deleting LevelDB file a8b5d20f4f
21. Sjors force-pushed on Feb 11, 2018
22. 
    MarcoFalke commented at 9:44 pm on February 11, 2018: member

    utACK a8b5d20f4f171828b2bd70ab2405c42b1e452e5b
23. laanwj merged this on Feb 12, 2018
24. laanwj closed this on Feb 12, 2018

    ---

25. laanwj referenced this in commit 79313d2e20 on Feb 12, 2018
26. laanwj referenced this in commit d44cd7ed4b on Feb 12, 2018
27. Sjors deleted the branch on Feb 12, 2018
28. MarcoFalke removed the label Needs backport on Feb 12, 2018
29. HashUnlimited referenced this in commit 2c8f81dc63 on Mar 16, 2018
30. PastaPastaPasta referenced this in commit 7a795d3f01 on Mar 14, 2020
31. PastaPastaPasta referenced this in commit 8b13a956bc on Mar 14, 2020
32. PastaPastaPasta referenced this in commit 5e19f32b6b on Mar 15, 2020
33. deadalnix referenced this in commit 5ba4892705 on Jun 18, 2020
34. ftrader referenced this in commit 9b09266ea4 on Aug 17, 2020
35. ckti referenced this in commit c32dc3e689 on Mar 28, 2021
36. DrahtBot locked this on Sep 8, 2021

Contributors
Sjors sipa practicalswift MarcoFalke laanwj promag

Labels
Block storage

Milestone
0.16.0