Skip to content

Add -pruneduringinit option to temporarily use another prune target during IBD#31845

Closed
luke-jr wants to merge 1 commit into
bitcoin:masterfrom
luke-jr:pruneduringinit
Closed

Add -pruneduringinit option to temporarily use another prune target during IBD#31845
luke-jr wants to merge 1 commit into
bitcoin:masterfrom
luke-jr:pruneduringinit

Conversation

@luke-jr
Copy link
Copy Markdown
Member

@luke-jr luke-jr commented Feb 12, 2025

Low prune sizes can force flushing long before dbcache is full, making a significant impact on real-world IBD time.

This new option allows a higher (or lower) prune size during IBD to mitigate this. When assumeutxo is active, the entire additional budget is allocated to the background chainstate.

@DrahtBot
Copy link
Copy Markdown
Contributor

DrahtBot commented Feb 12, 2025

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31845.

Reviews

See the guideline for information on the review process.

Type Reviewers
Approach NACK stickies-v

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #31260 (scripted-diff: Type-safe settings retrieval by ryanofsky)
  • #30214 (refactor: Improve assumeutxo state representation by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot
Copy link
Copy Markdown
Contributor

🚧 At least one of the CI tasks failed.
Debug: https://github.com/bitcoin/bitcoin/runs/37065721535

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

  • Possibly due to a silent merge conflict (the changes in this pull request being
    incompatible with the current code in the target branch). If so, make sure to rebase on the latest
    commit of the target branch.

  • A sanitizer issue, which can only be found by compiling with the sanitizer and running the
    affected test.

  • An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

@andrewtoth
Copy link
Copy Markdown
Contributor

andrewtoth commented Feb 12, 2025

After #28280 and #30039 how significant is the impact of this?

@luke-jr
Copy link
Copy Markdown
Member Author

luke-jr commented Feb 12, 2025

After #28280 and #30039 how significant is the impact of this?

I don't know, but logically I would expect this to still improve things significantly.

Copy link
Copy Markdown
Contributor

@l0rinc l0rinc left a comment

Choose a reason for hiding this comment

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

Before merging I would like to run an IBD, a reindex and an assumeUTXO on the branch.

My main concern currently (or rather the part I don't yet fully understand) is that if "during IBD there shouldn't be reorgs in the first place" (given a reliable block-header-first sync), why is this even configurable, why not always skip storage of blocks and undos completely during IBD (or at least reduce it to 6 blocks or something trivial) when prune is enabled (e.g. only enable it after the last 100 blocks or after assumevalid or something similar)?

Also note that #31144 is meant to optimize block writing speed considerably.

Comment thread src/node/blockmanager_args.cpp Outdated
Comment thread src/node/blockmanager_args.cpp Outdated
Comment thread src/init.cpp Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this compatible with -fastprune? If so, we should cover the new feature with tests, separating IBD pruning from up-to-date pruning (maybe in feature_index_prune.py)

Comment thread src/kernel/blockmanager_opts.h Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how many block do we absolutely need (for reorgs, I guess) during IBD?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

idk, not sure why it matters

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To see how low we can go safely to speed up IBD

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's backward. We need to go higher to speed up IBD. Lower slows it down.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I meant that if we could avoid writing blocks (and especially undos, which likely don't serve any purpose during IBD as far as I can tell), we could speed up IBD measurably for pruned nodes.

If the reason for writing blocks is to avoid flushing to LevelDB, there may be better ways (as mentioned, #31645 + sorting speeds up dumping from e.g. 30 minutes to 7).

Comment thread src/node/blockmanager_args.cpp Outdated
Comment thread src/node/blockmanager_args.cpp Outdated
Comment thread src/node/blockmanager_args.cpp Outdated
Comment thread src/node/blockstorage.cpp Outdated
Comment thread src/node/blockmanager_args.cpp Outdated
Comment thread src/node/blockmanager_args.cpp Outdated
@andrewtoth
Copy link
Copy Markdown
Contributor

Can't we just do an IBD without pruning, then shutdown and restart with a prune height set? Or just call pruneblockchain after we are done IBD?

Copy link
Copy Markdown
Contributor

@stickies-v stickies-v left a comment

Choose a reason for hiding this comment

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

Can't we just do an IBD without pruning

Disabling pruning is only possible on devices with sufficient disk space, but I agree that a simple restart seems like an easy enough workaround if benchmarks indicate that this a worthwhile performance improvement:

bitcoind --prune=4000 --stopatheight={current_tip} && bitcoind --prune=300

Approach NACK, I don't think this needs a separate startup option. If benchmarks agree, I think a PR that improves the --prune documentation would be worthwhile.

@sedited
Copy link
Copy Markdown
Contributor

sedited commented Jun 11, 2025

Approach NACK, I don't think this needs a separate startup option. If benchmarks agree, I think a PR that improves the --prune documentation would be worthwhile.

While I think I agree that this does not need a separate startup option, the current flushing behaviour does indeed feel backwards. That should just be independently addressed though. I also echo l0rincs intuition from before, why are we writing to disk at all? At least to the assume valid height, I can't think of any good reason to do so, and even beyond a target height calculated on the fly from the prune target and the best header, still seems acceptable. Maybe @sipa can help us understand the threshold tradeoffs here?

@achow101
Copy link
Copy Markdown
Member

This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.

Closing due to lack of interest.

@achow101 achow101 closed this Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants