feat(stages, storage): pruning configuration #3341
Conversation
Codecov Report
... and 18 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
shekhirin
force-pushed
the
alexey/stage-prune
branch
2 times, most recently
from
8690453
to
503f211
Compare
shekhirin
force-pushed
the
alexey/stage-prune
branch
from
503f211
to
1cbf0b0
Compare
shekhirin
force-pushed
the
alexey/stage-prune
branch
from
6cef5e5
to
ad302cd
Compare
shekhirin
changed the title
PrunableStage and Pipeline::prune
shekhirin
requested review from
rkrasiuk,
mattsse,
Rjected,
rakita,
joshieDo,
onbjerg and
gakonst
as code owners
crates/storage/db/src/tables/mod.rs
Outdated
| table!( | ||
| /// Stores the highest pruned block number. | ||
| ( PruneStage ) StageId | BlockNumber | ||
| ); |
There was a problem hiding this comment.
i dream of collapsing SyncStage, SyncStageProgress & PruneStage into a single table
rkrasiuk
added
C-enhancement
This is its final form, plan to implement pruning for stages and trigger pruning in separate PRs |
Co-authored-by: Roman Krasiuk <rokrassyuk@gmail.com>
|
what's the plan for pruning outside the pipeline during live sync? |
hooking it into the |
|
will the threshold be present in the pipeline as well? since we don't know how far the target is from the tip |
I think it shouldn't be in the pipeline, but rather in the engine. I expect the distance between target block number for pruning and the tip to always be low:
|
shekhirin
force-pushed
the
alexey/stage-prune
branch
4 times, most recently
from
4fb4ac1
to
6411995
Compare
shekhirin
force-pushed
the
alexey/stage-prune
branch
from
6411995
to
97a6782
Compare
| use reth_codecs::Compact; | ||
|
|
||
| #[test] | ||
| fn prune_checkpoint_roundtrip() { |
There was a problem hiding this comment.
This is unnecessary since deriving main_codec automatically adds derive_arbitrary which generates these kind of roundtrip tests for a default of 256 cases. It's mentioned on derive_arbitratry, but not on the main_codec docs, should probably add.
...
running 1 test
test prune::mode::PruneModeTests::proptest ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 283 filtered out; finished in 0.01s
...There was a problem hiding this comment.
you're right, I see:
#[cfg(test)]
mod PruneCheckpointTests {
use super::Compact;
#[test]
fn proptest() {
let mut config = proptest::prelude::ProptestConfig::with_cases(256i32 as u32);
{
let mut config = config.__sugar_to_owned();
::proptest::sugar::force_no_fork(&mut config);
{
config.source_file = Some(file!( ));
let mut runner = ::proptest::test_runner::TestRunner::new(config);
let names = stringify!( field );
match runner.run(
&::proptest::strategy::Strategy::prop_map(
::proptest::arbitrary::any::<super::PruneCheckpoint>(),
|values| ::proptest::sugar::NamedArguments(names, values)),
|::proptest::sugar::NamedArguments(
_, field)|
{
let _: () = {
{
let mut buf = (::alloc::vec::Vec::new());
let len = field.clone().to_compact(&mut buf);
let (decoded, _) = super::PruneCheckpoint::from_compact(&buf, len);
assert!(field == decoded);
}
};
Ok(())
})
{
Ok(_) => (),
Err(e) => panic!("{}\n{}", e, runner),
}
}
}
}
}removed
|
Just giving some context for the change of field order on 506643e This should have been caught during compilation time (resulting in an error), but a couple weeks ago, this got changed and I missed this PR. Will follow up with reverting that change, alongside some better docs for |
Resolves #3430
PruneModeThere are two modes for pruning, when it's enabled:
Full– prune all blocks, used for full node.Distance– prune blocks before thehead-Nblock number. In other words, keep last N blocks.Before– prune blocks before the specified block number. The specified block number is not pruned.PruneTargetFor convenience,
PruneModecan be converted into thePruneTargetwhich represents either:Allvariant – prune all blocks, i.e. not save any data. Used for the full node.Block(BlockNumber)variant – prune blocks up to the specified block number, inclusive.PruneConfigAs per the description in the linked issue: #3430 (comment).