Skip to content
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

Delay initial pruning until after wallet init #6356

Merged
merged 1 commit into from
Aug 3, 2015

Conversation

ajweiss
Copy link
Contributor

@ajweiss ajweiss commented Jun 30, 2015

Don't prune until any wallet rescanning has taken place to avoid
potentially pruning blocks that the wallet rescan may need.

Fixes #6345

@jonasschnelli
Copy link
Contributor

Nice.
utACK. Have plans to test this soon.

@laanwj laanwj added the Wallet label Jul 2, 2015
@jonasschnelli
Copy link
Contributor

Tested ACK (was hard to reproduce).

Prove (mind the rescanning before the pruning):

Opening LevelDB in /home/gitian/.bitcoinPruned/blocks/index
Opened LevelDB successfully
Opening LevelDB in /home/gitian/.bitcoinPruned/chainstate
Opened LevelDB successfully
LoadBlockIndexDB: last block file = 300
LoadBlockIndexDB: last block file info: CBlockFileInfo(blocks=159, size=113623915, heights=365157...365313, time=2015-07-13...2015-07-14)
Checking all blk files are present...
LoadBlockIndexDB: transaction index disabled
LoadBlockIndexDB: hashBestChain=00000000000000000249e73efe3b8b525bb8aca95b9abc8f8cb2287d6eaec02b height=365313 date=2015-07-14 19:24:07 progress=0.999956
init message: Verifying blocks...
Verifying last 288 blocks at level 3
No coin database inconsistencies in last 4 blocks (5533 transactions)
 block index           43301ms
init message: Loading wallet...
nFileVersion = 119900
Keys: 102 plaintext, 0 encrypted, 102 w/ metadata, 102 total
 wallet                   90ms
init message: Rescanning...
Rescanning last 325539 blocks (from block 39774)...
Still rescanning. At block 352297. Progress=0.871212
Still rescanning. At block 357888. Progress=0.929899
Still rescanning. At block 364980. Progress=0.996939
 rescan               185582ms
Unsetting NODE_NETWORK on prune mode
Prune: target=550MiB actual=405MiB diff=144MiB max_prune_height=365025 removed 298 blk/rev pairs
Prune: UnlinkPrunedFiles deleted blk/rev (00000)
Prune: UnlinkPrunedFiles deleted blk/rev (00001)
Prune: UnlinkPrunedFiles deleted blk/rev (00002)
Prune: UnlinkPrunedFiles deleted blk/rev (00003)
Prune: UnlinkPrunedFiles deleted blk/rev (00004)
Prune: UnlinkPrunedFiles deleted blk/rev (00005)
Prune: UnlinkPrunedFiles deleted blk/rev (00006)

@laanwj
Copy link
Member

laanwj commented Jul 27, 2015

Looks good to me. utACK

@laanwj
Copy link
Member

laanwj commented Jul 27, 2015

A small nit: this puts the pruning operation under the init message Loading Wallet, and also under Step 8: load wallet in the source code - while it has nothing to do with the wallet.
As this can take a while, and also to keep clear separation, it may be better to add an InitMessage call on top and put it in its own logical step.

@ajweiss
Copy link
Contributor Author

ajweiss commented Jul 27, 2015

SGTM. I lean towards something a little more generic like "Performing data directory maintenance..." as opposed to "Pruning blockstore..." as it would allow for other similar data directory management tasks to be added in future without creating more translation work. Although, the others aren't as generic. Do you have a preference?

Don't prune until any wallet rescanning has taken place to avoid
potentially pruning blocks that the wallet rescan may need.
@ajweiss ajweiss force-pushed the ajw_1506_prune_after_walletscan branch from c3803f8 to f0cba6f Compare July 29, 2015 18:02
@ajweiss
Copy link
Contributor Author

ajweiss commented Jul 29, 2015

Ok, pushed. I used "Pruning blockstore..." for now as it's more in line with the descriptiveness of the other init messages. If there's objection, I can change it to something more generic.

@laanwj
Copy link
Member

laanwj commented Aug 3, 2015

@ajweiss Agreed that a more general categorization is a good idea, also to make localization easier. But yes at the moment the messages are very specific, so this is most consistent.

@laanwj laanwj merged commit f0cba6f into bitcoin:master Aug 3, 2015
laanwj added a commit that referenced this pull request Aug 3, 2015
f0cba6f Delay initial pruning until after wallet init (Adam Weiss)
@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.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fresh prune with out-of-sync wallet can fail (but shouldn't)
3 participants