[Access] Add registerDB pruning module #6397
Conversation
…ub.com:The-K-R-O-K/flow-go into UlyanaAndrukhiv/6068-registerDB-pruning-module
…ub.com:The-K-R-O-K/flow-go into UlyanaAndrukhiv/6068-registerDB-pruning-module
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6397 +/- ##
==========================================
- Coverage 41.26% 41.24% -0.02%
==========================================
Files 2061 2067 +6
Lines 182702 183019 +317
==========================================
+ Hits 75384 75485 +101
- Misses 101010 101211 +201
- Partials 6308 6323 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…ub.com:The-K-R-O-K/flow-go into UlyanaAndrukhiv/6068-registerDB-pruning-module
| node.Logger, | ||
| builder.RegisterDB, | ||
| pstorage.WithPrunerMetrics(builder.RegisterDBPrunerMetrics), | ||
| //pstorage.WithPruneThreshold(builder.registerDBPruneThreshold), |
There was a problem hiding this comment.
WithPruneThreshold is temporarily commented out and will be re-enabled once PR #6345 is merged.
…ling according to comments
…ub.com:The-K-R-O-K/flow-go into UlyanaAndrukhiv/6068-registerDB-pruning-module
Guitarheroua
requested review from
Guitarheroua and
zhangchiqing
and removed request for
Guitarheroua
| } | ||
|
|
||
| // Reset batchKeysToRemove to empty slice while retaining capacity | ||
| batchKeysToRemove = batchKeysToRemove[:0] |
There was a problem hiding this comment.
We could reset before sleeping.
also clear(batchKeysToRemove) would be better
There was a problem hiding this comment.
I thought about this as well. it turns our clear() called on a slice just sets all of the elements in the array to nil rather than changing the size of slice, so it wouldn't work here
| }() | ||
|
|
||
| for _, key := range lookupKeys { | ||
| if err := dbBatch.Delete(key, nil); err != nil { |
There was a problem hiding this comment.
Pebble batch support DeleteRange.
I wonder if it would be more efficient with it.
For instance, if we have the following registers and different height:
registerA-height100
registerA-height99
registerA-height98
registerA-height90
And we'd like to prune by height 100.
Instead of calling:
dbBatch.Delete(registerA-height99, pebble.Sync)
dbBatch.Delete(registerA-height98, pebble.Sync)
dbBatch.Delete(registerA-height90, pebble.Sync)
we could call:
dbBatch.DeleteRange(registerA-height99, registerA-height0, pebble.Sync)
I wonder if that would make a difference?
There was a problem hiding this comment.
In a lookupKeys variable we could have a mix of keys from different batches. For example:
registerA-height98
registerA-height90
...
registerB-height98
...
registerC-height98
registerC-height97
So, we need to call lookupKeyToRegisterID one extra time to group them, or we should group them on a step when we check the key.
Another Idea could be, to save them as a map of ranges, for example:
[registerA]{registerA-height98, registerA-height90},
[registerB]{registerB-height98, registerB-height98},
[registerC]{registerC-height98, registerC-height97}
…ub.com:The-K-R-O-K/flow-go into UlyanaAndrukhiv/6068-registerDB-pruning-module
…drukhiv/6068-registerDB-pruning-module
Closes #6068
In this PR:
prunermodule forregisterDBwhich will ensure that unneeded pruned data is removed from the db, freeing up disk space.prunerinto Access and Observer nodes.