feat(pruning): trigger explicitly compaction upon pruning (cometbft#1972

)

Blocked on cometbft/cometbft-db#111 and
benchmarking


Addresses #49 

Upon pruning we explicitly call the compaction function of the DB
backend. This has shown to immediately shrink the storage footprint.

We need to evaluate the duration of this compaction depending on the
storage size to be able to reason about the impact on Comet's regular
operations.

ToDo: 
-Extend the `storage` config section with following parameters:
- [ ] `in-process-compaction = false #Enable or disable in-process
compaction. False by default`
- [ ] `in-process-compaction-interval = 10 #Interval in number of blocks
to trigger explicit compaction; 10 by default`



<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments

---------

Co-authored-by: Andy Nogueira <[email protected]>
Co-authored-by: Anton Kaliaev <[email protected]>

.changelog/unreleased/breaking-changes/1972-store-prune-abci-responses.md

@@ -0,0 +1 @@
+- `[state/store]` go-API breaking change in `PruneABCIResponses`: added parameter to force compaction. ([\#1972](https://github.com/cometbft/cometbft/pull/1972))

.changelog/unreleased/breaking-changes/1972-store-pruning-api-breaking.md

@@ -0,0 +1 @@
+- `[state/store]` go-API breaking change in `PruneStates`: added parameter to pass the number of pruned states and return pruned entries in current pruning iteration. ([\#1972](https://github.com/cometbft/cometbft/pull/1972))

.changelog/unreleased/features/1972-compaction-config.md

@@ -0,0 +1 @@
+- `[config]` Add configuration parameters to tweak forced compaction. ([\#1972](https://github.com/cometbft/cometbft/pull/1972))

.changelog/unreleased/features/1972-force-compaction-on-pruning.md

@@ -0,0 +1 @@
+- `[store]` When pruning force compaction of the database. ([\#1972](https://github.com/cometbft/cometbft/pull/1972))

config/config.go

@@ -1272,7 +1272,21 @@ type StorageConfig struct {
 	DiscardABCIResponses bool `mapstructure:"discard_abci_responses"`
 	// Configuration related to storage pruning.
 	Pruning *PruningConfig `mapstructure:"pruning"`
-
+	// Compaction on pruning - enable or disable in-process compaction.
+	// If the DB backend supports it, this will force the DB to compact
+	// the database levels and save on storage space. Setting this to true
+	// is most beneficial when used in combination with pruning as it will
+	// phyisically delete the entries marked for deletion.
+	// false by default (forcing compaction is disabled).
+	Compact bool `mapstructure:"compact"`
+	// Compaction interval - number of blocks to try explicit compaction on.
+	// This parameter should be tuned depending on the number of items
+	// you expect to delete between two calls to forced compaction.
+	// If your retain height is 1 block, it is too much of an overhead
+	// to try compaction every block. But it should also not be a very
+	// large multiple of your retain height as it might occur bigger overheads.
+	// 1000 by default.
+	CompactionInterval int64 `mapstructure:"compaction_interval"`
 	// Hex representation of the hash of the genesis file.
 	// This is an optional parameter set when an operator provides
 	// a hash via the command line.
@@ -1288,6 +1302,8 @@ func DefaultStorageConfig() *StorageConfig {
 	return &StorageConfig{
 		DiscardABCIResponses: false,
 		Pruning:              DefaultPruningConfig(),
+		Compact:              false,
+		CompactionInterval:   1000,
 		GenesisHash:          "",
 	}
 }

config/toml.go

@@ -583,6 +583,18 @@ initial_block_retain_height = {{ .Storage.Pruning.DataCompanion.InitialBlockReta
 # already set a block results retain height, this is ignored.
 initial_block_results_retain_height = {{ .Storage.Pruning.DataCompanion.InitialBlockResultsRetainHeight }}
 
+# If set to true, CometBFT will force compaction to happen for databases that support this feature.
+# and save on storage space. Setting this to true is most benefits when used in combination
+# with pruning as it will phyisically delete the entries marked for deletion.
+# false by default (forcing compaction is disabled).
+compact = false
+
+# To avoid forcing compaction every time, this parameter instructs CometBFT to wait 
+# the given amount of blocks to be pruned before triggering compaction.
+# It should be tuned depending on the number of items. If your retain height is 1 block,
+# it is too much of an overhead to try compaction every block. But it should also not be a very
+# large multiple of your retain height as it might occur bigger overheads.
+compaction_interval = 1000
 
 # Hash of the Genesis file (as hex string), passed to CometBFT via the command line.
 # If this hash mismatches the hash that CometBFT computes on the genesis file,

internal/state/indexer/block/indexer.go

@@ -27,7 +27,7 @@ func IndexerFromConfig(cfg *config.Config, dbProvider config.DBProvider, chainID
 			return nil, nil, err
 		}
 
-		return kv.NewTxIndex(store), blockidxkv.New(dbm.NewPrefixDB(store, []byte("block_events"))), nil
+		return kv.NewTxIndex(store), blockidxkv.New(dbm.NewPrefixDB(store, []byte("block_events")), blockidxkv.WithCompaction(cfg.Storage.Compact, cfg.Storage.CompactionInterval)), nil
 
 	case "psql":
 		conn := cfg.TxIndex.PsqlConn

internal/state/indexer/block/kv/kv.go

@@ -39,12 +39,31 @@ type BlockerIndexer struct {
 	// Matching will be done both on height AND eventSeq
 	eventSeq int64
 	log      log.Logger
+
+	compact            bool
+	compactionInterval int64
+	lastPruned         int64
 }
+type IndexerOption func(*BlockerIndexer)
 
-func New(store dbm.DB) *BlockerIndexer {
-	return &BlockerIndexer{
+// WithCompaction sets the compaciton parameters.
+func WithCompaction(compact bool, compactionInterval int64) IndexerOption {
+	return func(idx *BlockerIndexer) {
+		idx.compact = compact
+		idx.compactionInterval = compactionInterval
+	}
+}
+
+func New(store dbm.DB, options ...IndexerOption) *BlockerIndexer {
+	bsIndexer := &BlockerIndexer{
 		store: store,
 	}
+
+	for _, option := range options {
+		option(bsIndexer)
+	}
+
+	return bsIndexer
 }
 
 func (idx *BlockerIndexer) SetLogger(l log.Logger) {
@@ -141,7 +160,7 @@ func (idx *BlockerIndexer) Prune(retainHeight int64) (int64, int64, error) {
 	if err != nil {
 		return 0, lastRetainHeight, err
 	}
-	deleted := 0
+	deleted := int64(0)
 	affectedHeights := make(map[int64]struct{})
 	for ; itr.Valid(); itr.Next() {
 		if keyBelongsToHeightRange(itr.Key(), lastRetainHeight, retainHeight) {
@@ -172,6 +191,12 @@ func (idx *BlockerIndexer) Prune(retainHeight int64) (int64, int64, error) {
 		return 0, lastRetainHeight, errWriteBatch
 	}
 
+	if idx.compact && idx.lastPruned+deleted >= idx.compactionInterval {
+		err = idx.store.Compact(nil, nil)
+		idx.lastPruned = 0
+	}
+	idx.lastPruned += deleted
+
 	return int64(len(affectedHeights)), retainHeight, err
 }
 

internal/state/pruner.go

@@ -38,6 +38,10 @@ type Pruner struct {
 	interval     time.Duration
 	observer     PrunerObserver
 	metrics      *Metrics
+
+	// Preserve the number of state entries pruned.
+	// Used to calculated correctly when to trigger compactions
+	prunedStates uint64
 }
 
 type prunerConfig struct {
@@ -446,10 +450,15 @@ func (p *Pruner) pruneABCIResToRetainHeight(lastRetainHeight int64) int64 {
 		return lastRetainHeight
 	}
 
+	// If the block retain height is 0, pruning of the block and state stores might be disabled
+	// This should not prevent Comet from pruning ABCI results if needed.
+	// We could by default always compact when pruning the responses, but in case the state store
+	// is being compacted we introduce an overhead that might cause performance penalties.
+	forceCompact := p.findMinBlockRetainHeight() == 0
 	// newRetainHeight is the height just after that which we have successfully
 	// pruned. In case of an error it will be 0, but then it will also be
 	// ignored.
-	numPruned, newRetainHeight, err := p.stateStore.PruneABCIResponses(targetRetainHeight)
+	numPruned, newRetainHeight, err := p.stateStore.PruneABCIResponses(targetRetainHeight, forceCompact)
 	if err != nil {
 		p.logger.Error("Failed to prune ABCI responses", "err", err, "targetRetainHeight", targetRetainHeight)
 		return lastRetainHeight
@@ -501,7 +510,9 @@ func (p *Pruner) pruneBlocksToHeight(height int64) (uint64, int64, error) {
 		return 0, 0, ErrFailedToPruneBlocks{Height: height, Err: err}
 	}
 	if pruned > 0 {
-		if err := p.stateStore.PruneStates(base, height, evRetainHeight); err != nil {
+		prunedStates, err := p.stateStore.PruneStates(base, height, evRetainHeight, p.prunedStates)
+		p.prunedStates += prunedStates
+		if err != nil {
 			return 0, 0, ErrFailedToPruneStates{Height: height, Err: err}
 		}
 	}