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

jmalicevic · thanethomson · web-flow · commit fe644d81ed39 · 2023-08-15T12:57:51.000+02:00
* Applied @serigo-mena's comments

* Update state/pruner.go

Co-authored-by: Thane Thomson &lt;connect@thanethomson.com&gt;

* Applied further comments from PR

* Fixed batching interval

---------

Co-authored-by: Thane Thomson &lt;connect@thanethomson.com&gt;
diff --git a/config/config.go b/config/config.go
@@ -1376,7 +1376,7 @@ func getDefaultMoniker() string {
 
 type PruningConfig struct {
 	// The time period between automated background pruning operations.
-	Interval time.Duration `mapstructure:"period"`
+	Interval time.Duration `mapstructure:"interval"`
 	// Data companion-related pruning configuration.
 	DataCompanion *DataCompanionPruningConfig `mapstructure:"data_companion"`
 }
diff --git a/state/pruner.go b/state/pruner.go
@@ -287,7 +287,7 @@ func (p *Pruner) pruneBlocksToRetainHeight(lastRetainHeight int64) int64 {
 		p.logger.Error("Failed to prune blocks", "err", err, "targetRetainHeight", targetRetainHeight, "newRetainHeight", newRetainHeight)
 	} else if pruned > 0 {
 		p.metrics.BlockStoreBaseHeight.Set(float64(newRetainHeight))
-		p.logger.Info("Pruned blocks", "count", pruned, "evidenceRetainHeight", evRetainHeight, "newRetainHeight", newRetainHeight)
+		p.logger.Debug("Pruned blocks", "count", pruned, "evidenceRetainHeight", evRetainHeight, "newRetainHeight", newRetainHeight)
 	}
 	return newRetainHeight
 }
@@ -332,13 +332,15 @@ func (p *Pruner) findMinRetainHeight() int64 {
 	appRetainHeight, err := p.stateStore.GetApplicationRetainHeight()
 	if err != nil {
 		if !errors.Is(err, ErrKeyNotFound) {
+			p.logger.Error("Unexpected error fetching application retain height", "err", err)
 			return 0
 		}
 		noAppRetainHeightSet = true
 	}
 	dcRetainHeight, err := p.stateStore.GetCompanionBlockRetainHeight()
 	if err != nil {
 		if !errors.Is(err, ErrKeyNotFound) {
+			p.logger.Error("Unexpected error fetching data companion retain height", "err", err)
 			return 0
 		}
 		// The Application height was set so we can return that immediately
diff --git a/state/store.go b/state/store.go
@@ -394,7 +394,7 @@ func (store dbStore) PruneStates(from int64, to int64, evidenceThresholdHeight i
 // pruned and the new retain height.
 func (store dbStore) PruneABCIResponses(targetRetainHeight int64) (int64, int64, error) {
 	if store.DiscardABCIResponses {
-		return 0, 0, errors.New("ABCI responses are discarded, nothing to prune")
+		return 0, 0, nil
 	}
 	lastRetainHeight, err := store.getLastABCIResponsesRetainHeight()
 	if err != nil {
@@ -415,7 +415,7 @@ func (store dbStore) PruneABCIResponses(targetRetainHeight int64) (int64, int64,
 			return pruned, lastRetainHeight + pruned, fmt.Errorf("failed to delete ABCI responses at height %d: %w", h, err)
 		}
 		batchPruned++
-		if batchPruned%1000 == 0 && batchPruned > 0 {
+		if batchPruned >= 1000 {
 			if err := batch.Write(); err != nil {
 				return pruned, lastRetainHeight + pruned, fmt.Errorf("failed to write ABCI responses deletion batch at height %d: %w", h, err)
 			}

Original file line number	Diff line number	Diff line change
`@@ -1376,7 +1376,7 @@ func getDefaultMoniker() string {`
`1376`	`1376`
`1377`	`1377`	`type PruningConfig struct {`
`1378`	`1378`	`// The time period between automated background pruning operations.`
`1379`		- Interval time.Duration `mapstructure:"period"`
	`1379`	+ Interval time.Duration `mapstructure:"interval"`
`1380`	`1380`	`// Data companion-related pruning configuration.`
`1381`	`1381`	DataCompanion *DataCompanionPruningConfig `mapstructure:"data_companion"`
`1382`	`1382`	`}`
Original file line number	Diff line number	Diff line change
`@@ -287,7 +287,7 @@ func (p *Pruner) pruneBlocksToRetainHeight(lastRetainHeight int64) int64 {`
`287`	`287`	`p.logger.Error("Failed to prune blocks", "err", err, "targetRetainHeight", targetRetainHeight, "newRetainHeight", newRetainHeight)`
`288`	`288`	`} else if pruned > 0 {`
`289`	`289`	`p.metrics.BlockStoreBaseHeight.Set(float64(newRetainHeight))`
`290`		`- p.logger.Info("Pruned blocks", "count", pruned, "evidenceRetainHeight", evRetainHeight, "newRetainHeight", newRetainHeight)`
	`290`	`+ p.logger.Debug("Pruned blocks", "count", pruned, "evidenceRetainHeight", evRetainHeight, "newRetainHeight", newRetainHeight)`
`291`	`291`	`}`
`292`	`292`	`return newRetainHeight`
`293`	`293`	`}`
`@@ -332,13 +332,15 @@ func (p *Pruner) findMinRetainHeight() int64 {`
`332`	`332`	`appRetainHeight, err := p.stateStore.GetApplicationRetainHeight()`
`333`	`333`	`if err != nil {`
`334`	`334`	`if !errors.Is(err, ErrKeyNotFound) {`
	`335`	`+ p.logger.Error("Unexpected error fetching application retain height", "err", err)`
`335`	`336`	`return 0`
`336`	`337`	`}`
`337`	`338`	`noAppRetainHeightSet = true`
`338`	`339`	`}`
`339`	`340`	`dcRetainHeight, err := p.stateStore.GetCompanionBlockRetainHeight()`
`340`	`341`	`if err != nil {`
`341`	`342`	`if !errors.Is(err, ErrKeyNotFound) {`
	`343`	`+ p.logger.Error("Unexpected error fetching data companion retain height", "err", err)`
`342`	`344`	`return 0`
`343`	`345`	`}`
`344`	`346`	`// The Application height was set so we can return that immediately`
Original file line number	Diff line number	Diff line change
`@@ -394,7 +394,7 @@ func (store dbStore) PruneStates(from int64, to int64, evidenceThresholdHeight i`
`394`	`394`	`// pruned and the new retain height.`
`395`	`395`	`func (store dbStore) PruneABCIResponses(targetRetainHeight int64) (int64, int64, error) {`
`396`	`396`	`if store.DiscardABCIResponses {`
`397`		`- return 0, 0, errors.New("ABCI responses are discarded, nothing to prune")`
	`397`	`+ return 0, 0, nil`
`398`	`398`	`}`
`399`	`399`	`lastRetainHeight, err := store.getLastABCIResponsesRetainHeight()`
`400`	`400`	`if err != nil {`
`@@ -415,7 +415,7 @@ func (store dbStore) PruneABCIResponses(targetRetainHeight int64) (int64, int64,`
`415`	`415`	`return pruned, lastRetainHeight + pruned, fmt.Errorf("failed to delete ABCI responses at height %d: %w", h, err)`
`416`	`416`	`}`
`417`	`417`	`batchPruned++`
`418`		`- if batchPruned%1000 == 0 && batchPruned > 0 {`
	`418`	`+ if batchPruned >= 1000 {`
`419`	`419`	`if err := batch.Write(); err != nil {`
`420`	`420`	`return pruned, lastRetainHeight + pruned, fmt.Errorf("failed to write ABCI responses deletion batch at height %d: %w", h, err)`
`421`	`421`	`}`