Conversation
Summary of ChangesHello @pthmas, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new feature to implement height-based pruning for Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a pruning mechanism for ev-abci execution artifacts, which is a valuable addition for managing storage. The implementation is straightforward, adding a PruneExecMeta method to the adapter and a corresponding Prune method to the store.
My review focuses on two main points. First, in pkg/store/store.go, the Prune function can be made more efficient and atomic by using batch database operations instead of individual deletes in a loop. This is important for performance and data consistency. Second, the new test in pkg/store/store_test.go uses a fragile method for generating keys from heights; it should be updated to use strconv for consistency with the production code and to ensure correctness for all height values.
pthmas
force-pushed
the
pierrick/prunning
branch
from
b3cd390 to
9e7777d
Compare
…ndency (#345) * Initial plan * Implement pruning config mapping and remove sonic dependency - Rename clearConflictingViperKeys to mapCosmosPruningToEvNode - Map cosmos-sdk pruning keys to ev-node format with proper value conversion - Add comprehensive tests for pruning mapping function - Remove sonic installation from all Dockerfiles Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Fix comment clarity in mapCosmosPruningToEvNode Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Simplify logic and add test for override behavior - Simplify switch statement logic in mapCosmosPruningToEvNode - Add test case for verifying cosmos settings override existing evnode settings Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Remove redundant empty string case in switch statement Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Address PR review feedback on pruning mapping - Change unknown pruning values to default to "disabled" instead of "all" - Convert cosmos-sdk pruning-interval from blocks to time duration - Add evnode.node.block_time to tests and use DefaultConfig for realistic testing - Update all test expectations to match the block-to-duration conversion Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Address PR review: use SDK flag constants and map unknown to metadata - Change default case to map unknown pruning values to "metadata" instead of "disabled" - Use cosmos-sdk server flag constants (FlagPruning, FlagPruningKeepRecent, FlagPruningInterval) instead of hardcoded strings - Update imports across server package to use qualified sdkserver import to avoid name collision - Update all tests to use the SDK flag constants - Update test expectations for unknown value mapping to metadata Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com>
Overview
PruneExecMetaAPI to delete per-height execution metadata (e.g. block responses, block IDs) up to a target height.ExecMetaPrunerinterface in the ev-abci adapter so it can be driven by ev-node’s pruning hook.Design notes
ExecMetaPrunerhook see no behavior change.