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

cli: Improve pruning documentation #12819

Merged
paritytech-processbot merged 13 commits into from
Dec 8, 2022
Merged

cli: Improve pruning documentation #12819

Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
7ece645
cli: Improve pruning documentation
lexnv Dec 1, 2022
fdbc9a0
cli: Keep `finalized` notation and remove `canonical` one
lexnv Dec 1, 2022
804aff6
cli: Fix cargo doc
lexnv Dec 1, 2022
eb579bc
cli: `PruningModeClap` IR enum
lexnv Dec 5, 2022
7eb7956
cli: Convert PruningModeClap into pruning modes
lexnv Dec 5, 2022
8da7e10
cli: Use `PruningModeClap`
lexnv Dec 5, 2022
9cf6f5e
cli: Rename to `DatabasePruningMode`
lexnv Dec 5, 2022
9379757
cli: Implement `FromStr` instead of `clap::ValueEnum`
lexnv Dec 5, 2022
5356629
Update client/cli/src/params/pruning_params.rs
lexnv Dec 8, 2022
f25b491
Fix clippy
lexnv Dec 8, 2022
e3ffe3e
cli: Add option documentation back
lexnv Dec 8, 2022
a4e88bf
Merge remote-tracking branch 'origin/master' into lexnv/cli_db_docume…
lexnv Dec 8, 2022
aee8844
Apply suggestions from code review
bkchr Dec 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions client/cli/src/params/pruning_params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,30 @@ use sc_service::{BlocksPruning, PruningMode};
/// Parameters to define the pruning mode
#[derive(Debug, Clone, PartialEq, Args)]
pub struct PruningParams {
/// Specify the state pruning mode, a number of blocks to keep or 'archive'.
/// Specify the state pruning mode.
///
/// Default is to keep only the last 256 blocks,
/// otherwise, the state can be kept for all of the blocks (i.e 'archive'),
/// or for all of the canonical blocks (i.e 'archive-canonical').
/// This mode specifies when the block's state (ie, storage)
/// should be pruned (ie, removed) from the database.
///
/// Options available:
/// 'archive' Keep the state of all blocks.
/// 'archive-canonical' Keep only the state of finalized blocks.
/// number Keep the state of the last number of finalized blocks.
///
/// The default option is to keep the last 256 blocks (number == 256).
#[arg(alias = "pruning", long, value_name = "PRUNING_MODE")]
pub state_pruning: Option<String>,
Copy link
Member

@bkchr bkchr Dec 1, 2022

Choose a reason for hiding this comment

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

We should change this to:

Suggested change
/// Options available:
/// 'archive' Keep the state of all blocks.
/// 'archive-canonical' Keep only the state of finalized blocks.
/// number Keep the state of the last number of finalized blocks.
///
/// The default option is to keep the last 256 blocks (number == 256).
#[arg(alias = "pruning", long, value_name = "PRUNING_MODE")]
pub state_pruning: Option<String>,
#[arg(alias = "pruning", long, value_name = "PRUNING_MODE", default = "256")]
pub state_pruning: PruningMode,

Then add the following enum:

enum PruningMode {
     Archive,
     ArchiveCanonical,
     Custom(u32),
}

impl clap::ValueEnum for PruningMode {
     .. impl the enum
}

Copy link
Contributor Author

@lexnv lexnv Dec 5, 2022

Choose a reason for hiding this comment

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

Yep, that does make sense! Thanks for the suggestion!

One problem with ValueEnum is that its happy with unit only enum variants. This is related to the value_variants that requires us to state the expected values. If the Custom(32) is omitted, the default value of "256" cannot be provided anymore.

To overcome this I've changed the ValueEnum to FromStr.
Let me know what you think of it

/// Specify the blocks pruning mode, a number of blocks to keep or 'archive'.
/// Specify the blocks pruning mode.
///
/// This mode specifies when the block's body (including justifications)
/// should be pruned (ie, removed) from the database.
///
/// Default is to keep all finalized blocks.
/// otherwise, all blocks can be kept (i.e 'archive'),
/// or for all canonical blocks (i.e 'archive-canonical'),
/// or for the last N blocks (i.e a number).
/// Options available:
/// 'archive' Keep all blocks.
/// 'archive-canonical' Keep only finalized blocks.
/// number Keep the last number of finalized blocks.
///
/// NOTE: only finalized blocks are subject for removal!
/// The default option is 'archive-canonical'.
#[arg(alias = "keep-blocks", long, value_name = "COUNT")]
pub blocks_pruning: Option<String>,
}
Expand Down