From 63c9ce9620161593baa69e879bd2571d8a969d88 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Wed, 15 Dec 2021 15:18:27 +0330 Subject: [PATCH 01/16] freezer: add readonly flag to table --- core/rawdb/freezer.go | 2 +- core/rawdb/freezer_table.go | 8 +++-- core/rawdb/freezer_table_test.go | 56 ++++++++++++++++---------------- 3 files changed, 34 insertions(+), 32 deletions(-) diff --git a/core/rawdb/freezer.go b/core/rawdb/freezer.go index e19c202adc84..c7d44ed39f5a 100644 --- a/core/rawdb/freezer.go +++ b/core/rawdb/freezer.go @@ -133,7 +133,7 @@ func newFreezer(datadir string, namespace string, readonly bool, maxTableSize ui // Create the tables. for name, disableSnappy := range tables { - table, err := newTable(datadir, name, readMeter, writeMeter, sizeGauge, maxTableSize, disableSnappy) + table, err := newTable(datadir, name, readMeter, writeMeter, sizeGauge, maxTableSize, disableSnappy, readonly) if err != nil { for _, table := range freezer.tables { table.Close() diff --git a/core/rawdb/freezer_table.go b/core/rawdb/freezer_table.go index 22405cf9b4f8..9b0453634094 100644 --- a/core/rawdb/freezer_table.go +++ b/core/rawdb/freezer_table.go @@ -94,7 +94,8 @@ type freezerTable struct { // so take advantage of that (https://golang.org/pkg/sync/atomic/#pkg-note-BUG). items uint64 // Number of items stored in the table (including items removed from tail) - noCompression bool // if true, disables snappy compression. Note: does not work retroactively + noCompression bool // if true, disables snappy compression. Note: does not work retroactively + readonly bool maxFileSize uint32 // Max file size for data-files name string path string @@ -120,7 +121,7 @@ type freezerTable struct { // NewFreezerTable opens the given path as a freezer table. func NewFreezerTable(path, name string, disableSnappy bool) (*freezerTable, error) { - return newTable(path, name, metrics.NilMeter{}, metrics.NilMeter{}, metrics.NilGauge{}, freezerTableSize, disableSnappy) + return newTable(path, name, metrics.NilMeter{}, metrics.NilMeter{}, metrics.NilGauge{}, freezerTableSize, disableSnappy, false) } // openFreezerFileForAppend opens a freezer table file and seeks to the end @@ -164,7 +165,7 @@ func truncateFreezerFile(file *os.File, size int64) error { // newTable opens a freezer table, creating the data and index files if they are // non existent. Both files are truncated to the shortest common length to ensure // they don't go out of sync. -func newTable(path string, name string, readMeter metrics.Meter, writeMeter metrics.Meter, sizeGauge metrics.Gauge, maxFilesize uint32, noCompression bool) (*freezerTable, error) { +func newTable(path string, name string, readMeter metrics.Meter, writeMeter metrics.Meter, sizeGauge metrics.Gauge, maxFilesize uint32, noCompression, readonly bool) (*freezerTable, error) { // Ensure the containing directory exists and open the indexEntry file if err := os.MkdirAll(path, 0755); err != nil { return nil, err @@ -192,6 +193,7 @@ func newTable(path string, name string, readMeter metrics.Meter, writeMeter metr path: path, logger: log.New("database", path, "table", name), noCompression: noCompression, + readonly: readonly, maxFileSize: maxFilesize, } if err := tab.repair(); err != nil { diff --git a/core/rawdb/freezer_table_test.go b/core/rawdb/freezer_table_test.go index 803809b5207f..2e7235d5873e 100644 --- a/core/rawdb/freezer_table_test.go +++ b/core/rawdb/freezer_table_test.go @@ -40,7 +40,7 @@ func TestFreezerBasics(t *testing.T) { // set cutoff at 50 bytes f, err := newTable(os.TempDir(), fmt.Sprintf("unittest-%d", rand.Uint64()), - metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true) + metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, false) if err != nil { t.Fatal(err) } @@ -85,7 +85,7 @@ func TestFreezerBasicsClosing(t *testing.T) { f *freezerTable err error ) - f, err = newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err = newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -99,7 +99,7 @@ func TestFreezerBasicsClosing(t *testing.T) { require.NoError(t, batch.commit()) f.Close() - f, err = newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err = newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -116,7 +116,7 @@ func TestFreezerBasicsClosing(t *testing.T) { t.Fatalf("test %d, got \n%x != \n%x", y, got, exp) } f.Close() - f, err = newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err = newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -131,7 +131,7 @@ func TestFreezerRepairDanglingHead(t *testing.T) { // Fill table { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -160,7 +160,7 @@ func TestFreezerRepairDanglingHead(t *testing.T) { // Now open it again { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -183,7 +183,7 @@ func TestFreezerRepairDanglingHeadLarge(t *testing.T) { // Fill a table and close it { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -209,7 +209,7 @@ func TestFreezerRepairDanglingHeadLarge(t *testing.T) { // Now open it again { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -232,7 +232,7 @@ func TestFreezerRepairDanglingHeadLarge(t *testing.T) { // And if we open it, we should now be able to read all of them (new values) { - f, _ := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, _ := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) for y := 1; y < 255; y++ { exp := getChunk(15, ^y) got, err := f.Retrieve(uint64(y)) @@ -254,7 +254,7 @@ func TestSnappyDetection(t *testing.T) { // Open with snappy { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -265,7 +265,7 @@ func TestSnappyDetection(t *testing.T) { // Open without snappy { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, false) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, false, false) if err != nil { t.Fatal(err) } @@ -277,7 +277,7 @@ func TestSnappyDetection(t *testing.T) { // Open with snappy { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -309,7 +309,7 @@ func TestFreezerRepairDanglingIndex(t *testing.T) { // Fill a table and close it { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -345,7 +345,7 @@ func TestFreezerRepairDanglingIndex(t *testing.T) { // 45, 45, 15 // with 3+3+1 items { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -366,7 +366,7 @@ func TestFreezerTruncate(t *testing.T) { // Fill table { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -382,7 +382,7 @@ func TestFreezerTruncate(t *testing.T) { // Reopen, truncate { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -407,7 +407,7 @@ func TestFreezerRepairFirstFile(t *testing.T) { // Fill table { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -440,7 +440,7 @@ func TestFreezerRepairFirstFile(t *testing.T) { // Reopen { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -475,7 +475,7 @@ func TestFreezerReadAndTruncate(t *testing.T) { // Fill table { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -491,7 +491,7 @@ func TestFreezerReadAndTruncate(t *testing.T) { // Reopen and read all files { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -523,7 +523,7 @@ func TestFreezerOffset(t *testing.T) { // Fill table { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 40, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 40, true, false) if err != nil { t.Fatal(err) } @@ -584,7 +584,7 @@ func TestFreezerOffset(t *testing.T) { // Now open again { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 40, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 40, true, false) if err != nil { t.Fatal(err) } @@ -638,7 +638,7 @@ func TestFreezerOffset(t *testing.T) { // Check that existing items have been moved to index 1M. { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 40, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 40, true, false) if err != nil { t.Fatal(err) } @@ -726,7 +726,7 @@ func TestSequentialRead(t *testing.T) { rm, wm, sg := metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge() fname := fmt.Sprintf("batchread-%d", rand.Uint64()) { // Fill table - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -736,7 +736,7 @@ func TestSequentialRead(t *testing.T) { f.Close() } { // Open it, iterate, verify iteration - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 50, true, false) if err != nil { t.Fatal(err) } @@ -757,7 +757,7 @@ func TestSequentialRead(t *testing.T) { } { // Open it, iterate, verify byte limit. The byte limit is less than item // size, so each lookup should only return one item - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 40, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 40, true, false) if err != nil { t.Fatal(err) } @@ -786,7 +786,7 @@ func TestSequentialReadByteLimit(t *testing.T) { rm, wm, sg := metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge() fname := fmt.Sprintf("batchread-2-%d", rand.Uint64()) { // Fill table - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 100, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 100, true, false) if err != nil { t.Fatal(err) } @@ -808,7 +808,7 @@ func TestSequentialReadByteLimit(t *testing.T) { {100, 109, 10}, } { { - f, err := newTable(os.TempDir(), fname, rm, wm, sg, 100, true) + f, err := newTable(os.TempDir(), fname, rm, wm, sg, 100, true, false) if err != nil { t.Fatal(err) } From 169e10092c1682feaad0629a13de780d3cf39a7e Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Wed, 15 Dec 2021 19:00:45 +0330 Subject: [PATCH 02/16] freezer: enforce readonly in table repair --- core/rawdb/freezer_table.go | 41 ++++++++++++++----- core/rawdb/freezer_table_test.go | 69 ++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 9 deletions(-) diff --git a/core/rawdb/freezer_table.go b/core/rawdb/freezer_table.go index 9b0453634094..dd120839eeef 100644 --- a/core/rawdb/freezer_table.go +++ b/core/rawdb/freezer_table.go @@ -178,7 +178,14 @@ func newTable(path string, name string, readMeter metrics.Meter, writeMeter metr // Compressed idx idxName = fmt.Sprintf("%s.cidx", name) } - offsets, err := openFreezerFileForAppend(filepath.Join(path, idxName)) + var err error + var offsets *os.File + if readonly { + // Will fail if table doesn't exist + offsets, err = openFreezerFileForReadOnly(filepath.Join(path, idxName)) + } else { + offsets, err = openFreezerFileForAppend(filepath.Join(path, idxName)) + } if err != nil { return nil, err } @@ -229,6 +236,9 @@ func (t *freezerTable) repair() error { } // Ensure the index is a multiple of indexEntrySize bytes if overflow := stat.Size() % indexEntrySize; overflow != 0 { + if t.readonly { + return fmt.Errorf("table index has invalid length") + } truncateFreezerFile(t.index, stat.Size()-overflow) // New file can't trigger this path } // Retrieve the file sizes and prepare for truncation @@ -254,7 +264,11 @@ func (t *freezerTable) repair() error { t.index.ReadAt(buffer, offsetsSize-indexEntrySize) lastIndex.unmarshalBinary(buffer) - t.head, err = t.openFile(lastIndex.filenum, openFreezerFileForAppend) + if t.readonly { + t.head, err = t.openFile(lastIndex.filenum, openFreezerFileForReadOnly) + } else { + t.head, err = t.openFile(lastIndex.filenum, openFreezerFileForAppend) + } if err != nil { return err } @@ -267,6 +281,9 @@ func (t *freezerTable) repair() error { contentExp = int64(lastIndex.offset) for contentExp != contentSize { + if t.readonly { + return fmt.Errorf("head file has unexpected size. %d != %d", contentSize, contentExp) + } // Truncate the head file to the last offset pointer if contentExp < contentSize { t.logger.Warn("Truncating dangling head", "indexed", common.StorageSize(contentExp), "stored", common.StorageSize(contentSize)) @@ -304,11 +321,13 @@ func (t *freezerTable) repair() error { } } // Ensure all reparation changes have been written to disk - if err := t.index.Sync(); err != nil { - return err - } - if err := t.head.Sync(); err != nil { - return err + if !t.readonly { + if err := t.index.Sync(); err != nil { + return err + } + if err := t.head.Sync(); err != nil { + return err + } } // Update the item and byte counters and return t.items = uint64(t.itemOffset) + uint64(offsetsSize/indexEntrySize-1) // last indexEntry points to the end of the data file @@ -336,8 +355,12 @@ func (t *freezerTable) preopen() (err error) { return err } } - // Open head in read/write - t.head, err = t.openFile(t.headId, openFreezerFileForAppend) + if t.readonly { + t.head, err = t.openFile(t.headId, openFreezerFileForReadOnly) + } else { + // Open head in read/write + t.head, err = t.openFile(t.headId, openFreezerFileForAppend) + } return err } diff --git a/core/rawdb/freezer_table_test.go b/core/rawdb/freezer_table_test.go index 2e7235d5873e..3f9b41ae2114 100644 --- a/core/rawdb/freezer_table_test.go +++ b/core/rawdb/freezer_table_test.go @@ -829,3 +829,72 @@ func TestSequentialReadByteLimit(t *testing.T) { } } } + +func TestFreezerReadonlyBasics(t *testing.T) { + // Case 1: Check it fails on non-existent file. + _, err := newTable(os.TempDir(), + fmt.Sprintf("readonlytest-%d", rand.Uint64()), + metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, true) + if err == nil { + t.Fatal("readonly table instantiation should fail for non-existent table") + } + + // Case 2: Check that it fails on invalid index length. + tmpdir := os.TempDir() + fname := fmt.Sprintf("readonlytest-%d", rand.Uint64()) + idxFile, err := openFreezerFileForAppend(filepath.Join(tmpdir, fmt.Sprintf("%s.ridx", fname))) + if err != nil { + t.Errorf("Failed to open index file: %v\n", err) + } + // size should not be a multiple of indexEntrySize. + idxFile.Write(make([]byte, 17)) + idxFile.Close() + _, err = newTable(tmpdir, fname, + metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, true) + if err == nil { + t.Errorf("readonly table instantiation should fail for invalid index size") + } + + // Case 3: Open table non-readonly table to write some data. + // Then corrupt the head file and make sure opening the table + // again in readonly triggers an error. + fname = fmt.Sprintf("readonlytest-%d", rand.Uint64()) + f, err := newTable(os.TempDir(), fname, + metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, false) + writeChunks(t, f, 8, 32) + // Corrupt table file + if _, err := f.head.Write([]byte{1, 1}); err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } + _, err = newTable(os.TempDir(), fname, + metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, true) + if err == nil { + t.Errorf("readonly table instantiation should fail for corrupt table file") + } + + // Case 4: Write some data to a table and later re-open it as readonly. + // Should be successful. + fname = fmt.Sprintf("readonlytest-%d", rand.Uint64()) + f, err = newTable(os.TempDir(), fname, + metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, false) + writeChunks(t, f, 32, 128) + if err := f.Close(); err != nil { + t.Fatal(err) + } + f, err = newTable(os.TempDir(), fname, + metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, true) + if err != nil { + t.Error(err) + } + v, err := f.Retrieve(10) + if err != nil { + t.Fatal(err) + } + exp := getChunk(128, 10) + if !bytes.Equal(v, exp) { + t.Errorf("retrieved value is incorrect") + } +} From 41f42bfff54023f406916b2c55e47c4522af49fa Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Thu, 16 Dec 2021 11:23:05 +0330 Subject: [PATCH 03/16] freezer: enforce readonly in newFreezer --- core/rawdb/freezer.go | 37 +++++++++++++++++++++++++++++++++++-- core/rawdb/freezer_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/core/rawdb/freezer.go b/core/rawdb/freezer.go index c7d44ed39f5a..0622784e6bd4 100644 --- a/core/rawdb/freezer.go +++ b/core/rawdb/freezer.go @@ -144,8 +144,15 @@ func newFreezer(datadir string, namespace string, readonly bool, maxTableSize ui freezer.tables[name] = table } - // Truncate all tables to common length. - if err := freezer.repair(); err != nil { + if freezer.readonly { + // In readonly mode only validate, don't truncate. + // validate also sets `freezer.frozen`. + err = freezer.validate() + } else { + // Truncate all tables to common length. + err = freezer.repair() + } + if err != nil { for _, table := range freezer.tables { table.Close() } @@ -308,6 +315,32 @@ func (f *freezer) Sync() error { return nil } +// validate checks that every table has the same length. +// Used instead of `repair` in readonly mode. +func (f *freezer) validate() error { + if len(f.tables) == 0 { + return nil + } + var length uint64 + var name string + // Hack to get length of any table + for k, table := range f.tables { + length = atomic.LoadUint64(&table.items) + name = k + break + } + log.Info("validate", "name", name, "length", length) + // Now check every table against that length + for k, table := range f.tables { + items := atomic.LoadUint64(&table.items) + if length != items { + return fmt.Errorf("freezer tables %s and %s have differing lengths: %d != %d", k, name, items, length) + } + } + atomic.StoreUint64(&f.frozen, length) + return nil +} + // repair truncates all data tables to the same length. func (f *freezer) repair() error { min := uint64(math.MaxUint64) diff --git a/core/rawdb/freezer_test.go b/core/rawdb/freezer_test.go index fa84f803068b..d5c3749e5d21 100644 --- a/core/rawdb/freezer_test.go +++ b/core/rawdb/freezer_test.go @@ -253,6 +253,44 @@ func TestFreezerConcurrentModifyTruncate(t *testing.T) { } } +func TestFreezerReadonlyValidate(t *testing.T) { + tables := map[string]bool{"a": true, "b": true} + dir, err := ioutil.TempDir("", "freezer") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + // Open non-readonly freezer and fill individual tables + // with different amount of data. + f, err := newFreezer(dir, "", false, 2049, tables) + if err != nil { + t.Fatal("can't open freezer", err) + } + var item = make([]byte, 1024) + aBatch := f.tables["a"].newBatch() + require.NoError(t, aBatch.AppendRaw(0, item)) + require.NoError(t, aBatch.AppendRaw(1, item)) + require.NoError(t, aBatch.AppendRaw(2, item)) + require.NoError(t, aBatch.commit()) + bBatch := f.tables["b"].newBatch() + require.NoError(t, bBatch.AppendRaw(0, item)) + require.NoError(t, bBatch.commit()) + if f.tables["a"].items != 3 { + t.Fatalf("unexpected number of items in table") + } + if f.tables["b"].items != 1 { + t.Fatalf("unexpected number of items in table") + } + require.NoError(t, f.Close()) + + // Re-openening as readonly should fail when validating + // table lengths. + f, err = newFreezer(dir, "", true, 2049, tables) + if err == nil { + t.Fatal("readonly freezer should fail with differing table lengths") + } +} + func newFreezerForTesting(t *testing.T, tables map[string]bool) (*freezer, string) { t.Helper() From a5360a97ebee348c4e37e833ebe021e2a9d1021d Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Thu, 16 Dec 2021 12:50:01 +0330 Subject: [PATCH 04/16] minor fix --- core/rawdb/freezer_table_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/rawdb/freezer_table_test.go b/core/rawdb/freezer_table_test.go index 3f9b41ae2114..88b201536bcd 100644 --- a/core/rawdb/freezer_table_test.go +++ b/core/rawdb/freezer_table_test.go @@ -861,6 +861,9 @@ func TestFreezerReadonlyBasics(t *testing.T) { fname = fmt.Sprintf("readonlytest-%d", rand.Uint64()) f, err := newTable(os.TempDir(), fname, metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, false) + if err != nil { + t.Fatalf("failed to instantiate table: %v", err) + } writeChunks(t, f, 8, 32) // Corrupt table file if _, err := f.head.Write([]byte{1, 1}); err != nil { @@ -880,6 +883,9 @@ func TestFreezerReadonlyBasics(t *testing.T) { fname = fmt.Sprintf("readonlytest-%d", rand.Uint64()) f, err = newTable(os.TempDir(), fname, metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, false) + if err != nil { + t.Fatalf("failed to instantiate table: %v\n", err) + } writeChunks(t, f, 32, 128) if err := f.Close(); err != nil { t.Fatal(err) From 8f42a64920ff0d2bafa2af1a8e055d7864d18279 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Thu, 16 Dec 2021 18:32:58 +0330 Subject: [PATCH 05/16] minor --- core/rawdb/freezer_table.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/core/rawdb/freezer_table.go b/core/rawdb/freezer_table.go index dd120839eeef..c290164f7b2e 100644 --- a/core/rawdb/freezer_table.go +++ b/core/rawdb/freezer_table.go @@ -321,13 +321,11 @@ func (t *freezerTable) repair() error { } } // Ensure all reparation changes have been written to disk - if !t.readonly { - if err := t.index.Sync(); err != nil { - return err - } - if err := t.head.Sync(); err != nil { - return err - } + if err := t.index.Sync(); err != nil { + return err + } + if err := t.head.Sync(); err != nil { + return err } // Update the item and byte counters and return t.items = uint64(t.itemOffset) + uint64(offsetsSize/indexEntrySize-1) // last indexEntry points to the end of the data file From 114a81e0d6b2a18eea5cc784afcf3af4b05aa6ac Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 10 Jan 2022 09:21:17 +0100 Subject: [PATCH 06/16] core/rawdb: test that writing during readonly fails --- core/rawdb/freezer_table_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/core/rawdb/freezer_table_test.go b/core/rawdb/freezer_table_test.go index 88b201536bcd..6f1885583aad 100644 --- a/core/rawdb/freezer_table_test.go +++ b/core/rawdb/freezer_table_test.go @@ -903,4 +903,13 @@ func TestFreezerReadonlyBasics(t *testing.T) { if !bytes.Equal(v, exp) { t.Errorf("retrieved value is incorrect") } + // Now write some data. This should fail either during AppendRaw or Commit + batch := f.newBatch() + writeErr := batch.AppendRaw(32, make([]byte, 1)) + if writeErr == nil { + writeErr = batch.commit() + } + if writeErr == nil { + t.Fatalf("Writing to readonly table should fail") + } } From 208c7a9d45be27c1adc32b701bed5d08e9668c41 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Mon, 10 Jan 2022 15:38:56 +0100 Subject: [PATCH 07/16] rm unused log --- core/rawdb/freezer.go | 1 - 1 file changed, 1 deletion(-) diff --git a/core/rawdb/freezer.go b/core/rawdb/freezer.go index 0622784e6bd4..9f854801ff12 100644 --- a/core/rawdb/freezer.go +++ b/core/rawdb/freezer.go @@ -329,7 +329,6 @@ func (f *freezer) validate() error { name = k break } - log.Info("validate", "name", name, "length", length) // Now check every table against that length for k, table := range f.tables { items := atomic.LoadUint64(&table.items) From 2ddb5ec4ba7534bf6edbdfec158ea99a2eed5036 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Tue, 11 Jan 2022 10:09:57 +0100 Subject: [PATCH 08/16] check readonly on batch append --- core/rawdb/freezer_batch.go | 10 ++++++++++ core/rawdb/freezer_table_test.go | 11 +++++------ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/core/rawdb/freezer_batch.go b/core/rawdb/freezer_batch.go index 762fa8f25f19..563d8442c95d 100644 --- a/core/rawdb/freezer_batch.go +++ b/core/rawdb/freezer_batch.go @@ -115,6 +115,11 @@ func (batch *freezerTableBatch) reset() { // precautionary parameter to ensure data correctness, but the table will reject already // existing data. func (batch *freezerTableBatch) Append(item uint64, data interface{}) error { + // Sanity check. In principle this method shouldn't be called at all + // in readonly mode. + if batch.t.readonly { + return fmt.Errorf("permission error: table in readonly mode") + } if item != batch.curItem { return fmt.Errorf("%w: have %d want %d", errOutOrderInsertion, item, batch.curItem) } @@ -135,6 +140,11 @@ func (batch *freezerTableBatch) Append(item uint64, data interface{}) error { // precautionary parameter to ensure data correctness, but the table will reject already // existing data. func (batch *freezerTableBatch) AppendRaw(item uint64, blob []byte) error { + // Sanity check. In principle this method shouldn't be called at all + // in readonly mode. + if batch.t.readonly { + return fmt.Errorf("permission error: table in readonly mode") + } if item != batch.curItem { return fmt.Errorf("%w: have %d want %d", errOutOrderInsertion, item, batch.curItem) } diff --git a/core/rawdb/freezer_table_test.go b/core/rawdb/freezer_table_test.go index 6f1885583aad..925b33884ba8 100644 --- a/core/rawdb/freezer_table_test.go +++ b/core/rawdb/freezer_table_test.go @@ -903,13 +903,12 @@ func TestFreezerReadonlyBasics(t *testing.T) { if !bytes.Equal(v, exp) { t.Errorf("retrieved value is incorrect") } - // Now write some data. This should fail either during AppendRaw or Commit + // Now write some data. Append/AppendRaw should fail. batch := f.newBatch() - writeErr := batch.AppendRaw(32, make([]byte, 1)) - if writeErr == nil { - writeErr = batch.commit() + if err := batch.AppendRaw(32, make([]byte, 1)); err == nil { + t.Errorf("Writing to readonly table should fail") } - if writeErr == nil { - t.Fatalf("Writing to readonly table should fail") + if err := batch.Append(0, make([]byte, 1)); err == nil { + t.Errorf("Writing to readonly table should fail") } } From d04392adda9b1256778ca7834fa7cdcee013aa44 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Tue, 11 Jan 2022 10:11:58 +0100 Subject: [PATCH 09/16] minor --- core/rawdb/freezer_table_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/rawdb/freezer_table_test.go b/core/rawdb/freezer_table_test.go index 925b33884ba8..68ef8568d971 100644 --- a/core/rawdb/freezer_table_test.go +++ b/core/rawdb/freezer_table_test.go @@ -893,7 +893,7 @@ func TestFreezerReadonlyBasics(t *testing.T) { f, err = newTable(os.TempDir(), fname, metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, true) if err != nil { - t.Error(err) + t.Fatal(err) } v, err := f.Retrieve(10) if err != nil { @@ -903,6 +903,7 @@ func TestFreezerReadonlyBasics(t *testing.T) { if !bytes.Equal(v, exp) { t.Errorf("retrieved value is incorrect") } + // Now write some data. Append/AppendRaw should fail. batch := f.newBatch() if err := batch.AppendRaw(32, make([]byte, 1)); err == nil { From 163dcbcd27ec2441d26342ef109591b2ca35de8e Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Tue, 11 Jan 2022 12:10:28 +0100 Subject: [PATCH 10/16] Revert "check readonly on batch append" This reverts commit 2ddb5ec4ba7534bf6edbdfec158ea99a2eed5036. --- core/rawdb/freezer_batch.go | 10 ---------- core/rawdb/freezer_table_test.go | 11 ++++++----- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/core/rawdb/freezer_batch.go b/core/rawdb/freezer_batch.go index 563d8442c95d..762fa8f25f19 100644 --- a/core/rawdb/freezer_batch.go +++ b/core/rawdb/freezer_batch.go @@ -115,11 +115,6 @@ func (batch *freezerTableBatch) reset() { // precautionary parameter to ensure data correctness, but the table will reject already // existing data. func (batch *freezerTableBatch) Append(item uint64, data interface{}) error { - // Sanity check. In principle this method shouldn't be called at all - // in readonly mode. - if batch.t.readonly { - return fmt.Errorf("permission error: table in readonly mode") - } if item != batch.curItem { return fmt.Errorf("%w: have %d want %d", errOutOrderInsertion, item, batch.curItem) } @@ -140,11 +135,6 @@ func (batch *freezerTableBatch) Append(item uint64, data interface{}) error { // precautionary parameter to ensure data correctness, but the table will reject already // existing data. func (batch *freezerTableBatch) AppendRaw(item uint64, blob []byte) error { - // Sanity check. In principle this method shouldn't be called at all - // in readonly mode. - if batch.t.readonly { - return fmt.Errorf("permission error: table in readonly mode") - } if item != batch.curItem { return fmt.Errorf("%w: have %d want %d", errOutOrderInsertion, item, batch.curItem) } diff --git a/core/rawdb/freezer_table_test.go b/core/rawdb/freezer_table_test.go index 68ef8568d971..b53732d5b5f1 100644 --- a/core/rawdb/freezer_table_test.go +++ b/core/rawdb/freezer_table_test.go @@ -904,12 +904,13 @@ func TestFreezerReadonlyBasics(t *testing.T) { t.Errorf("retrieved value is incorrect") } - // Now write some data. Append/AppendRaw should fail. + // Now write some data. This should fail either during AppendRaw or Commit batch := f.newBatch() - if err := batch.AppendRaw(32, make([]byte, 1)); err == nil { - t.Errorf("Writing to readonly table should fail") + writeErr := batch.AppendRaw(32, make([]byte, 1)) + if writeErr == nil { + writeErr = batch.commit() } - if err := batch.Append(0, make([]byte, 1)); err == nil { - t.Errorf("Writing to readonly table should fail") + if writeErr == nil { + t.Fatalf("Writing to readonly table should fail") } } From e0e70f098a25a76b2e627ff366d4d7ec82d6b636 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Tue, 11 Jan 2022 12:38:33 +0100 Subject: [PATCH 11/16] review fixes --- cmd/geth/dbcmd.go | 2 +- core/rawdb/freezer.go | 10 ++++++---- core/rawdb/freezer_table.go | 16 ++++++---------- core/rawdb/freezer_table_test.go | 5 +++-- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/cmd/geth/dbcmd.go b/cmd/geth/dbcmd.go index c2c42276b535..ff4c06de267b 100644 --- a/cmd/geth/dbcmd.go +++ b/cmd/geth/dbcmd.go @@ -539,7 +539,7 @@ func freezerInspect(ctx *cli.Context) error { defer stack.Close() path := filepath.Join(stack.ResolvePath("chaindata"), "ancient") log.Info("Opening freezer", "location", path, "name", kind) - if f, err := rawdb.NewFreezerTable(path, kind, disableSnappy); err != nil { + if f, err := rawdb.NewFreezerTable(path, kind, disableSnappy, true); err != nil { return err } else { f.DumpIndex(start, end) diff --git a/core/rawdb/freezer.go b/core/rawdb/freezer.go index 9f854801ff12..d078c2d531f2 100644 --- a/core/rawdb/freezer.go +++ b/core/rawdb/freezer.go @@ -321,12 +321,14 @@ func (f *freezer) validate() error { if len(f.tables) == 0 { return nil } - var length uint64 - var name string + var ( + length uint64 + name string + ) // Hack to get length of any table - for k, table := range f.tables { + for kind, table := range f.tables { length = atomic.LoadUint64(&table.items) - name = k + name = kind break } // Now check every table against that length diff --git a/core/rawdb/freezer_table.go b/core/rawdb/freezer_table.go index c290164f7b2e..7aeea3970141 100644 --- a/core/rawdb/freezer_table.go +++ b/core/rawdb/freezer_table.go @@ -120,8 +120,8 @@ type freezerTable struct { } // NewFreezerTable opens the given path as a freezer table. -func NewFreezerTable(path, name string, disableSnappy bool) (*freezerTable, error) { - return newTable(path, name, metrics.NilMeter{}, metrics.NilMeter{}, metrics.NilGauge{}, freezerTableSize, disableSnappy, false) +func NewFreezerTable(path, name string, disableSnappy, readonly bool) (*freezerTable, error) { + return newTable(path, name, metrics.NilMeter{}, metrics.NilMeter{}, metrics.NilGauge{}, freezerTableSize, disableSnappy, readonly) } // openFreezerFileForAppend opens a freezer table file and seeks to the end @@ -178,8 +178,10 @@ func newTable(path string, name string, readMeter metrics.Meter, writeMeter metr // Compressed idx idxName = fmt.Sprintf("%s.cidx", name) } - var err error - var offsets *os.File + var ( + err error + offsets *os.File + ) if readonly { // Will fail if table doesn't exist offsets, err = openFreezerFileForReadOnly(filepath.Join(path, idxName)) @@ -236,9 +238,6 @@ func (t *freezerTable) repair() error { } // Ensure the index is a multiple of indexEntrySize bytes if overflow := stat.Size() % indexEntrySize; overflow != 0 { - if t.readonly { - return fmt.Errorf("table index has invalid length") - } truncateFreezerFile(t.index, stat.Size()-overflow) // New file can't trigger this path } // Retrieve the file sizes and prepare for truncation @@ -281,9 +280,6 @@ func (t *freezerTable) repair() error { contentExp = int64(lastIndex.offset) for contentExp != contentSize { - if t.readonly { - return fmt.Errorf("head file has unexpected size. %d != %d", contentSize, contentExp) - } // Truncate the head file to the last offset pointer if contentExp < contentSize { t.logger.Warn("Truncating dangling head", "indexed", common.StorageSize(contentExp), "stored", common.StorageSize(contentSize)) diff --git a/core/rawdb/freezer_table_test.go b/core/rawdb/freezer_table_test.go index b53732d5b5f1..1c7682202c12 100644 --- a/core/rawdb/freezer_table_test.go +++ b/core/rawdb/freezer_table_test.go @@ -830,7 +830,7 @@ func TestSequentialReadByteLimit(t *testing.T) { } } -func TestFreezerReadonlyBasics(t *testing.T) { +func TestFreezerReadonly(t *testing.T) { // Case 1: Check it fails on non-existent file. _, err := newTable(os.TempDir(), fmt.Sprintf("readonlytest-%d", rand.Uint64()), @@ -904,7 +904,8 @@ func TestFreezerReadonlyBasics(t *testing.T) { t.Errorf("retrieved value is incorrect") } - // Now write some data. This should fail either during AppendRaw or Commit + // Case 5: Now write some data via a batch. + // This should fail either during AppendRaw or Commit batch := f.newBatch() writeErr := batch.AppendRaw(32, make([]byte, 1)) if writeErr == nil { From 60dd8435e0cba5657707eb5fab3dc8edecb12082 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Tue, 11 Jan 2022 14:28:00 +0100 Subject: [PATCH 12/16] minor test refactor --- core/rawdb/freezer_table_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/rawdb/freezer_table_test.go b/core/rawdb/freezer_table_test.go index 1c7682202c12..15464e1bd768 100644 --- a/core/rawdb/freezer_table_test.go +++ b/core/rawdb/freezer_table_test.go @@ -831,8 +831,9 @@ func TestSequentialReadByteLimit(t *testing.T) { } func TestFreezerReadonly(t *testing.T) { + tmpdir := os.TempDir() // Case 1: Check it fails on non-existent file. - _, err := newTable(os.TempDir(), + _, err := newTable(tmpdir, fmt.Sprintf("readonlytest-%d", rand.Uint64()), metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, true) if err == nil { @@ -840,7 +841,6 @@ func TestFreezerReadonly(t *testing.T) { } // Case 2: Check that it fails on invalid index length. - tmpdir := os.TempDir() fname := fmt.Sprintf("readonlytest-%d", rand.Uint64()) idxFile, err := openFreezerFileForAppend(filepath.Join(tmpdir, fmt.Sprintf("%s.ridx", fname))) if err != nil { @@ -859,7 +859,7 @@ func TestFreezerReadonly(t *testing.T) { // Then corrupt the head file and make sure opening the table // again in readonly triggers an error. fname = fmt.Sprintf("readonlytest-%d", rand.Uint64()) - f, err := newTable(os.TempDir(), fname, + f, err := newTable(tmpdir, fname, metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, false) if err != nil { t.Fatalf("failed to instantiate table: %v", err) @@ -872,7 +872,7 @@ func TestFreezerReadonly(t *testing.T) { if err := f.Close(); err != nil { t.Fatal(err) } - _, err = newTable(os.TempDir(), fname, + _, err = newTable(tmpdir, fname, metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, true) if err == nil { t.Errorf("readonly table instantiation should fail for corrupt table file") @@ -881,7 +881,7 @@ func TestFreezerReadonly(t *testing.T) { // Case 4: Write some data to a table and later re-open it as readonly. // Should be successful. fname = fmt.Sprintf("readonlytest-%d", rand.Uint64()) - f, err = newTable(os.TempDir(), fname, + f, err = newTable(tmpdir, fname, metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, false) if err != nil { t.Fatalf("failed to instantiate table: %v\n", err) @@ -890,7 +890,7 @@ func TestFreezerReadonly(t *testing.T) { if err := f.Close(); err != nil { t.Fatal(err) } - f, err = newTable(os.TempDir(), fname, + f, err = newTable(tmpdir, fname, metrics.NewMeter(), metrics.NewMeter(), metrics.NewGauge(), 50, true, true) if err != nil { t.Fatal(err) From b1aecc28287b2db897bd3d0139d05867ba6fa3ea Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Tue, 11 Jan 2022 15:07:50 +0100 Subject: [PATCH 13/16] attempt at fixing windows issue --- core/rawdb/freezer_table.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/core/rawdb/freezer_table.go b/core/rawdb/freezer_table.go index 7aeea3970141..4e0b62edc6f5 100644 --- a/core/rawdb/freezer_table.go +++ b/core/rawdb/freezer_table.go @@ -316,12 +316,14 @@ func (t *freezerTable) repair() error { contentExp = int64(lastIndex.offset) } } - // Ensure all reparation changes have been written to disk - if err := t.index.Sync(); err != nil { - return err - } - if err := t.head.Sync(); err != nil { - return err + if !t.readonly { + // Ensure all reparation changes have been written to disk + if err := t.index.Sync(); err != nil { + return err + } + if err := t.head.Sync(); err != nil { + return err + } } // Update the item and byte counters and return t.items = uint64(t.itemOffset) + uint64(offsetsSize/indexEntrySize-1) // last indexEntry points to the end of the data file From 0ad8cf86543154b8cae3a9a1c008caa144019106 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Tue, 11 Jan 2022 15:51:31 +0100 Subject: [PATCH 14/16] add comment re windows sync issue --- core/rawdb/freezer_table.go | 1 + 1 file changed, 1 insertion(+) diff --git a/core/rawdb/freezer_table.go b/core/rawdb/freezer_table.go index 4e0b62edc6f5..7cfba70c5004 100644 --- a/core/rawdb/freezer_table.go +++ b/core/rawdb/freezer_table.go @@ -316,6 +316,7 @@ func (t *freezerTable) repair() error { contentExp = int64(lastIndex.offset) } } + // Sync() fails for read-only files on windows. if !t.readonly { // Ensure all reparation changes have been written to disk if err := t.index.Sync(); err != nil { From 6255ab196c37028ff5c8089016eea27e24ac4ae9 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Tue, 11 Jan 2022 15:59:22 +0100 Subject: [PATCH 15/16] k->kind --- core/rawdb/freezer.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/rawdb/freezer.go b/core/rawdb/freezer.go index d078c2d531f2..88c72625eede 100644 --- a/core/rawdb/freezer.go +++ b/core/rawdb/freezer.go @@ -332,10 +332,10 @@ func (f *freezer) validate() error { break } // Now check every table against that length - for k, table := range f.tables { + for kind, table := range f.tables { items := atomic.LoadUint64(&table.items) if length != items { - return fmt.Errorf("freezer tables %s and %s have differing lengths: %d != %d", k, name, items, length) + return fmt.Errorf("freezer tables %s and %s have differing lengths: %d != %d", kind, name, items, length) } } atomic.StoreUint64(&f.frozen, length) From 803035b720feef3a359aa9592f4467cbcefe2898 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Tue, 11 Jan 2022 20:00:12 +0100 Subject: [PATCH 16/16] open readonly db for genesis check --- cmd/utils/flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index ffff2c92cb84..7a0203a69382 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -1675,7 +1675,7 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) { if ctx.GlobalIsSet(DataDirFlag.Name) { // Check if we have an already initialized chain and fall back to // that if so. Otherwise we need to generate a new genesis spec. - chaindb := MakeChainDatabase(ctx, stack, false) // TODO (MariusVanDerWijden) make this read only + chaindb := MakeChainDatabase(ctx, stack, true) if rawdb.ReadCanonicalHash(chaindb, 0) != (common.Hash{}) { cfg.Genesis = nil // fallback to db content }