Add offset support to MemoryFreezer#1965
Add offset support to MemoryFreezer#1965LarryArnault45 wants to merge 3 commits into0xPolygon:developfrom
Conversation
|
This PR is stale because it has been open 21 days with no activity. Remove stale label or comment or this will be closed in 14 days. |
LarryArnault45
force-pushed
the
memory-freezer-offset
branch
from
0fab17e to
6097d13
Compare
|
|
Hi @pratikspatil024, I fixed the CI issues. Could you approve the workflow runs when you get a chance? Thanks! |
|
This PR is stale because it has been open 21 days with no activity. Remove stale label or comment or this will be closed in 14 days. |
|
@LarryArnault45 - could you push an empty commit? |
There was a problem hiding this comment.
Pull request overview
This PR adds pruning-offset support to the in-memory ancient store (MemoryFreezer) so it can represent an ancient DB that starts at a non-zero block number (e.g., after pruning older ancients), aligning behavior with the file-based freezer’s offset concept.
Changes:
- Extend
NewMemoryFreezerto accept anoffsetand initialize internal counters accordingly. - Apply the offset in memory freezer write sequencing and in
Ancient,TruncateHead, andTruncateTail. - Update call sites and tests to use the new
NewMemoryFreezersignature.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
core/rawdb/freezer_memory.go |
Adds offset parameter + uses it in read/write/truncation paths for the in-memory freezer. |
core/rawdb/freezer_memory_test.go |
Updates tests to the new constructor signature. |
core/rawdb/chain_freezer.go |
Passes the pruning offset into the in-memory freezer when running without a datadir. |
core/rawdb/ancient_scheme.go |
Updates state freezer’s in-memory path to pass offset 0. |
Comments suppressed due to low confidence (1)
core/rawdb/freezer_memory.go:268
Ancientnow subtractsf.offsetfromnumberwithout guardingnumber < offset. With unsigned arithmetic this underflows and relies on downstream bounds checks to fail; it also makes the error cause less clear. Consider explicitly returningerrOutOfBounds(or a dedicated error) whennumber < f.offset.Load(), and ensureAncientRangeapplies the same offset semantics for consistency (currently it passesstartthrough unchanged).
func (f *MemoryFreezer) Ancient(kind string, number uint64) ([]byte, error) {
f.lock.RLock()
defer f.lock.RUnlock()
t := f.tables[kind]
if t == nil {
return nil, errUnknownTable
}
data, err := t.retrieve(number-f.offset.Load(), 1, 0)
if err != nil {
return nil, err
}
return data[0], nil
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| return NewMemoryFreezer(false, tables) | ||
| return NewMemoryFreezer(false, 0, tables) | ||
| }) | ||
| } |
| // NewMemoryFreezer initializes an in-memory freezer instance. | ||
| func NewMemoryFreezer(readonly bool, tableName map[string]freezerTableConfig) *MemoryFreezer { | ||
| func NewMemoryFreezer(readonly bool, offset uint64, tableName map[string]freezerTableConfig) *MemoryFreezer { | ||
| tables := make(map[string]*memoryTable) | ||
| for name, cfg := range tableName { | ||
| tables[name] = newMemoryTable(name, cfg) | ||
| } | ||
| return &MemoryFreezer{ | ||
| freezer := &MemoryFreezer{ | ||
| writeBatch: newMemoryBatch(), | ||
| readonly: readonly, | ||
| tables: tables, | ||
| } | ||
|
|
||
| freezer.offset.Store(offset) | ||
|
|
||
| // Some blocks in ancientDB may have already been frozen and been pruned, so adding the offset to | ||
| // represent the absolute number of blocks already frozen. | ||
| freezer.items += offset | ||
|
|
||
| return freezer | ||
| } |
| return old, nil | ||
| } | ||
| for _, table := range f.tables { | ||
| if err := table.truncateHead(items); err != nil { | ||
| if err := table.truncateHead(items - f.offset.Load()); err != nil { | ||
| return 0, err | ||
| } | ||
| } |
| } | ||
| for _, table := range f.tables { | ||
| if table.config.prunable { | ||
| if err := table.truncateTail(tail); err != nil { | ||
| if err := table.truncateTail(tail - f.offset.Load()); err != nil { | ||
| return 0, err | ||
| } | ||
| } |
|
|
@pratikspatil024 , did it, sir. |
MemoryFreezer was missing offset handling, causing issues when used with ancient database pruning.