From bcf19bc4be7d37671831c4ca3b5fa527c13afdf2 Mon Sep 17 00:00:00 2001
From: zhangsoledad <787953403@qq.com>
Date: Wed, 17 Jun 2020 15:41:07 +0800
Subject: [PATCH] core/rawdb: swap tailId and itemOffset for deleted items in
 freezer (#21220)

* fix(freezer): tailId filenum offset were misplaced

* core/rawdb: assume first item in freezer always start from zero
---
 core/rawdb/freezer_table.go      | 25 ++++++++++++-----
 core/rawdb/freezer_table_test.go | 46 +++++++++++++++++++++++++-------
 2 files changed, 55 insertions(+), 16 deletions(-)

diff --git a/core/rawdb/freezer_table.go b/core/rawdb/freezer_table.go
index 9cff4216c..e11a27430 100644
--- a/core/rawdb/freezer_table.go
+++ b/core/rawdb/freezer_table.go
@@ -232,8 +232,8 @@ func (t *freezerTable) repair() error {
 	t.index.ReadAt(buffer, 0)
 	firstIndex.unmarshalBinary(buffer)
 
-	t.tailId = firstIndex.offset
-	t.itemOffset = firstIndex.filenum
+	t.tailId = firstIndex.filenum
+	t.itemOffset = firstIndex.offset
 
 	t.index.ReadAt(buffer, offsetsSize-indexEntrySize)
 	lastIndex.unmarshalBinary(buffer)
@@ -519,16 +519,27 @@ func (t *freezerTable) Append(item uint64, blob []byte) error {
 // getBounds returns the indexes for the item
 // returns start, end, filenumber and error
 func (t *freezerTable) getBounds(item uint64) (uint32, uint32, uint32, error) {
-	var startIdx, endIdx indexEntry
 	buffer := make([]byte, indexEntrySize)
-	if _, err := t.index.ReadAt(buffer, int64(item*indexEntrySize)); err != nil {
-		return 0, 0, 0, err
-	}
-	startIdx.unmarshalBinary(buffer)
+	var startIdx, endIdx indexEntry
+	// Read second index
 	if _, err := t.index.ReadAt(buffer, int64((item+1)*indexEntrySize)); err != nil {
 		return 0, 0, 0, err
 	}
 	endIdx.unmarshalBinary(buffer)
+	// Read first index (unless it's the very first item)
+	if item != 0 {
+		if _, err := t.index.ReadAt(buffer, int64(item*indexEntrySize)); err != nil {
+			return 0, 0, 0, err
+		}
+		startIdx.unmarshalBinary(buffer)
+	} else {
+		// Special case if we're reading the first item in the freezer. We assume that
+		// the first item always start from zero(regarding the deletion, we
+		// only support deletion by files, so that the assumption is held).
+		// This means we can use the first item metadata to carry information about
+		// the 'global' offset, for the deletion-case
+		return 0, endIdx.offset, endIdx.filenum, nil
+	}
 	if startIdx.filenum != endIdx.filenum {
 		// If a piece of data 'crosses' a data-file,
 		// it's actually in one piece on the second data-file.
diff --git a/core/rawdb/freezer_table_test.go b/core/rawdb/freezer_table_test.go
index dd1ff744a..b22c58e13 100644
--- a/core/rawdb/freezer_table_test.go
+++ b/core/rawdb/freezer_table_test.go
@@ -552,8 +552,8 @@ func TestOffset(t *testing.T) {
 		tailId := uint32(2)     // First file is 2
 		itemOffset := uint32(4) // We have removed four items
 		zeroIndex := indexEntry{
-			offset:  tailId,
-			filenum: itemOffset,
+			filenum: tailId,
+			offset:  itemOffset,
 		}
 		buf := zeroIndex.marshallBinary()
 		// Overwrite index zero
@@ -567,39 +567,67 @@ func TestOffset(t *testing.T) {
 
 	}
 	// Now open again
-	{
+	checkPresent := func(numDeleted uint64) {
 		f, err := newCustomTable(os.TempDir(), fname, rm, wm, sg, 40, true)
 		if err != nil {
 			t.Fatal(err)
 		}
 		f.printIndex()
 		// It should allow writing item 6
-		f.Append(6, getChunk(20, 0x99))
+		f.Append(numDeleted+2, getChunk(20, 0x99))
 
 		// It should be fine to fetch 4,5,6
-		if got, err := f.Retrieve(4); err != nil {
+		if got, err := f.Retrieve(numDeleted); err != nil {
 			t.Fatal(err)
 		} else if exp := getChunk(20, 0xbb); !bytes.Equal(got, exp) {
 			t.Fatalf("expected %x got %x", exp, got)
 		}
-		if got, err := f.Retrieve(5); err != nil {
+		if got, err := f.Retrieve(numDeleted + 1); err != nil {
 			t.Fatal(err)
 		} else if exp := getChunk(20, 0xaa); !bytes.Equal(got, exp) {
 			t.Fatalf("expected %x got %x", exp, got)
 		}
-		if got, err := f.Retrieve(6); err != nil {
+		if got, err := f.Retrieve(numDeleted + 2); err != nil {
 			t.Fatal(err)
 		} else if exp := getChunk(20, 0x99); !bytes.Equal(got, exp) {
 			t.Fatalf("expected %x got %x", exp, got)
 		}
 
 		// It should error at 0, 1,2,3
-		for i := 0; i < 4; i++ {
-			if _, err := f.Retrieve(uint64(i)); err == nil {
+		for i := numDeleted - 1; i > numDeleted-10; i-- {
+			if _, err := f.Retrieve(i); err == nil {
 				t.Fatal("expected err")
 			}
 		}
 	}
+	checkPresent(4)
+	// Now, let's pretend we have deleted 1M items
+	{
+		// Read the index file
+		p := filepath.Join(os.TempDir(), fmt.Sprintf("%v.ridx", fname))
+		indexFile, err := os.OpenFile(p, os.O_RDWR, 0644)
+		if err != nil {
+			t.Fatal(err)
+		}
+		indexBuf := make([]byte, 3*indexEntrySize)
+		indexFile.Read(indexBuf)
+
+		// Update the index file, so that we store
+		// [ file = 2, offset = 1M ] at index zero
+
+		tailId := uint32(2)           // First file is 2
+		itemOffset := uint32(1000000) // We have removed 1M items
+		zeroIndex := indexEntry{
+			offset:  itemOffset,
+			filenum: tailId,
+		}
+		buf := zeroIndex.marshallBinary()
+		// Overwrite index zero
+		copy(indexBuf, buf)
+		indexFile.WriteAt(indexBuf, 0)
+		indexFile.Close()
+	}
+	checkPresent(1000000)
 }
 
 // TODO (?)
-- 
GitLab