Limit the impact of reorganisations on the database (fix #925) #930

pull sipa wants to merge 1 commits into bitcoin:master from sipa:minireorg changing 2 files +64 −17

sipa commented at 2:51 PM on March 11, 2012: member

Sometimes a new block arrives in a new chain that was already the best valid one, but wasn't marked that way. This happens for example when network rules change to recover after a fork.

In this case, it is not necessary to do the entire reorganisation inside a single db commit. These can become huge, and exceed the objects/lockers limits in bdb. This patch limits the blocks the actual reorganisation is applied to, and adds the next blocks afterwards in separate db transactions.

Mostly-closes #925.

Limit the impact of reorganisations on the database

Sometimes a new block arrives in a new chain that was already the
best valid one, but wasn't marked that way. This happens for example
when network rules change to recover after a fork.

In this case, it is not necessary to do the entire reorganisation
inside a single db commit. These can become huge, and exceed the
objects/lockers limits in bdb. This patch limits the blocks the
actual reorganisation is applied to, and adds the next blocks
afterwards in separate db transactions.

d68dcf741e

in src/main.cpp:None in 15dc624936 outdated

1456 | @@ -1457,6 +1457,31 @@ bool static Reorganize(CTxDB& txdb, CBlockIndex* pindexNew)
1457 |          printf("runCommand error: system(%s) returned %d\n", strCommand.c_str(), nErr);
1458 |  }
1459 |  
1460 | +// assumes pindexNew->pprev == pindexBest

gavinandresen commented at 4:00 PM on March 12, 2012:

"Unchecked" sounds scary. Maybe SetBestChainInner(). And an assert(pindexNew->pprev == pindexBest) would be good.

sipa commented at 8:26 PM on March 12, 2012:

Fixed.

in src/main.cpp:None in 15dc624936 outdated

1544 |          }
1545 | +
1546 | +        reverse(vpindexSecondary.begin(), vpindexSecondary.end());
1547 | +
1548 | +        // Connect futher blocks
1549 | +        BOOST_FOREACH(CBlockIndex *pindex, vpindexSecondary)

gavinandresen commented at 4:01 PM on March 12, 2012:

I believe there is a BOOST_REVERSE_FOREACH() that would eliminate the reverse() call.

sipa commented at 8:26 PM on March 12, 2012:

Fixed.

in src/main.cpp:None in 15dc624936 outdated
```
1509 | +        // list of blocks that need to be connected afterwards
1510 | +        std::vector<CBlockIndex*> vpindexSecondary;
1511 | +
1512 | +        // Reorganize is costly in terms of db load, as it works in a single db transaction.
1513 | +        // Try to limit how much needs to be done inside
1514 | +        while (pindexIntermediate->pprev && pindexIntermediate->pprev->bnChainWork > pindexBest->bnChainWork)
```
gavinandresen commented at 4:05 PM on March 12, 2012:

Mostly a note to myself... but I have a feeling we might soon implement 'gentle' block discouragement-- if we get another block/chain with equal work to the one we have now, perhaps orphan the existing one if we like the new one better (maybe: it removes more transactions from the memory pool than the current best, or it is from one of my mining consortium buddies, or has some of my wallet transactions in it).

Anyway, the ChainWork > condition here must match the condition elsewhere, and if we ever change it to something more complicated the condition here needs careful testing/thought.

sipa commented at 7:04 PM on March 12, 2012:

This can probably be abstracted into a common function later. What about an operator< / operator== for CBlockIndex ?
gmaxwell commented at 10:21 PM on March 12, 2012: contributor

Ack. Passes my testnet reorg tests.
gmaxwell referenced this in commit 89cccc83f8 on Mar 12, 2012
gmaxwell merged this on Mar 12, 2012
gmaxwell closed this on Mar 12, 2012
coblee referenced this in commit ea1c925697 on Jul 17, 2012
suprnurd referenced this in commit 072fd72ca3 on Dec 5, 2017
ptschip referenced this in commit 987991bf3c on Feb 4, 2018
lateminer referenced this in commit 5061b486c2 on Oct 30, 2019
sipa referenced this in commit c020cbaa5c on Jul 14, 2021
DrahtBot locked this on Sep 8, 2021

Contributors

sipa

gavinandresen

gmaxwell

Linked (view graph)

#925 Large blockchain re-orgs can fail