Skip to content

Conversation

jmalicevic
Copy link
Contributor

@jmalicevic jmalicevic commented Oct 6, 2023

Closes #1422

Backports Commits: commits: 04dfaf5, c710bb8 (metrics for pruning), fe644d8


PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments

jmalicevic and others added 2 commits October 5, 2023 12:50
* Added pruning mechanism for blocks and abci block results
* Added support for datacompanion and application retain heights
---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Thane Thomson <connect@thanethomson.com>
@jmalicevic jmalicevic added the P:storage-optimization Priority: Give operators greater control over storage and storage optimization label Oct 6, 2023
@jmalicevic jmalicevic self-assigned this Oct 6, 2023
@jmalicevic jmalicevic requested a review from a team as a code owner October 6, 2023 08:38
@jmalicevic jmalicevic marked this pull request as draft October 6, 2023 09:38
jmalicevic and others added 3 commits October 6, 2023 11:42
* 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 <connect@thanethomson.com>
* Applied @serigo-mena's comments

* Update state/pruner.go

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Applied further comments from PR

* Fixed batching interval

---------

Co-authored-by: Thane Thomson <connect@thanethomson.com>
@jmalicevic jmalicevic marked this pull request as ready for review October 6, 2023 10:26
@cason cason linked an issue Oct 10, 2023 that may be closed by this pull request
3 tasks
Copy link

@cason cason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Legit.

@thanethomson thanethomson changed the title ADR-101: Backport pruning mecanism to v0.38.x ADR-101: Backport pruning mechanism to v0.38.x-experimental Oct 10, 2023
@jmalicevic jmalicevic merged commit de8f307 into v0.38.x-experimental Oct 11, 2023
@jmalicevic jmalicevic deleted the jasmina/backport-pruning-mechanism-038 branch October 11, 2023 09:23
lasarojc pushed a commit that referenced this pull request Nov 13, 2023
* ADR 101: Implement pruning mechanism (#1150)

* Added pruning mechanism for blocks and abci block results
* Added support for datacompanion and application retain heights
---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Thane Thomson <connect@thanethomson.com>

* 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  (#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 <connect@thanethomson.com>

* ADR-101: Pruning mechanism minor fixes (#1246)

* Applied @serigo-mena's comments

* Update state/pruner.go

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Applied further comments from PR

* Fixed batching interval

---------

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Delete bad changelog entry

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Daniel Cason <daniel.cason@informal.systems>
jmalicevic added a commit to informalsystems/cometbft that referenced this pull request Jul 30, 2025
…#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 <connect@thanethomson.com>
Co-authored-by: Thane Thomson <connect@thanethomson.com>

* 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 <connect@thanethomson.com>

* ADR-101: Pruning mechanism minor fixes (cometbft#1246)

* Applied @serigo-mena's comments

* Update state/pruner.go

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Applied further comments from PR

* Fixed batching interval

---------

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Delete bad changelog entry

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Daniel Cason <daniel.cason@informal.systems>
marcello33 pushed a commit to 0xPolygon/cometbft that referenced this pull request Aug 21, 2025
* ADR-101: Backport pruning mechanism to v0.38.x-experimental (cometbft#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 <connect@thanethomson.com>
Co-authored-by: Thane Thomson <connect@thanethomson.com>

* 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 <connect@thanethomson.com>

* ADR-101: Pruning mechanism minor fixes (cometbft#1246)

* Applied @serigo-mena's comments

* Update state/pruner.go

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Applied further comments from PR

* Fixed batching interval

---------

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Delete bad changelog entry

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Daniel Cason <daniel.cason@informal.systems>

* Removed redundant code in tests

* Call observer only when retain height changes (cometbft#1490) (cometbft#1491)

* ADR 101: Refactor height check-related logic and tests (cometbft#1271)

* state: Invert logic of whether retain heights are set to be more readable

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor findMinRetainHeight logic for clarity and to respect config

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Rename findMinRetainHeight to findMinBlockRetainHeight for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Rename Pruner.SetApplicationRetainHeight to SetApplicationBlockRetainHeight for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Rename pruner SetCompanionRetainHeight to SetCompanionBlockRetainHeight for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - shorten local variable name

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - rename local method name for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - remove redundant condition

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - use helper instead of redefining logic

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - shorten local variable names

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - shorten local variable names

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* 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 <connect@thanethomson.com>

* 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 <connect@thanethomson.com>

* 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 <connect@thanethomson.com>

* state: Refactor - shorten local variable names

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* 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 <connect@thanethomson.com>

* node: Tell pruner whether companion is enabled

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Expand error message detail

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state+store: Align tests with new logic

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - shorten local variable names

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* rpc/grpc: Return trace IDs in error responses from pruning service

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Refactor/expand pruning service initialization logic

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* test/e2e: Minor cosmetic tweaks to gRPC tests

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* test/e2e: Enable data companion pruning

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Refactor - extract pruner creation as function

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* test/e2e: Only enable companion-based pruning on full nodes and validators

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Only start ABCI results pruning routine when data companion is enabled

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Fix TestMinRetainHeight logic

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Fix companion retain height initialization logic

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* 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 <connect@thanethomson.com>

* state: Rename background routines

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Apply change from code review

Signed-off-by: Thane Thomson <connect@thanethomson.com>

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add compaction to pruner

* Reduced duplicate code in state_test.go

* Update test/e2e/generator/generate.go

Co-authored-by: Sergio Mena <sergio@informal.systems>

* Refactored mores tests

* Disabled data companion activation

* Fixed the pruned states and results counter

* Refactoring based on review

* Removed unused argument from Prune function;

* enable abci result pruning by default

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Daniel Cason <daniel.cason@informal.systems>
Co-authored-by: Sergio Mena <sergio@informal.systems>
marcello33 pushed a commit to 0xPolygon/cometbft that referenced this pull request Aug 25, 2025
* ADR-101: Backport pruning mechanism to v0.38.x-experimental (cometbft#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 <connect@thanethomson.com>
Co-authored-by: Thane Thomson <connect@thanethomson.com>

* 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 <connect@thanethomson.com>

* ADR-101: Pruning mechanism minor fixes (cometbft#1246)

* Applied @serigo-mena's comments

* Update state/pruner.go

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Applied further comments from PR

* Fixed batching interval

---------

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Delete bad changelog entry

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Daniel Cason <daniel.cason@informal.systems>

* Removed redundant code in tests

* Call observer only when retain height changes (cometbft#1490) (cometbft#1491)

* ADR 101: Refactor height check-related logic and tests (cometbft#1271)

* state: Invert logic of whether retain heights are set to be more readable

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor findMinRetainHeight logic for clarity and to respect config

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Rename findMinRetainHeight to findMinBlockRetainHeight for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Rename Pruner.SetApplicationRetainHeight to SetApplicationBlockRetainHeight for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Rename pruner SetCompanionRetainHeight to SetCompanionBlockRetainHeight for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - shorten local variable name

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - rename local method name for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - remove redundant condition

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - use helper instead of redefining logic

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - shorten local variable names

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - shorten local variable names

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* 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 <connect@thanethomson.com>

* 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 <connect@thanethomson.com>

* 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 <connect@thanethomson.com>

* state: Refactor - shorten local variable names

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* 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 <connect@thanethomson.com>

* node: Tell pruner whether companion is enabled

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Expand error message detail

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state+store: Align tests with new logic

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - shorten local variable names

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* rpc/grpc: Return trace IDs in error responses from pruning service

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Refactor/expand pruning service initialization logic

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* test/e2e: Minor cosmetic tweaks to gRPC tests

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* test/e2e: Enable data companion pruning

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Refactor - extract pruner creation as function

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* test/e2e: Only enable companion-based pruning on full nodes and validators

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Only start ABCI results pruning routine when data companion is enabled

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Fix TestMinRetainHeight logic

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Fix companion retain height initialization logic

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* 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 <connect@thanethomson.com>

* state: Rename background routines

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Apply change from code review

Signed-off-by: Thane Thomson <connect@thanethomson.com>

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* ADR - 101: Backport indexer pruning mechanism to v0.38.x-experimental (cometbft#1476)

* indexer: Implement pruning mechanism (cometbft#1176)

* done stateDB writes batching

* remove forgotten debug print

* benchmarking for state db save

* state db save benchmarking

* batching in store

* benchmarking for store batching

* Added pruning mechanism  -initial version

* removed comments

* Rewrore pruning mechanism as service. Working code but tests need to change.

* Minor edits

* Removed comments

* Fixed linter

* Fixed linter

* Fixed state and block store tests
Proper ticker used instead of sleep for pruning service job

* Fixed failing http test

* fixed linter errors and reduced sleep in http test

* Fixed e2e test and failing linter

* Applied some of @thanethomson's PR comments

* add changelog

* Separated retain height functions

* fixed linter and test

* Fixed code in select min height`

* Fixed bugs causing failing tests

* Added config flag for companion initial height
Fixed failing test due to wrong check on blockstore base height

* Tested setting the companion initial height via config.
Moved pruner initialization into blockExecutor again.

* Added config for pruning frequency

* minor

* Added additional test for companion retain height setting

* script for adjasting test loadtime parameeters

* Added simple testing tool for goleveldb

* measuring tool + pruning setting

* draft of tx_index pruning

* using bus to prune tx_index

* draft of pruning

* working prototipe on tx_index txIndexer pruning

* consensus: Fix test broken due to missing state store mocks

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* config: Refactor to match updated ADR

ADR-101 was updated recently to reflect the desired configuration file
changes - this updates the code to match the ADR.

It also changes the "frequency" variable to more accurately be called an
"interval", since it represents a sleep period between pruning
operations as opposed to the number of times it should be run per
second. The interval is also defined as a time.Duration now to
facilitate more ergonomic usage from the configuration file.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Format

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Rename pruner sleep time to "interval"

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor pruner config

Make the pruner config private such that its configuration is fixed at
runtime. With the previous approach, the PrunerInterval method could be
abused to change the interval at runtime since it returns a function
that can operate on the public type.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Fix minor nits in function docstrings

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Remove unused method

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Remove unnecessary OnStart/OnStop logic

The base service OnStart/OnStop logic is purposefully empty, so there's
no need to explicitly call it. And by not defining the OnStop method on
Pruner the base service's OnStop (a noop) will automatically be called
because of Go's "inheritance" mechanism.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Make pruner belong to Node primarily

Move the pruner "ownership" the Node. This doesn't actually have a
meaningful effect on the lifetime of the pruner, but it is more of a
signal to readers of the code that it should be considered a service in
its own right, and the block executor simply has access to it to trigger
one method call.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* store: Format comment for readability

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Add docstring for PruneABCIResponses

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor pruningRoutine to condense logging

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor pruningRoutine - extract function for block pruning

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor pruningRoutine - extract function for ABCI result pruning

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Only save companion retain height on startup if not yet present

Expand the check on node creation to only set the companion retain
height if 3 conditions are met:

1. The companion retain height has not yet been set previously.
2. The companion is enabled.
3. The configured companion retain height is > 0.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* config: Impose restriction on pruning interval to be > 0

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* test for txindex pruning

* node: Fail construction if setting data companion retain height fails

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Start pruner on node startup after state sync

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* light: Remove sleep from test

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Handle error cases where retain height keys are not set

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Make Pruner.FindMinRetainHeight private

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Improve logging in block executor pruning

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* block indexer pruning + test

* Format

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Export FindMinRetainHeight exclusively for testing

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Extract function for initializing companion retain height

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entries

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* docs: Update configuration-related content

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* finilizing

* undoing testing setup

* Revert "done stateDB writes batching"

This reverts commit 78d8d38.

* Revert "benchmarking for state db save"

This reverts commit f3b757a.

* Revert "batching in store"

This reverts commit 7ca253f.

* Revert "benchmarking for store batching"

This reverts commit 2fa2e38.

* Revert "add changelog"

This reverts commit ec60b4a.

* Revert "fixed linter and test"

This reverts commit fac927e.

* Revert "remove forgotten comments"

This reverts commit fae7d59.

* fixed go.sum

* shut up linter

* add changelog

* node: Stop pruner when the node stops

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Remove redundunt variable

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* remove redundunt search for block heights

* rebased to PR 1150

* state: Refactor ABCI response pruning

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Add parameter names to Store interface for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Rename helper for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Optimize ABCI responses pruning

Avoid iterating through all possible heights when pruning ABCI
responses.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fixed couting of the last height pruned for abci respnses and blocks

* Set application retain height on startup to avoid pruning by the data companion
before the application has indicated so

* Apply suggestions from code review

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Set app retain height on startup only if it was not set before

* Fixed linter

* Simplified abci res pruning

* fixed tests to support indexerService in pruner

* forgotten file

* updated test

* ADR 101: Recommended updates to pruner service (cometbft#1201)

* state: Reference retain heights instead of pruned heights for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Explicitly assign variable values for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - early returns

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Add mutex to Pruner to serialize retain height setting requests

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Add convenience getters to Pruner

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Export Pruner errors

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Rename PrunerInterval option to WithPrunerInterval for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Fix error message references

Signed-off-by: Thane Thomson <connect@thanethomson.com>

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* ADR 101: Add observer for pruner (cometbft#1183)

* state: Add observer for Pruner

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Wire in pruner observer

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Make test more deterministic

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix renames

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Simplify NoopPrunerObserver usage

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Fix order of expected/actual params in assertion

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Clarify field meanings

Signed-off-by: Thane Thomson <connect@thanethomson.com>

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* persisting last retained height for the block indexer

* separate index pruning

* adjust go.sum

* Separated ABCI result pruning into separate routine

Added observer into the pruner tests.

* optimizing event search

* linter fix

* pruning benchmark

* fix linter

* persisting indexer retain height

* indexer last retain height

* fix linter

* Update state/pruner.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* remove reduntant dependencies

* added separate persisting of block and tx indexer retain heights

* ADR-101: implement gRPC `PruningService` (cometbft#1154)

* proto: Add proto files for the PruningService

* proto: generate *.pb.go files for PruningService

* config: add grpc.privileged incl. pruning_service

As described in ADR-101, add the node configuration section named
grpc.privileged to control the privileged server socket,
containing a pruning_service section for the pruning service.

* config: add pruning service config to the template

* grpc: package for privileged server

The privileged server optionally instantiated with the pruning service.

* node: add setup for the privileged gRPC server

* rpc: enable PruningService in test helper config

Also stop the makeAddrs function from growing grotesquely repetitive
and replace it with makeAddr returning a single, supposedly random,
local address string.

* grpc: privileged client with PruningService

Add client-side support for the privileged connection that features
an optionally enabled pruning service.

* e2e tests for PruningService

* config: refer to [storage.pruning] section in config.toml

Replace potentially confusing text in the comments on the pruning service configuration.

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* grpc: tracing with error logs for PruningService

---------

Co-authored-by: Andy Nogueira <me@andynogueira.dev>
Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Update state/txindex/indexer_service.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* Update state/indexer/block/kv/kv.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* 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 <connect@thanethomson.com>

* using built in bytes function

* fix missing import

* reducing self-written functions

* exporting getKeys for testing

* removing indexer service as middle layer between indexers and pruner

* separate tx and block indexers

* Update state/pruner.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* Update state/pruner.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* fix naming

* revert go.mod go.sum changes

* Update state/pruner.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* fix ill merge resolution

* batch pruning

* block indexer: non-nil keyArray

* block indexer: remove event keys record optimization

* block indexer: remove getEventKeys optimization

* periodically flushing pruning batch

* revert package renaming, delete unused functions

* minor changes

* .changelog: add missing method to pruner update

* txindexer: change panic to returning an error

* remove exposal of GetKeys outside testing

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>
Co-authored-by: Adi Seredinschi <a@seredinschi.net>
Co-authored-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Mikhail Zabaluev <mikhail@informal.systems>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>

* Fixed bad merge

* Fixed changelog structure

* Fixed changelog order

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: werty144 <anton-paramonov2000@yandex.ru>
Co-authored-by: Adi Seredinschi <a@seredinschi.net>
Co-authored-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Mikhail Zabaluev <mikhail@informal.systems>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>

* ADR-101 v0.38.x - Backport metrics for monitoring indexer pruning (cometbft#1479)

* indexer: Implement pruning mechanism (cometbft#1176)

* done stateDB writes batching

* remove forgotten debug print

* benchmarking for state db save

* state db save benchmarking

* batching in store

* benchmarking for store batching

* Added pruning mechanism  -initial version

* removed comments

* Rewrore pruning mechanism as service. Working code but tests need to change.

* Minor edits

* Removed comments

* Fixed linter

* Fixed linter

* Fixed state and block store tests
Proper ticker used instead of sleep for pruning service job

* Fixed failing http test

* fixed linter errors and reduced sleep in http test

* Fixed e2e test and failing linter

* Applied some of @thanethomson's PR comments

* add changelog

* Separated retain height functions

* fixed linter and test

* Fixed code in select min height`

* Fixed bugs causing failing tests

* Added config flag for companion initial height
Fixed failing test due to wrong check on blockstore base height

* Tested setting the companion initial height via config.
Moved pruner initialization into blockExecutor again.

* Added config for pruning frequency

* minor

* Added additional test for companion retain height setting

* script for adjasting test loadtime parameeters

* Added simple testing tool for goleveldb

* measuring tool + pruning setting

* draft of tx_index pruning

* using bus to prune tx_index

* draft of pruning

* working prototipe on tx_index txIndexer pruning

* consensus: Fix test broken due to missing state store mocks

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* config: Refactor to match updated ADR

ADR-101 was updated recently to reflect the desired configuration file
changes - this updates the code to match the ADR.

It also changes the "frequency" variable to more accurately be called an
"interval", since it represents a sleep period between pruning
operations as opposed to the number of times it should be run per
second. The interval is also defined as a time.Duration now to
facilitate more ergonomic usage from the configuration file.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Format

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Rename pruner sleep time to "interval"

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor pruner config

Make the pruner config private such that its configuration is fixed at
runtime. With the previous approach, the PrunerInterval method could be
abused to change the interval at runtime since it returns a function
that can operate on the public type.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Fix minor nits in function docstrings

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Remove unused method

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Remove unnecessary OnStart/OnStop logic

The base service OnStart/OnStop logic is purposefully empty, so there's
no need to explicitly call it. And by not defining the OnStop method on
Pruner the base service's OnStop (a noop) will automatically be called
because of Go's "inheritance" mechanism.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Make pruner belong to Node primarily

Move the pruner "ownership" the Node. This doesn't actually have a
meaningful effect on the lifetime of the pruner, but it is more of a
signal to readers of the code that it should be considered a service in
its own right, and the block executor simply has access to it to trigger
one method call.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* store: Format comment for readability

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Add docstring for PruneABCIResponses

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor pruningRoutine to condense logging

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor pruningRoutine - extract function for block pruning

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor pruningRoutine - extract function for ABCI result pruning

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Only save companion retain height on startup if not yet present

Expand the check on node creation to only set the companion retain
height if 3 conditions are met:

1. The companion retain height has not yet been set previously.
2. The companion is enabled.
3. The configured companion retain height is > 0.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* config: Impose restriction on pruning interval to be > 0

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* test for txindex pruning

* node: Fail construction if setting data companion retain height fails

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Start pruner on node startup after state sync

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* light: Remove sleep from test

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Handle error cases where retain height keys are not set

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Make Pruner.FindMinRetainHeight private

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Improve logging in block executor pruning

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* block indexer pruning + test

* Format

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Export FindMinRetainHeight exclusively for testing

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Extract function for initializing companion retain height

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entries

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* docs: Update configuration-related content

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* finilizing

* undoing testing setup

* Revert "done stateDB writes batching"

This reverts commit 78d8d38.

* Revert "benchmarking for state db save"

This reverts commit f3b757a.

* Revert "batching in store"

This reverts commit 7ca253f.

* Revert "benchmarking for store batching"

This reverts commit 2fa2e38.

* Revert "add changelog"

This reverts commit ec60b4a.

* Revert "fixed linter and test"

This reverts commit fac927e.

* Revert "remove forgotten comments"

This reverts commit fae7d59.

* fixed go.sum

* shut up linter

* add changelog

* node: Stop pruner when the node stops

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Remove redundunt variable

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* remove redundunt search for block heights

* rebased to PR 1150

* state: Refactor ABCI response pruning

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Add parameter names to Store interface for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Rename helper for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Optimize ABCI responses pruning

Avoid iterating through all possible heights when pruning ABCI
responses.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fixed couting of the last height pruned for abci respnses and blocks

* Set application retain height on startup to avoid pruning by the data companion
before the application has indicated so

* Apply suggestions from code review

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Set app retain height on startup only if it was not set before

* Fixed linter

* Simplified abci res pruning

* fixed tests to support indexerService in pruner

* forgotten file

* updated test

* ADR 101: Recommended updates to pruner service (cometbft#1201)

* state: Reference retain heights instead of pruned heights for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Explicitly assign variable values for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Refactor - early returns

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Add mutex to Pruner to serialize retain height setting requests

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Add convenience getters to Pruner

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Export Pruner errors

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Rename PrunerInterval option to WithPrunerInterval for clarity

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Fix error message references

Signed-off-by: Thane Thomson <connect@thanethomson.com>

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* ADR 101: Add observer for pruner (cometbft#1183)

* state: Add observer for Pruner

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Wire in pruner observer

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Make test more deterministic

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix renames

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Simplify NoopPrunerObserver usage

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Fix order of expected/actual params in assertion

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* state: Clarify field meanings

Signed-off-by: Thane Thomson <connect@thanethomson.com>

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* persisting last retained height for the block indexer

* separate index pruning

* adjust go.sum

* Separated ABCI result pruning into separate routine

Added observer into the pruner tests.

* optimizing event search

* linter fix

* pruning benchmark

* fix linter

* persisting indexer retain height

* indexer last retain height

* fix linter

* Update state/pruner.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* remove reduntant dependencies

* added separate persisting of block and tx indexer retain heights

* ADR-101: implement gRPC `PruningService` (cometbft#1154)

* proto: Add proto files for the PruningService

* proto: generate *.pb.go files for PruningService

* config: add grpc.privileged incl. pruning_service

As described in ADR-101, add the node configuration section named
grpc.privileged to control the privileged server socket,
containing a pruning_service section for the pruning service.

* config: add pruning service config to the template

* grpc: package for privileged server

The privileged server optionally instantiated with the pruning service.

* node: add setup for the privileged gRPC server

* rpc: enable PruningService in test helper config

Also stop the makeAddrs function from growing grotesquely repetitive
and replace it with makeAddr returning a single, supposedly random,
local address string.

* grpc: privileged client with PruningService

Add client-side support for the privileged connection that features
an optionally enabled pruning service.

* e2e tests for PruningService

* config: refer to [storage.pruning] section in config.toml

Replace potentially confusing text in the comments on the pruning service configuration.

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* grpc: tracing with error logs for PruningService

---------

Co-authored-by: Andy Nogueira <me@andynogueira.dev>
Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Update state/txindex/indexer_service.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* Update state/indexer/block/kv/kv.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* 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 <connect@thanethomson.com>

* using built in bytes function

* fix missing import

* reducing self-written functions

* exporting getKeys for testing

* removing indexer service as middle layer between indexers and pruner

* separate tx and block indexers

* Update state/pruner.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* Update state/pruner.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* fix naming

* revert go.mod go.sum changes

* Update state/pruner.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* fix ill merge resolution

* batch pruning

* block indexer: non-nil keyArray

* block indexer: remove event keys record optimization

* block indexer: remove getEventKeys optimization

* periodically flushing pruning batch

* revert package renaming, delete unused functions

* minor changes

* .changelog: add missing method to pruner update

* txindexer: change panic to returning an error

* remove exposal of GetKeys outside testing

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>
Co-authored-by: Adi Seredinschi <a@seredinschi.net>
Co-authored-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Mikhail Zabaluev <mikhail@informal.systems>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>

* Fixed bad merge

* Fixed changelog structure

* TxIndexer and BlockIndexer pruning metrics (cometbft#1334)

* added tx and block indexer pruning metrics

* add changelog

* added base height metric for tx and block indexer

* Fixed changelog order

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: werty144 <anton-paramonov2000@yandex.ru>
Co-authored-by: Adi Seredinschi <a@seredinschi.net>
Co-authored-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Mikhail Zabaluev <mikhail@informal.systems>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>

* Add compaction to pruner

* Reduced duplicate code in state_test.go

* Update test/e2e/generator/generate.go

Co-authored-by: Sergio Mena <sergio@informal.systems>

* Refactored mores tests

* Disabled data companion activation

* Fixed the pruned states and results counter

* Fixed the pruned states and results counter

* Prune indexer by iterating

* Refactoring based on review

* Removed unused argument from Prune function;

* enable abci result pruning by default

* Sergio's fix for pruning tx events as well and minor fixes in logs

Co-authored-by: Sergio Mena <sergio.mena@circle.com>

* Applied comments from PR review

* Fix linter and failing tests

* Fixed tests, prperly initialized retainHeights

* Fixes in config and indexder key retrieval

* Reverted the comparison to block store height whe nsetting indexer retain height - one can prune the blockstore and not prune the indexer store

* Fixed linter

* Removed printfs

* removed duplciate line

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Daniel Cason <daniel.cason@informal.systems>
Co-authored-by: werty144 <anton-paramonov2000@yandex.ru>
Co-authored-by: Adi Seredinschi <a@seredinschi.net>
Co-authored-by: Mikhail Zabaluev <mikhail@informal.systems>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
Co-authored-by: Sergio Mena <sergio@informal.systems>
Co-authored-by: Sergio Mena <sergio.mena@circle.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P:storage-optimization Priority: Give operators greater control over storage and storage optimization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ADR-101 v0.38.x: Backport block pruning mechanism
3 participants