Force on-the-fly compaction during pertxout upgrade #10526

sipa commented at 6:49 PM on June 4, 2017: member

It seems that LevelDB tends to leave the old "per txid" UTXO entries in the database lying around for a significant amount of time during and after the per-txout upgrade. This introduces a CompactRange function in the database wrapper, and invokes it after every batch of updates in CCoinsViewDB::Upgrade(). This lowers temporary disk usage during and after the upgrade.

fanquake added the label UTXO Db and Indexes on Jun 5, 2017

laanwj commented at 12:47 PM on June 5, 2017: member

utACK https://github.com/bitcoin/bitcoin/pull/10526/commits/fe84334135afa79845d18e472b71536cca72fd0d

gmaxwell commented at 8:26 AM on June 6, 2017: contributor

Concept ACK

laanwj commented at 1:12 PM on June 7, 2017: member

Original chainstate (up to height=430234).

2.4G    testbtc.olddb/chainstate

After converting using master (and shutting down immediately):

2017-06-07 12:54:30 Upgrading database...
2017-06-07 13:09:39 LoadBlockIndexDB: last block file = 628

4.5G    chainstate

Converting using this pull (and shutting down immediately):

2017-06-07 12:29:37 Upgrading database...
2017-06-07 12:41:50 LoadBlockIndexDB: last block file = 628

4.4G    testbtc.newdb/chainstate

In this test, it doesn't seem to make a significant difference.

sipa commented at 7:54 AM on July 17, 2017: member

I wonder what to do here. Without any forced compaction, it's likely that many nodes will remain with a database that still contains table files for the old rows for an extended period of time.

TheBlueMatt commented at 1:25 PM on July 17, 2017: member

Looks like this needed a 15 tag... If it's not too late let's call it a bugfix, tag it 15, and try to get it in this week.

On July 17, 2017 3:54:35 AM EDT, Pieter Wuille notifications@github.com wrote:

I wonder what to do here. Without any forced compaction, it's likely that many nodes will remain with a database that still contains table files for the old rows for an extended period of time.

TheBlueMatt commented at 2:18 PM on July 17, 2017: member

Also, needs rebase.

laanwj added this to the milestone 0.15.0 on Jul 20, 2017

laanwj commented at 10:06 AM on July 24, 2017: member

After converting up to block height=430234 (same dataset as last time), hardwiring shutdown immediately after the block database processing.

Tested with rebased version here: https://github.com/laanwj/bitcoin/tree/2017_07_sipa_onthefly

2.1G    chainstate/

With master:

4.3G    chainstate/

So it seems the negative result last time was a fluke. utACK.

laanwj commented at 12:29 PM on July 24, 2017: member

I repeated a few times, and it seems that it's like a coin toss. Sometimes it ends up as 4.4G, sometimes as 2.1G. When the former, every time after running bitcoind again, even terminating immediately after chainstate processing, it was at 2.1G after shutting down. It might be that a cleanup is re-triggered when re-opening the database, or when closing it.

sipa force-pushed on Jul 24, 2017

sipa commented at 10:03 PM on July 24, 2017: member

Rebased. I've changed the final compact request to be for the entire per-tx range of keys, rather than just the last group updated. Maybe that triggers compaction sooner. Or not.

in src/txdb.cpp:378 in e9cccbf336 outdated

 374 | @@ -375,6 +375,8 @@ bool CCoinsViewDB::Upgrade() {
 375 |      CDBBatch batch(db);
 376 |      uiInterface.SetProgressBreakAction(StartShutdown);
 377 |      int reportDone = 0;
 378 | +    std::pair<unsigned char, uint256> key;

morcos commented at 5:25 PM on July 27, 2017:

Is this meant to be shadowed by the key defined 7 lines down?

sipa commented at 4:08 AM on July 28, 2017:

Fixed.

morcos commented at 5:52 PM on July 27, 2017: member

If someone was running with master before this commit, how does their database ever get compacted?

TheBlueMatt commented at 5:56 PM on July 27, 2017: member

@morcos leveldb does automatic background compaction.

morcos commented at 6:22 PM on July 27, 2017: member

@morcos leveldb does automatic background compaction.

when? I have nodes running for a week that have 5.8G chainstate despite recent repeated stopping and starting.

gmaxwell commented at 7:03 PM on July 27, 2017: contributor

@morcos Yea, I have one two weeks now. Perhaps we should also have some general compaction thing to clean up existing databases too....

TheBlueMatt commented at 7:05 PM on July 27, 2017: member

I got some strange testing results...current master:

Convert time 26:30, final chainstate size 3.5G
Convert time 28:05, final chainstate size 3.5G

This PR:

Convert time: 16:51, final chainstate size 5.9G (down to 2.7 immediately upon restart)
Convert time: 16:11, final chainstate size 5.9G (down to 2.7 immediately upon restart)

Either way, I dont care about the excess usage if its faster and gets cleared upon restart (might get cleared in the background, too, i just made bitcoind do a clean exit right after chain state gets loaded).

TheBlueMatt commented at 7:05 PM on July 27, 2017: member

@morcos hmm, I guess its just done when tables fill up, it should /eventually/ happen, but may take a long time, I suppose.

morcos commented at 2:34 AM on July 28, 2017: member

@TheBlueMatt wow, that's some really long convert times My convert times with or without this PR were on the order of between 4 mins and 7 mins with some variation.

TheBlueMatt commented at 2:36 AM on July 28, 2017: member

Heh, thats what putting a database on spinning rust on a CoW filesystem does to you....I've seen it much faster under other conditions.

On 07/27/17 22:34, Alex Morcos wrote:

@TheBlueMatt https://github.com/thebluematt wow, that's some really long convert times My convert times with or without this PR were on the order of between 4 mins and 7 mins with some variation.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/10526#issuecomment-318540082, or mute the thread https://github.com/notifications/unsubscribe-auth/AAnoHj14tGPkhrDI1xEjbwZ03P1iikGrks5sSUjJgaJpZM4NvdNV.

Force on-the-fly compaction during pertxout upgrade efeb273305

sipa force-pushed on Jul 28, 2017

TheBlueMatt commented at 1:52 PM on July 30, 2017: member

utACK efeb273305e3e4d2c42e1e153de83c1cb6f0a28c, I tested an earlier version of the patch which should be equivalent, however.

laanwj merged this on Aug 1, 2017

laanwj closed this on Aug 1, 2017

laanwj referenced this in commit 754aa02b8a on Aug 1, 2017

codablock referenced this in commit 80a0277fa6 on Sep 27, 2017

codablock referenced this in commit 5ce06bfc90 on Oct 12, 2017

codablock referenced this in commit 9b624aa520 on Oct 26, 2017

codablock referenced this in commit 892ae5f732 on Oct 26, 2017

codablock referenced this in commit 1ea77a2b92 on Oct 26, 2017

codablock referenced this in commit 8b30c008fd on Oct 30, 2017

codablock referenced this in commit d0eef10342 on Oct 31, 2017

codablock referenced this in commit 585666d5a8 on Oct 31, 2017

codablock referenced this in commit 4102211a39 on Oct 31, 2017

UdjinM6 referenced this in commit dc4ee5f38e on Nov 8, 2017

MarcoFalke locked this on Sep 8, 2021