feat(manager): run dymint store block pruning in background #1053
Conversation
|
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you! |
There was a problem hiding this comment.
I can't see any bugs but there are a lot of errors which are not logged or propogated
Also it's not very DRY wrt other pruning code, especially things like if pruned%1000 == 0 && pruned > 0 where these magic numbers
Are you sure everywhere the error is ignored it's really a good idea?
Could you clean up some of the duplicated code/magic nums?
Not a blocker so will approve if you think it's ok
| blockCache: &Cache{ | ||
| cache: make(map[uint64]types.CachedBlock), | ||
| }, | ||
| pruningC: make(chan int64, 10), // use of buffered channel to avoid blocking applyBlock thread. In case channel is full, pruning will be skipped, but the retain height can be pruned in the next iteration. |
There was a problem hiding this comment.
what is the different between using size 10 and size 0 here? isn't 0 effectively the same, but simpler?
There was a problem hiding this comment.
imo is very similar but not exactly the same. in very active rollapps is probably the same. in not that active rollapps it gives some more chance to finish and prune later blocks that otherwise can be left unpruned till new blocks are created, but i agree there is not a big difference.
indexers/blockindexer/kv/kv.go
Outdated
| func (idx *BlockerIndexer) pruneBlocks(from, to uint64) (uint64, error) { | ||
| pruned := uint64(0) | ||
| batch := idx.store.NewBatch() | ||
| defer batch.Discard() | ||
|
|
||
| flush := func(batch store.KVBatch, height int64) error { | ||
| err := batch.Commit() | ||
| if err != nil { | ||
| return fmt.Errorf("flush batch to disk: height %d: %w", height, err) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| for h := int64(from); h < int64(to); h++ { | ||
| ok, err := idx.Has(h) | ||
| if err != nil || !ok { | ||
| continue | ||
| } | ||
| key, err := heightKey(h) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| if err := batch.Delete(key); err != nil { | ||
| continue | ||
| } | ||
| if err := idx.pruneEvents(h, batch); err != nil { | ||
| continue | ||
| } | ||
| pruned++ | ||
|
|
||
| // flush every 1000 blocks to avoid batches becoming too large | ||
| if pruned%1000 == 0 && pruned > 0 { | ||
| err := flush(batch, h) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| batch.Discard() | ||
| batch = idx.store.NewBatch() | ||
| } | ||
| } |
There was a problem hiding this comment.
looks like a bit duplicated logic with the other PR/other pruning logic? nevermind
indexers/txindex/kv/kv_test.go
Outdated
| @@ -326,6 +372,49 @@ func txResultWithEvents(events []abci.Event) *abci.TxResult { | |||
| }, | |||
| } | |||
| } | |||
| func getRandomTxResult(height int64, events []abci.Event) *abci.TxResult { | |||
| tx := types.Tx(randomTxHash()) | |||
| fmt.Println(tx.Hash()) | |||
indexers/blockindexer/kv/kv.go
Outdated
| if err != nil { | ||
| continue | ||
| } | ||
| if err := batch.Delete(key); err != nil { | ||
| continue | ||
| } | ||
| if err := idx.pruneEvents(h, batch); err != nil { | ||
| continue | ||
| } | ||
| pruned++ |
| if from <= 0 { | ||
| return 0, fmt.Errorf("from height must be greater than 0: %w", gerrc.ErrInvalidArgument) | ||
| } | ||
|
|
||
| if to <= from { | ||
| return 0, fmt.Errorf("to height must be greater than from height: to: %d: from: %d: %w", to, from, gerrc.ErrInvalidArgument) | ||
| } |
There was a problem hiding this comment.
i think it does not harm, and it avoids executing unnecessary code, although it should not happen.
srene
force-pushed
the
srene/1038-pruning-bg
branch
from
2a19b2e
to
441a4e9
Compare
(cherry picked from commit 5055ae7)
(cherry picked from commit 5055ae7)
PR Standards
Opening a pull request should be able to meet the following requirements
--
PR naming convention: https://hackmd.io/@nZpxHZ0CT7O5ngTp0TP9mg/HJP_jrm7A
Close #1038
<-- Briefly describe the content of this pull request -->
For Author:
godoccommentsFor Reviewer:
After reviewer approval: