Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Make CLI state pruning optional again #13017

Merged
merged 2 commits into from
Dec 26, 2022
Merged

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Dec 24, 2022

The state pruning setting is stored in the database when it is created. In later runs it is fine to drop the --state-pruning CLI argument as the setting is stored in the database. The state db will only return an error if the stored state pruning doesn't match the state pruning given via CLI. Users of Polkadot have reported exactly this behavior. They had their nodes running without --state-pruning, but initially they had created a database using archive. With the latest Polkadot release their node rejected to start, because the default value is 256 which is different to archive.

Recently we improved the state pruning CLI handling and accidentally made the state pruning value always present (as we set some default value for the clap). If we could find out if a user has passed a value or the default value was taken, we could keep the default value in the CLI interface, but clap isn't supporting this right now. So, we need to go back and make state_pruning an optional with the default written into the docs.

It also adds a test to ensure that we don't break this behavior again.

The state pruning setting is stored in the database when it is created. In later runs it is fine to
drop the `--state-pruning` CLI argument as the setting is stored in the database. The state db will
only return an error if the stored state pruning doesn't match the state pruning given via CLI.

Recently we improved the state pruning CLI handling and accidentally made the state pruning value
always present (as we set some default value for the clap). If we could find out if a user has
passed a value or the default value was taken, we could keep the default value in the CLI interface,
but clap isn't supporting this right now. So, we need to go back and make `state_pruning` an
optional with the default written into the docs.

It also adds a test to ensure that we don't break this behavior again.
@bkchr bkchr added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes B5-clientnoteworthy C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Dec 24, 2022
@bkchr bkchr requested review from lexnv and a team December 24, 2022 12:39
Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍 (code-wise; don't know enough to reason about business logic)

@bkchr bkchr merged commit 27e9a1c into master Dec 26, 2022
@bkchr bkchr deleted the bkchr-fix-state-pruning-cli branch December 26, 2022 16:37
rossbulat pushed a commit that referenced this pull request Dec 28, 2022
* Make CLI state pruning optional again

The state pruning setting is stored in the database when it is created. In later runs it is fine to
drop the `--state-pruning` CLI argument as the setting is stored in the database. The state db will
only return an error if the stored state pruning doesn't match the state pruning given via CLI.

Recently we improved the state pruning CLI handling and accidentally made the state pruning value
always present (as we set some default value for the clap). If we could find out if a user has
passed a value or the default value was taken, we could keep the default value in the CLI interface,
but clap isn't supporting this right now. So, we need to go back and make `state_pruning` an
optional with the default written into the docs.

It also adds a test to ensure that we don't break this behavior again.

* More docs
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-37/1736/1

ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
* Make CLI state pruning optional again

The state pruning setting is stored in the database when it is created. In later runs it is fine to
drop the `--state-pruning` CLI argument as the setting is stored in the database. The state db will
only return an error if the stored state pruning doesn't match the state pruning given via CLI.

Recently we improved the state pruning CLI handling and accidentally made the state pruning value
always present (as we set some default value for the clap). If we could find out if a user has
passed a value or the default value was taken, we could keep the default value in the CLI interface,
but clap isn't supporting this right now. So, we need to go back and make `state_pruning` an
optional with the default written into the docs.

It also adds a test to ensure that we don't break this behavior again.

* More docs
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* Make CLI state pruning optional again

The state pruning setting is stored in the database when it is created. In later runs it is fine to
drop the `--state-pruning` CLI argument as the setting is stored in the database. The state db will
only return an error if the stored state pruning doesn't match the state pruning given via CLI.

Recently we improved the state pruning CLI handling and accidentally made the state pruning value
always present (as we set some default value for the clap). If we could find out if a user has
passed a value or the default value was taken, we could keep the default value in the CLI interface,
but clap isn't supporting this right now. So, we need to go back and make `state_pruning` an
optional with the default written into the docs.

It also adds a test to ensure that we don't break this behavior again.

* More docs
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
Status: done
Development

Successfully merging this pull request may close these issues.

4 participants