From eb5a4493165c339b3c1fc7d6a94ed3e9193230ad Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 4 Feb 2025 00:08:03 +0100 Subject: [PATCH] core/rawdb: change freezerTableMeta.flushOffset to int64 The flushOffset is a file position, which is usually tracked as int64 in Go. It's better to keep it as the same type to avoid conversion. Overall, this change removes some conversions, and introduces some. The new conversions are in cases where flushOffset is compared with numEntries*indexEntrySize computations. --- core/rawdb/freezer_meta.go | 14 ++++++++++---- core/rawdb/freezer_meta_test.go | 4 ++-- core/rawdb/freezer_table.go | 26 +++++++++++++------------- core/rawdb/freezer_table_test.go | 2 +- 4 files changed, 26 insertions(+), 20 deletions(-) diff --git a/core/rawdb/freezer_meta.go b/core/rawdb/freezer_meta.go index 3b60847d3c..3cda9ae45c 100644 --- a/core/rawdb/freezer_meta.go +++ b/core/rawdb/freezer_meta.go @@ -19,8 +19,10 @@ package rawdb import ( "errors" "io" + "math" "os" + "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rlp" ) @@ -62,7 +64,7 @@ type freezerTableMeta struct { // // The offset could be moved forward by applying sync operation, or be moved // backward in cases of head/tail truncation, etc. - flushOffset uint64 + flushOffset int64 } // decodeV1 attempts to decode the metadata structure in v1 format. If fails or @@ -109,11 +111,15 @@ func decodeV2(file *os.File) *freezerTableMeta { if o.Version != freezerTableV2 { return nil } + if o.Offset > math.MaxInt64 { + log.Error("Invalid flushOffset %d in freezer metadata", o.Offset, "file", file.Name()) + return nil + } return &freezerTableMeta{ file: file, version: freezerTableV2, virtualTail: o.Tail, - flushOffset: o.Offset, + flushOffset: int64(o.Offset), } } @@ -152,7 +158,7 @@ func (m *freezerTableMeta) setVirtualTail(tail uint64, sync bool) error { } // setFlushOffset sets the flush offset and flushes the metadata if sync is true. -func (m *freezerTableMeta) setFlushOffset(offset uint64, sync bool) error { +func (m *freezerTableMeta) setFlushOffset(offset int64, sync bool) error { m.flushOffset = offset return m.write(sync) } @@ -167,7 +173,7 @@ func (m *freezerTableMeta) write(sync bool) error { var o obj o.Version = freezerVersion // forcibly use the current version o.Tail = m.virtualTail - o.Offset = m.flushOffset + o.Offset = uint64(m.flushOffset) _, err := m.file.Seek(0, io.SeekStart) if err != nil { diff --git a/core/rawdb/freezer_meta_test.go b/core/rawdb/freezer_meta_test.go index b02c230901..31f8c519c8 100644 --- a/core/rawdb/freezer_meta_test.go +++ b/core/rawdb/freezer_meta_test.go @@ -79,7 +79,7 @@ func TestUpgradeMetadata(t *testing.T) { if meta.virtualTail != uint64(100) { t.Fatal("Unexpected virtual tail field") } - if meta.flushOffset != uint64(0) { + if meta.flushOffset != 0 { t.Fatal("Unexpected flush offset field") } @@ -95,7 +95,7 @@ func TestUpgradeMetadata(t *testing.T) { if meta.virtualTail != uint64(100) { t.Fatal("Unexpected virtual tail field") } - if meta.flushOffset != uint64(100) { + if meta.flushOffset != 100 { t.Fatal("Unexpected flush offset field") } } diff --git a/core/rawdb/freezer_table.go b/core/rawdb/freezer_table.go index 114848f7b6..88351a7819 100644 --- a/core/rawdb/freezer_table.go +++ b/core/rawdb/freezer_table.go @@ -320,8 +320,8 @@ func (t *freezerTable) repair() error { // offset is updated, leaving a dangling reference that points to a position // outside the file. If so, the offset will be reset to the new end of the // file during the next run. - if t.metadata.flushOffset > uint64(newOffset) { - if err := t.metadata.setFlushOffset(uint64(newOffset), true); err != nil { + if t.metadata.flushOffset > newOffset { + if err := t.metadata.setFlushOffset(newOffset, true); err != nil { return err } } @@ -406,31 +406,31 @@ func (t *freezerTable) repairIndex() error { // index file to avoid clearing the entire table. if t.metadata.version == freezerTableV1 { t.logger.Info("Recovering freezer flushOffset for legacy table", "offset", size) - return t.metadata.setFlushOffset(uint64(size), true) + return t.metadata.setFlushOffset(size, true) } switch { case size == indexEntrySize && t.metadata.flushOffset == 0: // It's a new freezer table with no content. // Move the flush offset to the end of the file. - return t.metadata.setFlushOffset(uint64(size), true) + return t.metadata.setFlushOffset(size, true) - case size == int64(t.metadata.flushOffset): + case size == t.metadata.flushOffset: // flushOffset is aligned with the index file, all is well. return nil - case size > int64(t.metadata.flushOffset): + case size > t.metadata.flushOffset: // Extra index items have been detected beyond the flush offset. Since these // entries correspond to data that has not been fully flushed to disk in the // last run (because of unclean shutdown), their integrity cannot be guaranteed. // To ensure consistency, these index items will be truncated, as there is no // reliable way to validate or recover their associated data. - extraSize := size - int64(t.metadata.flushOffset) + extraSize := size - t.metadata.flushOffset if t.readonly { return fmt.Errorf("index file(path: %s, name: %s) contains %d garbage data bytes", t.path, t.name, extraSize) } t.logger.Warn("Truncating freezer items after flushOffset", "size", extraSize) - return truncateFreezerFile(t.index, int64(t.metadata.flushOffset)) + return truncateFreezerFile(t.index, t.metadata.flushOffset) default: // size < flushOffset // Flush offset refers to a position larger than index file. The only @@ -442,7 +442,7 @@ func (t *freezerTable) repairIndex() error { return nil // do nothing in read only mode } t.logger.Warn("Rewinding freezer flushOffset", "old", t.metadata.flushOffset, "new", size) - return t.metadata.setFlushOffset(uint64(size), true) + return t.metadata.setFlushOffset(size, true) } } @@ -629,8 +629,8 @@ func (t *freezerTable) truncateHead(items uint64) error { // offset is updated, leaving a dangling reference that points to a position // outside the file. If so, the offset will be reset to the new end of the // file during the next run. - if t.metadata.flushOffset > newOffset { - if err := t.metadata.setFlushOffset(newOffset, true); err != nil { + if t.metadata.flushOffset > int64(newOffset) { + if err := t.metadata.setFlushOffset(int64(newOffset), true); err != nil { return err } } @@ -811,7 +811,7 @@ func (t *freezerTable) truncateTail(items uint64) error { // // Note, both the index and head data file has been persisted before performing // tail truncation and all the items in these files are regarded as complete. - shorten := indexEntrySize * (newDeleted - deleted) + shorten := indexEntrySize * int64(newDeleted-deleted) if t.metadata.flushOffset <= shorten { return fmt.Errorf("invalid index flush offset: %d, shorten: %d", t.metadata.flushOffset, shorten) } else { @@ -1197,7 +1197,7 @@ func (t *freezerTable) syncWithNoLock() error { return err } offset := stat.Size() - trackError(t.metadata.setFlushOffset(uint64(offset), true)) + trackError(t.metadata.setFlushOffset(offset, true)) return err } diff --git a/core/rawdb/freezer_table_test.go b/core/rawdb/freezer_table_test.go index 4bbb3aaf70..9a72af6ccc 100644 --- a/core/rawdb/freezer_table_test.go +++ b/core/rawdb/freezer_table_test.go @@ -1432,7 +1432,7 @@ func TestFlushOffsetTracking(t *testing.T) { var cases = []struct { op func(*freezerTable) - offset uint64 + offset int64 }{ { // Data files: