From be24bf64122f5a75e7db598ca0d9d0571933c06d Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 16 Feb 2022 16:48:47 +0300 Subject: [PATCH] storage: drop Put and Delete from Store interface It's only changed with PutChangeSet, single KV operations are handled by MemCachedStore. --- pkg/core/storage/boltdb_store.go | 17 ------- pkg/core/storage/leveldb_store.go | 10 ---- pkg/core/storage/memcached_store.go | 19 +++++++ pkg/core/storage/memcached_store_test.go | 56 ++++++++++++++++----- pkg/core/storage/memory_store.go | 29 +---------- pkg/core/storage/memory_store_test.go | 7 ++- pkg/core/storage/store.go | 7 ++- pkg/core/storage/storeandbatch_test.go | 63 +++++------------------- 8 files changed, 82 insertions(+), 126 deletions(-) diff --git a/pkg/core/storage/boltdb_store.go b/pkg/core/storage/boltdb_store.go index f98eaf78a..4045998b9 100644 --- a/pkg/core/storage/boltdb_store.go +++ b/pkg/core/storage/boltdb_store.go @@ -47,15 +47,6 @@ func NewBoltDBStore(cfg BoltDBOptions) (*BoltDBStore, error) { return &BoltDBStore{db: db}, nil } -// Put implements the Store interface. -func (s *BoltDBStore) Put(key, value []byte) error { - return s.db.Update(func(tx *bbolt.Tx) error { - b := tx.Bucket(Bucket) - err := b.Put(key, value) - return err - }) -} - // Get implements the Store interface. func (s *BoltDBStore) Get(key []byte) (val []byte, err error) { err = s.db.View(func(tx *bbolt.Tx) error { @@ -73,14 +64,6 @@ func (s *BoltDBStore) Get(key []byte) (val []byte, err error) { return } -// Delete implements the Store interface. -func (s *BoltDBStore) Delete(key []byte) error { - return s.db.Update(func(tx *bbolt.Tx) error { - b := tx.Bucket(Bucket) - return b.Delete(key) - }) -} - // PutChangeSet implements the Store interface. func (s *BoltDBStore) PutChangeSet(puts map[string][]byte, stores map[string][]byte) error { var err error diff --git a/pkg/core/storage/leveldb_store.go b/pkg/core/storage/leveldb_store.go index 2c35d3374..24aa74891 100644 --- a/pkg/core/storage/leveldb_store.go +++ b/pkg/core/storage/leveldb_store.go @@ -36,11 +36,6 @@ func NewLevelDBStore(cfg LevelDBOptions) (*LevelDBStore, error) { }, nil } -// Put implements the Store interface. -func (s *LevelDBStore) Put(key, value []byte) error { - return s.db.Put(key, value, nil) -} - // Get implements the Store interface. func (s *LevelDBStore) Get(key []byte) ([]byte, error) { value, err := s.db.Get(key, nil) @@ -50,11 +45,6 @@ func (s *LevelDBStore) Get(key []byte) ([]byte, error) { return value, err } -// Delete implements the Store interface. -func (s *LevelDBStore) Delete(key []byte) error { - return s.db.Delete(key, nil) -} - // PutChangeSet implements the Store interface. func (s *LevelDBStore) PutChangeSet(puts map[string][]byte, stores map[string][]byte) error { tx, err := s.db.OpenTransaction() diff --git a/pkg/core/storage/memcached_store.go b/pkg/core/storage/memcached_store.go index 3429ecf0f..7602a113d 100644 --- a/pkg/core/storage/memcached_store.go +++ b/pkg/core/storage/memcached_store.go @@ -65,6 +65,25 @@ func (s *MemCachedStore) Get(key []byte) ([]byte, error) { return s.ps.Get(key) } +// Put puts new KV pair into the store. Never returns an error. +func (s *MemCachedStore) Put(key, value []byte) error { + newKey := string(key) + vcopy := slice.Copy(value) + s.mut.Lock() + put(s.chooseMap(key), newKey, vcopy) + s.mut.Unlock() + return nil +} + +// Delete drops KV pair from the store. Never returns an error. +func (s *MemCachedStore) Delete(key []byte) error { + newKey := string(key) + s.mut.Lock() + put(s.chooseMap(key), newKey, nil) + s.mut.Unlock() + return nil +} + // GetBatch returns currently accumulated changeset. func (s *MemCachedStore) GetBatch() *MemBatch { s.mut.RLock() diff --git a/pkg/core/storage/memcached_store_test.go b/pkg/core/storage/memcached_store_test.go index 8bbad9f72..0b7565e6b 100644 --- a/pkg/core/storage/memcached_store_test.go +++ b/pkg/core/storage/memcached_store_test.go @@ -12,6 +12,34 @@ import ( "github.com/stretchr/testify/require" ) +func TestMemCachedPutGetDelete(t *testing.T) { + ps := NewMemoryStore() + s := NewMemCachedStore(ps) + key := []byte("foo") + value := []byte("bar") + + require.NoError(t, s.Put(key, value)) + + result, err := s.Get(key) + assert.Nil(t, err) + require.Equal(t, value, result) + + err = s.Delete(key) + assert.Nil(t, err) + + _, err = s.Get(key) + assert.NotNil(t, err) + assert.Equal(t, err, ErrKeyNotFound) + + // Double delete. + err = s.Delete(key) + assert.Nil(t, err) + + // Nonexistent. + key = []byte("sparse") + assert.NoError(t, s.Delete(key)) +} + func testMemCachedStorePersist(t *testing.T, ps Store) { // cached Store ts := NewMemCachedStore(ps) @@ -123,7 +151,7 @@ func TestCachedGetFromPersistent(t *testing.T) { ps := NewMemoryStore() ts := NewMemCachedStore(ps) - assert.NoError(t, ps.Put(key, value)) + assert.NoError(t, ps.PutChangeSet(map[string][]byte{string(key): value}, nil)) val, err := ts.Get(key) assert.Nil(t, err) assert.Equal(t, value, val) @@ -156,14 +184,14 @@ func TestCachedSeek(t *testing.T) { ts = NewMemCachedStore(ps) ) for _, v := range lowerKVs { - require.NoError(t, ps.Put(v.Key, v.Value)) + require.NoError(t, ps.PutChangeSet(map[string][]byte{string(v.Key): v.Value}, nil)) } for _, v := range deletedKVs { - require.NoError(t, ps.Put(v.Key, v.Value)) + require.NoError(t, ps.PutChangeSet(map[string][]byte{string(v.Key): v.Value}, nil)) require.NoError(t, ts.Delete(v.Key)) } for _, v := range updatedKVs { - require.NoError(t, ps.Put(v.Key, []byte("stub"))) + require.NoError(t, ps.PutChangeSet(map[string][]byte{string(v.Key): v.Value}, nil)) require.NoError(t, ts.Put(v.Key, v.Value)) } foundKVs := make(map[string][]byte) @@ -199,36 +227,38 @@ func benchmarkCachedSeek(t *testing.B, ps Store, psElementsCount, tsElementsCoun ) for i := 0; i < psElementsCount; i++ { // lower KVs with matching prefix that should be found - require.NoError(t, ps.Put(append(lowerPrefixGood, random.Bytes(10)...), []byte("value"))) + require.NoError(t, ts.Put(append(lowerPrefixGood, random.Bytes(10)...), []byte("value"))) // lower KVs with non-matching prefix that shouldn't be found - require.NoError(t, ps.Put(append(lowerPrefixBad, random.Bytes(10)...), []byte("value"))) + require.NoError(t, ts.Put(append(lowerPrefixBad, random.Bytes(10)...), []byte("value"))) // deleted KVs with matching prefix that shouldn't be found key := append(deletedPrefixGood, random.Bytes(10)...) - require.NoError(t, ps.Put(key, []byte("deleted"))) + require.NoError(t, ts.Put(key, []byte("deleted"))) if i < tsElementsCount { require.NoError(t, ts.Delete(key)) } // deleted KVs with non-matching prefix that shouldn't be found key = append(deletedPrefixBad, random.Bytes(10)...) - require.NoError(t, ps.Put(key, []byte("deleted"))) + require.NoError(t, ts.Put(key, []byte("deleted"))) if i < tsElementsCount { require.NoError(t, ts.Delete(key)) } // updated KVs with matching prefix that should be found key = append(updatedPrefixGood, random.Bytes(10)...) - require.NoError(t, ps.Put(key, []byte("stub"))) + require.NoError(t, ts.Put(key, []byte("stub"))) if i < tsElementsCount { require.NoError(t, ts.Put(key, []byte("updated"))) } // updated KVs with non-matching prefix that shouldn't be found key = append(updatedPrefixBad, random.Bytes(10)...) - require.NoError(t, ps.Put(key, []byte("stub"))) + require.NoError(t, ts.Put(key, []byte("stub"))) if i < tsElementsCount { require.NoError(t, ts.Put(key, []byte("updated"))) } } + _, err := ts.PersistSync() + require.NoError(t, err) t.ReportAllocs() t.ResetTimer() @@ -347,14 +377,14 @@ func TestCachedSeekSorting(t *testing.T) { ts = NewMemCachedStore(ps) ) for _, v := range lowerKVs { - require.NoError(t, ps.Put(v.Key, v.Value)) + require.NoError(t, ps.PutChangeSet(map[string][]byte{string(v.Key): v.Value}, nil)) } for _, v := range deletedKVs { - require.NoError(t, ps.Put(v.Key, v.Value)) + require.NoError(t, ps.PutChangeSet(map[string][]byte{string(v.Key): v.Value}, nil)) require.NoError(t, ts.Delete(v.Key)) } for _, v := range updatedKVs { - require.NoError(t, ps.Put(v.Key, []byte("stub"))) + require.NoError(t, ps.PutChangeSet(map[string][]byte{string(v.Key): v.Value}, nil)) require.NoError(t, ts.Put(v.Key, v.Value)) } var foundKVs []KeyValue diff --git a/pkg/core/storage/memory_store.go b/pkg/core/storage/memory_store.go index 64c47a287..62551928d 100644 --- a/pkg/core/storage/memory_store.go +++ b/pkg/core/storage/memory_store.go @@ -5,8 +5,6 @@ import ( "sort" "strings" "sync" - - "github.com/nspcc-dev/neo-go/pkg/util/slice" ) // MemoryStore is an in-memory implementation of a Store, mainly @@ -51,31 +49,6 @@ func put(m map[string][]byte, key string, value []byte) { m[key] = value } -// Put implements the Store interface. Never returns an error. -func (s *MemoryStore) Put(key, value []byte) error { - newKey := string(key) - vcopy := slice.Copy(value) - s.mut.Lock() - put(s.chooseMap(key), newKey, vcopy) - s.mut.Unlock() - return nil -} - -// drop deletes a key-value pair from the store, it's supposed to be called -// with mutex locked. -func drop(m map[string][]byte, key string) { - m[key] = nil -} - -// Delete implements Store interface. Never returns an error. -func (s *MemoryStore) Delete(key []byte) error { - newKey := string(key) - s.mut.Lock() - drop(s.chooseMap(key), newKey) - s.mut.Unlock() - return nil -} - // PutChangeSet implements the Store interface. Never returns an error. func (s *MemoryStore) PutChangeSet(puts map[string][]byte, stores map[string][]byte) error { s.mut.Lock() @@ -103,7 +76,7 @@ func (s *MemoryStore) SeekGC(rng SeekRange, keep func(k, v []byte) bool) error { // sensitive to the order of KV pairs. s.seek(rng, func(k, v []byte) bool { if !keep(k, v) { - drop(s.chooseMap(k), string(k)) + delete(s.chooseMap(k), string(k)) } return true }) diff --git a/pkg/core/storage/memory_store_test.go b/pkg/core/storage/memory_store_test.go index a8ae360be..eb3770c6a 100644 --- a/pkg/core/storage/memory_store_test.go +++ b/pkg/core/storage/memory_store_test.go @@ -20,10 +20,13 @@ func BenchmarkMemorySeek(t *testing.B) { searchPrefix = []byte{1} badPrefix = []byte{2} ) + ts := NewMemCachedStore(ms) for i := 0; i < count; i++ { - require.NoError(t, ms.Put(append(searchPrefix, random.Bytes(10)...), random.Bytes(10))) - require.NoError(t, ms.Put(append(badPrefix, random.Bytes(10)...), random.Bytes(10))) + require.NoError(t, ts.Put(append(searchPrefix, random.Bytes(10)...), random.Bytes(10))) + require.NoError(t, ts.Put(append(badPrefix, random.Bytes(10)...), random.Bytes(10))) } + _, err := ts.PersistSync() + require.NoError(t, err) t.ReportAllocs() t.ResetTimer() diff --git a/pkg/core/storage/store.go b/pkg/core/storage/store.go index 0776466cc..8d4bfad9d 100644 --- a/pkg/core/storage/store.go +++ b/pkg/core/storage/store.go @@ -82,12 +82,11 @@ type SeekRange struct { var ErrKeyNotFound = errors.New("key not found") type ( - // Store is anything that can persist and retrieve the blockchain. - // information. + // Store is the underlying KV backend for the blockchain data, it's + // not intended to be used directly, you wrap it with some memory cache + // layer most of the time. Store interface { - Delete(k []byte) error Get([]byte) ([]byte, error) - Put(k, v []byte) error // PutChangeSet allows to push prepared changeset to the Store. PutChangeSet(puts map[string][]byte, stor map[string][]byte) error // Seek can guarantee that provided key (k) and value (v) are the only valid until the next call to f. diff --git a/pkg/core/storage/storeandbatch_test.go b/pkg/core/storage/storeandbatch_test.go index 67da01cf1..0bc8917c5 100644 --- a/pkg/core/storage/storeandbatch_test.go +++ b/pkg/core/storage/storeandbatch_test.go @@ -19,17 +19,6 @@ type dbSetup struct { type dbTestFunction func(*testing.T, Store) -func testStorePutAndGet(t *testing.T, s Store) { - key := []byte("foo") - value := []byte("bar") - - require.NoError(t, s.Put(key, value)) - - result, err := s.Get(key) - assert.Nil(t, err) - require.Equal(t, value, result) -} - func testStoreGetNonExistent(t *testing.T, s Store) { key := []byte("sparse") @@ -37,7 +26,7 @@ func testStoreGetNonExistent(t *testing.T, s Store) { assert.Equal(t, err, ErrKeyNotFound) } -func testStoreSeek(t *testing.T, s Store) { +func pushSeekDataSet(t *testing.T, s Store) []KeyValue { // Use the same set of kvs to test Seek with different prefix/start values. kvs := []KeyValue{ {[]byte("10"), []byte("bar")}, @@ -48,10 +37,17 @@ func testStoreSeek(t *testing.T, s Store) { {[]byte("30"), []byte("bare")}, {[]byte("31"), []byte("barf")}, } + up := NewMemCachedStore(s) for _, v := range kvs { - require.NoError(t, s.Put(v.Key, v.Value)) + require.NoError(t, up.Put(v.Key, v.Value)) } + _, err := up.PersistSync() + require.NoError(t, err) + return kvs +} +func testStoreSeek(t *testing.T, s Store) { + kvs := pushSeekDataSet(t, s) check := func(t *testing.T, goodprefix, start []byte, goodkvs []KeyValue, backwards bool, cont func(k, v []byte) bool) { // Seek result expected to be sorted in an ascending (for forwards seeking) or descending (for backwards seeking) way. cmpFunc := func(i, j int) bool { @@ -209,43 +205,8 @@ func testStoreSeek(t *testing.T, s Store) { }) } -func testStoreDeleteNonExistent(t *testing.T, s Store) { - key := []byte("sparse") - - assert.NoError(t, s.Delete(key)) -} - -func testStorePutAndDelete(t *testing.T, s Store) { - key := []byte("foo") - value := []byte("bar") - - require.NoError(t, s.Put(key, value)) - - err := s.Delete(key) - assert.Nil(t, err) - - _, err = s.Get(key) - assert.NotNil(t, err) - assert.Equal(t, err, ErrKeyNotFound) - - // Double delete. - err = s.Delete(key) - assert.Nil(t, err) -} - func testStoreSeekGC(t *testing.T, s Store) { - kvs := []KeyValue{ - {[]byte("10"), []byte("bar")}, - {[]byte("11"), []byte("bara")}, - {[]byte("20"), []byte("barb")}, - {[]byte("21"), []byte("barc")}, - {[]byte("22"), []byte("bard")}, - {[]byte("30"), []byte("bare")}, - {[]byte("31"), []byte("barf")}, - } - for _, v := range kvs { - require.NoError(t, s.Put(v.Key, v.Value)) - } + kvs := pushSeekDataSet(t, s) err := s.SeekGC(SeekRange{Prefix: []byte("1")}, func(k, v []byte) bool { return true }) @@ -275,9 +236,7 @@ func TestAllDBs(t *testing.T) { {"MemCached", newMemCachedStoreForTesting}, {"Memory", newMemoryStoreForTesting}, } - var tests = []dbTestFunction{testStorePutAndGet, - testStoreGetNonExistent, testStoreSeek, - testStoreDeleteNonExistent, testStorePutAndDelete, + var tests = []dbTestFunction{testStoreGetNonExistent, testStoreSeek, testStoreSeekGC} for _, db := range DBs { for _, test := range tests {