Conversation
|
The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).
|
|
Claude finished @julienrbrt's task in 2m 38s —— View job Comprehensive Code Review CompleteI've completed a thorough code review of the block pruning implementation. Here's my detailed analysis: SummaryThis PR successfully implements height-based pruning for ev-node's block store and execution metadata. The overall design is solid and follows Go best practices. However, there are several areas that need attention before merging, particularly around error handling, batch size safety, and test coverage. Critical Issues1. Batch Size Safety Concern (
|
3e1e8e0 to
ed66fe6
Compare
26628f5 to
4d65b1e
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2984 +/- ##
==========================================
- Coverage 61.89% 61.25% -0.64%
==========================================
Files 111 112 +1
Lines 11113 11355 +242
==========================================
+ Hits 6878 6956 +78
- Misses 3496 3630 +134
- Partials 739 769 +30
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
7aef549 to
4d31269
Compare
3e07144 to
26d0327
Compare
94f37d2 to
ddc064a
Compare
* Initial plan * Add recovery history retention pruning Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * feat: add configurable recovery history retention Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Refactor recovery pruning into pruner component Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Address pruner review feedback Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Update evm test module dependency Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Address code review feedback Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Refine pruner checks and docs Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Clarify PruneExecMeta comment Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Rename recovery history setting Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Adjust pruner interval and defaults Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * fixes * updates * updates * updates * comment --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> Co-authored-by: Julien Robert <julien@rbrt.fr>
|
|
||
| When pruning is enabled, the pruner runs at the configured interval and removes data beyond the retention window (`pruning_keep_recent`). The system uses intelligent batching to avoid overwhelming the node: | ||
|
|
||
| - **Batch sizes are automatically calculated** based on your `pruning_interval` and `block_time` |
There was a problem hiding this comment.
in testing was there any degradation to the performance of the node when pruning?
There was a problem hiding this comment.
i restored a snapshot and tried against Eden. there was only at the beginning.
I am still testing. I can post pprof before/after and system usage. it won't be a go benchmark.
tac0turtle
left a comment
There was a problem hiding this comment.
it looks good, it would still be good to see benchmarks or have some numbers on how/if this slows down operation of node
tac0turtle
left a comment
There was a problem hiding this comment.
looks good now, thank you
|
Overview
Closes: #3063
Closes: #2093
/h/{height},/d/{height},/c/{height},/i/{hash}up to a target height.height → DA heightmapping) for pruned heights./s/{height}(state),/t(current height), and global metadata keys untouched.last-pruned-block-heightmetadata key so pruning is monotonic and idempotent.store.PruneBlocks.DAIncludedHeightKey), so only DA-included blocks are ever pruned, andPruningKeepRecent, pruning is skipped.PruneExecMetato delete per-heightExecMetaentries up to the same target height.ExecMetaPrunerinterface, and the submitter calls it from the DA inclusion pruning hook.StoreAdapter’sTail()now uses thelast-pruned-block-heightmetadata to skip fully pruned ranges instead of linearly scanning from genesis.Config
New node config fields / flags:
--evnode.node.pruning_enabled--evnode.node.pruning_keep_recent--evnode.node.pruning_intervalPruning actually runs only when all three are set to non-trivial values (enabled,
keep_recent > 0,interval > 0).Design trade-offs
Best-effort pruning:
If pruning fails (either store or exec metadata), we log the error but do not halt DA inclusion. This avoids stopping the node but means disk usage can grow silently if pruning keeps failing; operators will need to watch logs/metrics.
Scope:
This PR prunes:
ExecMetaPrunerhook when available.