feat(pruner): ring #9230
feat(pruner): ring #9230
Conversation
crates/prune/prune/src/pruner.rs
Outdated
| // Prune static file segments first | ||
| for segment in self.static_file_segments() { | ||
| let result = self.prune_segment( | ||
| provider, | ||
| tip_block_number, | ||
| limiter, | ||
| &*segment, | ||
| PrunePurpose::StaticFile, | ||
| )?; | ||
| match result { | ||
| ControlFlow::Continue(output) => { | ||
| progress = output.progress; | ||
|
|
||
| if output.pruned > 0 { | ||
| limiter.increment_deleted_entries_count_by(output.pruned); | ||
| pruned += output.pruned; | ||
| stats.insert(segment.segment(), (output.progress, output.pruned)); | ||
| } | ||
| } | ||
| ControlFlow::LimitReached => break, |
There was a problem hiding this comment.
static file segments will always be first, that doesn't tackle the problem in a safe way that's tolerant to all kinds of user defined prune configs. for that static files need to be flat with other segments in the ring abstraction.
crates/prune/prune/src/pruner.rs
Outdated
| match result? { | ||
| ControlFlow::Continue(output) => { | ||
| progress = output.progress; | ||
|
|
||
| if output.pruned > 0 { | ||
| limiter.increment_deleted_entries_count_by(output.pruned); | ||
| pruned += output.pruned; | ||
| stats.insert(segment_name, (output.progress, output.pruned)); | ||
| } | ||
| } | ||
| } else { | ||
| debug!(target: "pruner", segment = ?segment.segment(), ?purpose, "Nothing to prune for the segment"); | ||
| ControlFlow::LimitReached => break, |
There was a problem hiding this comment.
this will loop until time limit reached, even when there are no segments to prune, a waste of resources
There was a problem hiding this comment.
when the limit is reached, this
reth/crates/prune/prune/src/pruner.rs
Lines 280 to 282 in 2cd1505
break in the match clause you commented on
There was a problem hiding this comment.
ah wait, you described a different issue, why it will loop if there are no segments?
crates/prune/prune/src/pruner.rs
Outdated
| &*segment, | ||
| PrunePurpose::User, | ||
| ); | ||
| pruned_segments.push(segment); |
There was a problem hiding this comment.
I see the indirection here, this is how you avoid looping until timeout if all segments are empty.
a segment should only be added to pruned if it didn't quit because limit is reached. otherwise can occur that some segment is unfortunate to get the position which tends to time out, just when it started, since we don't prune random ranges on each prune job.
There was a problem hiding this comment.
a segment should only be added to pruned if it didn't quit because limit is reached
then we will prune the segment until completion, no? and it defeats the purpose of the ring
There was a problem hiding this comment.
it won't defeat its purpose, ring still ensures all segments are pruned till completion, not only same first few
another safe way would be to start with a random segment each prune job
|
blocked by #8746 |
No description provided.