Skip to content

Conversation

luke-jr
Copy link
Collaborator

@luke-jr luke-jr commented Aug 7, 2025

No description provided.

Comment on lines +372 to 374
uint64_t prune_target_temporarily{(uint64_t)m_opts.prune_target_during_init};
if (prune_target_temporarily <= target) {
target = std::max(min_overall_target, (uint64_t)m_opts.prune_target_during_init);
Copy link

@kuegi kuegi Sep 7, 2025

Choose a reason for hiding this comment

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

why not also use it in the std::max ?

@kuegi
Copy link

kuegi commented Sep 8, 2025

why not go the extra mile and also check if target itself (not only the boost) is within the diskspace and cap it if necessary?

@Ataraxia009
Copy link

Concept NACK

I think overriding a genuinely inputted parameter by the user as an active argument: i.e.: -pruneduringinit should not be tampered with mid execution. Better to fail and let the user handle it as they see fit.

@luke-jr
Copy link
Collaborator Author

luke-jr commented Sep 17, 2025

why not go the extra mile and also check if target itself (not only the boost) is within the diskspace and cap it if necessary?

That could risk pruning more than the user ever wanted to, which is not repairable.

I think overriding a genuinely inputted parameter by the user as an active argument: i.e.: -pruneduringinit should not be tampered with mid execution. Better to fail and let the user handle it as they see fit.

There is currently no "smart" way to use as much disk as possible during IBD, and pruneduringinit is temporal by nature (after IBD completes, the normal prune setting will take over)

@Ataraxia009
Copy link

I'm having some trouble imagining why people would want pruning configurations to this level of nuance to begin with, what are some of the use cases of pruneduringinit just so we have a clearer picture?

Theres also a bunch of problems with the code i can see...
Also, -pruneduringinit would be broken for nodes using a utxo snapshot?
As far as I can tell, IsInitialBlockDownload() is false right after the snapshot reaches chaintip, so the boost won't apply for most of the IBD?

@luke-jr
Copy link
Collaborator Author

luke-jr commented Sep 20, 2025

Pruning forces the chainstate to commit to disk, significantly slowing down the initial sync. The original goal behind pruneduringinit is to avoid pruning longer, so sync completes faster, but immediately transition to the final prune size after it's done.

So if you have 5 TB free disk space, but don't want the node to use more than minimum, you can set -prune=550 -pruneduringinit=0 to disable pruning entirely during sync, and reduce to 550 MB later.

If we take free disk space into consideration as proposed by this PR, pruneduringinit=0 would be safe regardless of free disk space.

@Ataraxia009
Copy link

Pruning forces the chainstate to commit to disk, significantly slowing down the initial sync. The original goal behind pruneduringinit is to avoid pruning longer, so sync completes faster, but immediately transition to the final prune size after it's done.

Makes sense.

If we take free disk space into consideration as proposed by this PR, pruneduringinit=0 would be safe regardless of free disk space.

I am on the fence about overriding explicit user input over here because these users are anyways quite on the technical end of the spectrum but i dont have strong conviction either way

However, this is broken for snapshot loaded nodes because most of your IBD will ignore pruneduringinit.
I would probably address the bug first, which would change the way this GetPruneTargetForChainstate is written, having to redo the changes in this commit anyways

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.

3 participants