From 22edf449bc4f8d9ed0c604195d31d795c3d72fec Mon Sep 17 00:00:00 2001 From: devopsbo3 <69951731+devopsbo3@users.noreply.github.com> Date: Fri, 10 Nov 2023 12:27:53 -0600 Subject: [PATCH] Revert "ethdb/pebble: prevent shutdown-panic (#27238)" This reverts commit 98ab75d69507fff7df367810166dd9abe39d0d35. --- ethdb/dbtest/testsuite.go | 26 -------------------- ethdb/memorydb/memorydb.go | 3 --- ethdb/pebble/pebble.go | 49 ++++++++------------------------------ 3 files changed, 10 insertions(+), 68 deletions(-) diff --git a/ethdb/dbtest/testsuite.go b/ethdb/dbtest/testsuite.go index d82dbd6992..e455215cb0 100644 --- a/ethdb/dbtest/testsuite.go +++ b/ethdb/dbtest/testsuite.go @@ -376,32 +376,6 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) { } } }) - - t.Run("OperatonsAfterClose", func(t *testing.T) { - db := New() - db.Put([]byte("key"), []byte("value")) - db.Close() - if _, err := db.Get([]byte("key")); err == nil { - t.Fatalf("expected error on Get after Close") - } - if _, err := db.Has([]byte("key")); err == nil { - t.Fatalf("expected error on Get after Close") - } - if err := db.Put([]byte("key2"), []byte("value2")); err == nil { - t.Fatalf("expected error on Put after Close") - } - if err := db.Delete([]byte("key")); err == nil { - t.Fatalf("expected error on Delete after Close") - } - - b := db.NewBatch() - if err := b.Put([]byte("batchkey"), []byte("batchval")); err != nil { - t.Fatalf("expected no error on batch.Put after Close, got %v", err) - } - if err := b.Write(); err == nil { - t.Fatalf("expected error on batch.Write after Close") - } - }) } // BenchDatabaseSuite runs a suite of benchmarks against a KeyValueStore database diff --git a/ethdb/memorydb/memorydb.go b/ethdb/memorydb/memorydb.go index f9f74322b5..6ca7ce7d91 100644 --- a/ethdb/memorydb/memorydb.go +++ b/ethdb/memorydb/memorydb.go @@ -244,9 +244,6 @@ func (b *batch) Write() error { b.db.lock.Lock() defer b.db.lock.Unlock() - if b.db.db == nil { - return errMemorydbClosed - } for _, keyvalue := range b.writes { if keyvalue.delete { delete(b.db.db, string(keyvalue.key)) diff --git a/ethdb/pebble/pebble.go b/ethdb/pebble/pebble.go index e538084777..5be4c1af73 100644 --- a/ethdb/pebble/pebble.go +++ b/ethdb/pebble/pebble.go @@ -70,9 +70,8 @@ type Database struct { seekCompGauge metrics.Gauge // Gauge for tracking the number of table compaction caused by read opt manualMemAllocGauge metrics.Gauge // Gauge for tracking amount of non-managed memory currently allocated - quitLock sync.RWMutex // Mutex protecting the quit channel and the closed flag + quitLock sync.Mutex // Mutex protecting the quit channel access quitChan chan chan error // Quit channel to stop the metrics collection before closing the database - closed bool // keep track of whether we're Closed log log.Logger // Contextual logger tracking the database path @@ -222,29 +221,23 @@ func New(file string, cache int, handles int, namespace string, readonly bool) ( func (d *Database) Close() error { d.quitLock.Lock() defer d.quitLock.Unlock() + // Allow double closing, simplifies things - if d.closed { + if d.quitChan == nil { return nil } - d.closed = true - if d.quitChan != nil { - errc := make(chan error) - d.quitChan <- errc - if err := <-errc; err != nil { - d.log.Error("Metrics collection failed", "err", err) - } - d.quitChan = nil + errc := make(chan error) + d.quitChan <- errc + if err := <-errc; err != nil { + d.log.Error("Metrics collection failed", "err", err) } + d.quitChan = nil + return d.db.Close() } // Has retrieves if a key is present in the key-value store. func (d *Database) Has(key []byte) (bool, error) { - d.quitLock.RLock() - defer d.quitLock.RUnlock() - if d.closed { - return false, pebble.ErrClosed - } _, closer, err := d.db.Get(key) if err == pebble.ErrNotFound { return false, nil @@ -257,11 +250,6 @@ func (d *Database) Has(key []byte) (bool, error) { // Get retrieves the given key if it's present in the key-value store. func (d *Database) Get(key []byte) ([]byte, error) { - d.quitLock.RLock() - defer d.quitLock.RUnlock() - if d.closed { - return nil, pebble.ErrClosed - } dat, closer, err := d.db.Get(key) if err != nil { return nil, err @@ -274,21 +262,11 @@ func (d *Database) Get(key []byte) ([]byte, error) { // Put inserts the given value into the key-value store. func (d *Database) Put(key []byte, value []byte) error { - d.quitLock.RLock() - defer d.quitLock.RUnlock() - if d.closed { - return pebble.ErrClosed - } return d.db.Set(key, value, pebble.NoSync) } // Delete removes the key from the key-value store. func (d *Database) Delete(key []byte) error { - d.quitLock.RLock() - defer d.quitLock.RUnlock() - if d.closed { - return pebble.ErrClosed - } return d.db.Delete(key, nil) } @@ -296,8 +274,7 @@ func (d *Database) Delete(key []byte) error { // database until a final write is called. func (d *Database) NewBatch() ethdb.Batch { return &batch{ - b: d.db.NewBatch(), - db: d, + b: d.db.NewBatch(), } } @@ -504,7 +481,6 @@ func (d *Database) meter(refresh time.Duration) { // when Write is called. A batch cannot be used concurrently. type batch struct { b *pebble.Batch - db *Database size int } @@ -529,11 +505,6 @@ func (b *batch) ValueSize() int { // Write flushes any accumulated data to disk. func (b *batch) Write() error { - b.db.quitLock.RLock() - defer b.db.quitLock.RUnlock() - if b.db.closed { - return pebble.ErrClosed - } return b.b.Commit(pebble.NoSync) }