feat(store): Partial back port of pruning and compaction from main #23
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…#1439) * ADR 101: Implement pruning mechanism (cometbft#1150) * Added pruning mechanism for blocks and abci block results * Added support for datacompanion and application retain heights --------- Signed-off-by: Thane Thomson <[email protected]> Co-authored-by: Thane Thomson <[email protected]> * Removed extra code added via cherry pick conflicts and regenerated mock * Fixed changelog placement and added entry on pruning mechanism breaking * ADR-101: Metrics to monitor the pruning (cometbft#1234) * Metrics to report on the data companion retain height * Added metric to report the application retain height as well * Added metrics to report the blockstore base height and the abci results base height --------- Co-authored-by: Thane Thomson <[email protected]> * ADR-101: Pruning mechanism minor fixes (cometbft#1246) * Applied @serigo-mena's comments * Update state/pruner.go Co-authored-by: Thane Thomson <[email protected]> * Applied further comments from PR * Fixed batching interval --------- Co-authored-by: Thane Thomson <[email protected]> * Delete bad changelog entry --------- Signed-off-by: Thane Thomson <[email protected]> Co-authored-by: Thane Thomson <[email protected]> Co-authored-by: Daniel Cason <[email protected]>
* state: Invert logic of whether retain heights are set to be more readable Signed-off-by: Thane Thomson <[email protected]> * state: Refactor findMinRetainHeight logic for clarity and to respect config Signed-off-by: Thane Thomson <[email protected]> * state: Rename findMinRetainHeight to findMinBlockRetainHeight for clarity Signed-off-by: Thane Thomson <[email protected]> * state: Rename Pruner.SetApplicationRetainHeight to SetApplicationBlockRetainHeight for clarity Signed-off-by: Thane Thomson <[email protected]> * state: Rename pruner SetCompanionRetainHeight to SetCompanionBlockRetainHeight for clarity Signed-off-by: Thane Thomson <[email protected]> * state: Refactor - shorten local variable name Signed-off-by: Thane Thomson <[email protected]> * state: Refactor - rename local method name for clarity Signed-off-by: Thane Thomson <[email protected]> * state: Refactor - remove redundant condition Signed-off-by: Thane Thomson <[email protected]> * state: Refactor - use helper instead of redefining logic Signed-off-by: Thane Thomson <[email protected]> * state: Refactor - shorten local variable names Signed-off-by: Thane Thomson <[email protected]> * state: Refactor - shorten local variable names Signed-off-by: Thane Thomson <[email protected]> * state: Refactor Pruner.SetApplicationBlockRetainHeight logic The application retain height should be set regardless of what the companion retain height is. Pruning should take place based on whichever of the two values is lower. There is no need to consider the companion retain height in this logic. Signed-off-by: Thane Thomson <[email protected]> * state: Refactor Pruner.SetCompanionBlockRetainHeight logic Similar refactor to that done for Pruner.SetApplicationBlockRetainHeight. There is no need to consider the application retain height during this function call. Signed-off-by: Thane Thomson <[email protected]> * state: Refactor Pruner.SetABCIResRetainHeight logic Aligns the logic of this method similar to that in the other retain height setter methods. Also fixes a logic bug where setting the retain height would skip updating the metrics. Signed-off-by: Thane Thomson <[email protected]> * state: Refactor - shorten local variable names Signed-off-by: Thane Thomson <[email protected]> * state: Simplify pruner logic assuming retain heights are always set We can simplify the pruner logic if we assume that the initial retain heights are always set on node startup. Signed-off-by: Thane Thomson <[email protected]> * node: Tell pruner whether companion is enabled Signed-off-by: Thane Thomson <[email protected]> * state: Expand error message detail Signed-off-by: Thane Thomson <[email protected]> * state+store: Align tests with new logic Signed-off-by: Thane Thomson <[email protected]> * state: Refactor - shorten local variable names Signed-off-by: Thane Thomson <[email protected]> * rpc/grpc: Return trace IDs in error responses from pruning service Signed-off-by: Thane Thomson <[email protected]> * node: Refactor/expand pruning service initialization logic Signed-off-by: Thane Thomson <[email protected]> * test/e2e: Minor cosmetic tweaks to gRPC tests Signed-off-by: Thane Thomson <[email protected]> * test/e2e: Enable data companion pruning Signed-off-by: Thane Thomson <[email protected]> * node: Refactor - extract pruner creation as function Signed-off-by: Thane Thomson <[email protected]> * test/e2e: Only enable companion-based pruning on full nodes and validators Signed-off-by: Thane Thomson <[email protected]> * state: Only start ABCI results pruning routine when data companion is enabled Signed-off-by: Thane Thomson <[email protected]> * state: Fix TestMinRetainHeight logic Signed-off-by: Thane Thomson <[email protected]> * node: Fix companion retain height initialization logic Signed-off-by: Thane Thomson <[email protected]> * test/e2e: Expand testing framework Allows explicit control over whether or not pruning should take a data companion into account for full nodes or validators in our E2E framework. This allows greater flexibility in the E2E framework to test nodes both with and without data companion-related functionality. Expands our standard CI manifest to expect data companions attached to two of the nodes. Signed-off-by: Thane Thomson <[email protected]> * state: Rename background routines Signed-off-by: Thane Thomson <[email protected]> * node: Apply change from code review Signed-off-by: Thane Thomson <[email protected]> --------- Signed-off-by: Thane Thomson <[email protected]>
sergio-mena
reviewed
Aug 5, 2025
sergio-mena
reviewed
Aug 5, 2025
sergio-mena
reviewed
Aug 5, 2025
sergio-mena
reviewed
Aug 5, 2025
There was a problem hiding this comment.
Overall LGTM. All very minor comments.
Only thing worth mentioning here is the question about keeping the companion-related logic here. But if the reason is to not heavily modify the code, to make Polygon folks' life easier when upgrading to CometBFT v1.x, then I'm OK with it.
If that's the case, I would hard-code disable the companion somewhere in the config code (e.g. in ValidateBasic) with a comment, something like "this feature will only be usable in v1.x".
Co-authored-by: Sergio Mena <[email protected]>
…tbft into jasmina/pruning0.38
…_0.38 feat: Pruning and compaction port to 0.38
jmalicevic
changed the title
|
|
This pull request has been automatically marked as stale because it has not had |
marcello33
approved these changes
Aug 21, 2025
Raneet10
approved these changes
Aug 21, 2025
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains cometbft#1422 and cometbft@6fc2c37 and cometbft#1491
The port of commit 6fc2c37 does not contain the gRPC related changes present in the original back port to
v0.38.x-experimentalas they are not essential for the pruning logic.The rest of the code was ported as is to make a future transition and alignment with cometbft main easier.
The functionality added by this PR:
RetainHeightreturned from the application periodically. The frequency is set in the config.