From ea04113558acfd6317143ffd728605ff8a4de2c8 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Tue, 15 Feb 2022 14:46:04 +0800 Subject: [PATCH] core/rawdb: fixes cornercase in tail deletion --- core/rawdb/freezer_meta.go | 5 ++-- core/rawdb/freezer_table.go | 49 +++++++++++++++++++------------- core/rawdb/freezer_table_test.go | 14 ++------- 3 files changed, 35 insertions(+), 33 deletions(-) diff --git a/core/rawdb/freezer_meta.go b/core/rawdb/freezer_meta.go index a77e563f9a39a..cf0aadb5b66dd 100644 --- a/core/rawdb/freezer_meta.go +++ b/core/rawdb/freezer_meta.go @@ -20,6 +20,7 @@ import ( "bytes" "errors" "fmt" + "io" "os" "github.com/ethereum/go-ethereum/rlp" @@ -175,7 +176,7 @@ func upgradeTableIndex(index *os.File, version uint16) (*os.File, *freezerTableM return nil, nil, err } default: - return nil, nil, errors.New("unknown freezer table index") + return nil, nil, errors.New("unknown freezer table version") } // Reopen the upgraded index file and load the metadata from it index, err := os.Open(index.Name()) @@ -205,7 +206,7 @@ func repairTableIndex(index *os.File) (*os.File, *freezerTableMeta, error) { return nil, nil, err } // Shift file cursor to the end for next write operation - _, err = index.Seek(0, 2) + _, err = index.Seek(0, io.SeekEnd) if err != nil { return nil, nil, err } diff --git a/core/rawdb/freezer_table.go b/core/rawdb/freezer_table.go index aaf81a0c757e9..f380cad3ecc01 100644 --- a/core/rawdb/freezer_table.go +++ b/core/rawdb/freezer_table.go @@ -406,6 +406,17 @@ func (t *freezerTable) truncateHead(items uint64) error { if existing <= items { return nil } + // Calculate the relative offset between the new head and tail, use + // it to access the corresponding index entry. If the requested target + // is even below the freezer tail, reject it. + var ( + itemOffset = atomic.LoadUint64(&t.itemOffset) + itemHidden = atomic.LoadUint64(&t.itemHidden) + tail = itemOffset + itemHidden + ) + if items < tail { + return errors.New("truncation below tail") + } // We need to truncate, save the old size for metrics tracking oldSize, err := t.sizeNolock() if err != nil { @@ -418,19 +429,7 @@ func (t *freezerTable) truncateHead(items uint64) error { } log("Truncating freezer table", "items", existing, "limit", items) - // Calculate the relative offset between the new head and tail, use - // it to access the corresponding index entry. If the requested target - // is even below the freezer tail, reject it. - var ( - itemOffset = atomic.LoadUint64(&t.itemOffset) - itemHidden = atomic.LoadUint64(&t.itemHidden) - tail = itemOffset + itemHidden - ) - if items < tail { - return errors.New("truncation below tail") - } offset := items - itemOffset - if err := truncateFreezerFile(t.index, int64(offset)*indexEntrySize+metaLength); err != nil { return err } @@ -498,7 +497,7 @@ func (t *freezerTable) truncateIndexFile(originDeleted, deleted, hidden uint64, return nil } -// truncateHead discards any recent data before the provided threshold number. +// truncateTail discards any recent data before the provided threshold number. func (t *freezerTable) truncateTail(items uint64) error { t.lock.Lock() defer t.lock.Unlock() @@ -516,17 +515,27 @@ func (t *freezerTable) truncateTail(items uint64) error { return errors.New("truncation above head") } // Load the index of new tail item after the deletion. - newTail, err := t.getIndex(int64(items-deleted), 0) + count, err := t.indexLen() if err != nil { return err } + var tailId uint32 + if uint64(count) == items-deleted { + tailId = t.headId + } else { + newTail, err := t.getIndex(int64(items-deleted), 0) + if err != nil { + return err + } + tailId = newTail.filenum + } // Freezer only supports deletion by file, just mark the entries as hidden - if t.tailId == newTail.filenum { + if t.tailId == tailId { atomic.StoreUint64(&t.itemHidden, items-deleted) return storeMetadata(t.index, newMetadata(t.tailId, deleted, items-deleted)) } - if t.tailId > newTail.filenum { - return fmt.Errorf("invalid index, tail-file %d, item-file %d", t.tailId, newTail.filenum) + if t.tailId > tailId { + return fmt.Errorf("invalid index, tail-file %d, item-file %d", t.tailId, tailId) } // We need to truncate, save the old size for metrics tracking oldSize, err := t.sizeNolock() @@ -540,16 +549,16 @@ func (t *freezerTable) truncateTail(items uint64) error { if err != nil { return err } - if cur.filenum != newTail.filenum { + if cur.filenum != tailId { break } newDeleted = current } - if err := t.truncateIndexFile(deleted, newDeleted, items-newDeleted, newTail.filenum); err != nil { + if err := t.truncateIndexFile(deleted, newDeleted, items-newDeleted, tailId); err != nil { return err } // Release any files before the current tail - t.tailId = newTail.filenum + t.tailId = tailId atomic.StoreUint64(&t.itemOffset, newDeleted) atomic.StoreUint64(&t.itemHidden, items-newDeleted) t.releaseFilesBefore(t.tailId, true) diff --git a/core/rawdb/freezer_table_test.go b/core/rawdb/freezer_table_test.go index dbbbabac19405..c2101f45a6952 100644 --- a/core/rawdb/freezer_table_test.go +++ b/core/rawdb/freezer_table_test.go @@ -760,7 +760,7 @@ func TestTruncateTail(t *testing.T) { }) // truncate all, the entire freezer should be deleted - f.truncateTail(6) + f.truncateTail(7) checkRetrieveError(t, f, map[uint64]error{ 0: errOutOfBounds, 1: errOutOfBounds, @@ -768,13 +768,11 @@ func TestTruncateTail(t *testing.T) { 3: errOutOfBounds, 4: errOutOfBounds, 5: errOutOfBounds, - }) - checkRetrieve(t, f, map[uint64][]byte{ - 6: getChunk(20, 0x11), + 6: errOutOfBounds, }) } -func TestTruncateHeadBelowTail(t *testing.T) { +func TestTruncateHead(t *testing.T) { t.Parallel() rm, wm, sg := metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge() fname := fmt.Sprintf("truncate-head-blow-tail-%d", rand.Uint64()) @@ -822,12 +820,6 @@ func TestTruncateHeadBelowTail(t *testing.T) { 5: getChunk(20, 0xaa), 6: getChunk(20, 0x11), }) - - f.truncateTail(5) // Lazy deleted the item-4, it's hidden - f.truncateHead(5) // New head is reset to item-4 - checkRetrieveError(t, f, map[uint64]error{ - 4: errOutOfBounds, // Hidden item - }) } func TestUpgradeLegacyFreezerTable(t *testing.T) {