Page MenuHomePhabricator

Correct units for "-dbcache" and "-prune" and upper prune limit
AbandonedPublic

Authored by PiRK on Oct 6 2020, 14:06.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Summary

All dbcache-related values in the code are measured in MiB (not in
megabytes, MB) or in bytes.
The GUI "-prune" values in GB are translated to the node values in MiB
correctly. The maximum of the "-prune" QSpinBox is not limited by the
default value of 99 (GB).
Also, this improves log readability.

Backport of Core PR15163 and PR15801

The second PR fixes a bug in the first one (bug description) and then extends the prune limit even further.

Test Plan

Build and install bitcoin-qt, check that this setting is not limited to current blockchain size.

Diff Detail

Event Timeline

Owners added a reviewer: Restricted Owners Package.Oct 6 2020, 14:06
PiRK requested review of this revision.Oct 6 2020, 14:06

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

deadalnix requested changes to this revision.Oct 6 2020, 14:57
deadalnix added a subscriber: deadalnix.

Point to the specific commit. This is clearly not the whole PR.

This revision now requires changes to proceed.Oct 6 2020, 14:57
PiRK retitled this revision from GUI: Options: Remove the upper-bound limit from pruning size setting to Correct units for "-dbcache" and "-prune" and upper prune limit.
PiRK edited the summary of this revision. (Show Details)

squash trivial remaining commit to merge PR15163 and PR15801

deadalnix requested changes to this revision.Oct 7 2020, 18:45

Ok now I'm getting a bunch more changes from 4ddeb2f860eee98fbe94725ea8885368068a03f2 and not the change to ui->pruneSize  for instance. This in a state where tracking every individual changes is tedious. Please make sure you go over this and do it well. Doing a ton of round trip is just wasting both of our time, and in addition, it is error prone.

Please point to the discussion about the bug in the description rather than point to a commit that will no help anyone figuring out what happened.

This revision now requires changes to proceed.Oct 7 2020, 18:45

This in a state where tracking every individual changes is tedious. Please make sure you go over this and do it well. Doing a ton of round trip is just wasting both of our time, and in addition, it is error prone.

I'm not sure what you are asking me to do. This is the result of squashing all 3 commits from both pull requests, as you suggested (https://reviews.bitcoinabc.org/D7781#183643). The only thing I'm not satisfied about is that arcanist decided to put the result of squashing in D7782 instead of D7781.

To try to clarify the situation, PR15163 does mostly nitpicking by changing MB into MiB in various strings, comments and variable names. But it also attempts to do something useful in the same commit : set a wider range for a widget allowing to set the prune size target, because the widget is by default set to range 0--99, and trying to set a larger value than 99 would not be accepted (it would be clipped to 99). Unfortunately, the piece of code setting that larger range is applied after the value is set, so it does not actually work.

PR 15801 than corrects the problem by moving the setRange code before the value is loaded, which is what should have been done in the first pull request. And then, in a second commit, the upper limit is raised even further to allow for values larger than the current blockchain size.

PR15163 has been backported separately in D7896, so I will just do PR15801 on its own.