-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Force on-the-fly compaction during pertxout upgrade #10526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
utACK fe84334 |
|
Concept ACK |
|
Original chainstate (up to height=430234). After converting using master (and shutting down immediately): Converting using this pull (and shutting down immediately): In this test, it doesn't seem to make a significant difference. |
|
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. |
|
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 ***@***.***> 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.
|
|
Also, needs rebase. |
|
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 With master: So it seems the negative result last time was a fluke. utACK. |
|
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. |
|
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
|
If someone was running with master before this commit, how does their database ever get compacted? |
|
@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. |
|
@morcos Yea, I have one two weeks now. Perhaps we should also have some general compaction thing to clean up existing databases too.... |
|
I got some strange testing results...current master:
This PR:
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). |
|
@morcos hmm, I guess its just done when tables fill up, it should /eventually/ happen, but may take a long time, I suppose. |
|
@TheBlueMatt wow, that's some really long convert times |
|
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
<#10526 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAnoHj14tGPkhrDI1xEjbwZ03P1iikGrks5sSUjJgaJpZM4NvdNV>.
|
|
utACK efeb273, I tested an earlier version of the patch which should be equivalent, however. |
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
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
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
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
CompactRangefunction in the database wrapper, and invokes it after every batch of updates inCCoinsViewDB::Upgrade(). This lowers temporary disk usage during and after the upgrade.