Skip to content

Conversation

@sipa
Copy link
Member

@sipa sipa commented Jun 4, 2017

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.

@laanwj
Copy link
Member

laanwj commented Jun 5, 2017

utACK fe84334

@gmaxwell
Copy link
Contributor

gmaxwell commented Jun 6, 2017

Concept ACK

@laanwj
Copy link
Member

laanwj commented Jun 7, 2017

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
Copy link
Member Author

sipa commented Jul 17, 2017

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
Copy link
Contributor

TheBlueMatt commented Jul 17, 2017 via email

@TheBlueMatt
Copy link
Contributor

Also, needs rebase.

@laanwj laanwj added this to the 0.15.0 milestone Jul 20, 2017
@laanwj
Copy link
Member

laanwj commented Jul 24, 2017

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
Copy link
Member

laanwj commented Jul 24, 2017

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
Copy link
Member Author

sipa commented Jul 24, 2017

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.

src/txdb.cpp Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@morcos
Copy link
Contributor

morcos commented Jul 27, 2017

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

@TheBlueMatt
Copy link
Contributor

@morcos leveldb does automatic background compaction.

@morcos
Copy link
Contributor

morcos commented Jul 27, 2017

@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
Copy link
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
Copy link
Contributor

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
Copy link
Contributor

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

@morcos
Copy link
Contributor

morcos commented Jul 28, 2017

@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
Copy link
Contributor

TheBlueMatt commented Jul 28, 2017 via email

@TheBlueMatt
Copy link
Contributor

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

@laanwj laanwj merged commit efeb273 into bitcoin:master Aug 1, 2017
laanwj added a commit that referenced this pull request Aug 1, 2017
efeb273 Force on-the-fly compaction during pertxout upgrade (Pieter Wuille)

Pull request description:

  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.

Tree-SHA512: fbf964c0a33f4e73709c999c8a2bfdef974779c15820907398a2f8828f5fa3e4e153ddd9031d6fc5083be81e22b999b9bd826fd063ad8b88f55c5e8342503290
codablock pushed a commit to codablock/dash that referenced this pull request Oct 26, 2017
efeb273 Force on-the-fly compaction during pertxout upgrade (Pieter Wuille)

Pull request description:

  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.

Tree-SHA512: fbf964c0a33f4e73709c999c8a2bfdef974779c15820907398a2f8828f5fa3e4e153ddd9031d6fc5083be81e22b999b9bd826fd063ad8b88f55c5e8342503290
codablock pushed a commit to codablock/dash that referenced this pull request Oct 31, 2017
efeb273 Force on-the-fly compaction during pertxout upgrade (Pieter Wuille)

Pull request description:

  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.

Tree-SHA512: fbf964c0a33f4e73709c999c8a2bfdef974779c15820907398a2f8828f5fa3e4e153ddd9031d6fc5083be81e22b999b9bd826fd063ad8b88f55c5e8342503290
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants