From 76f1bb8b834198892dee8fa62d5d90b25ddb8e78 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 9 Nov 2022 08:06:02 +0100 Subject: [PATCH 1/4] core/state: replace fastcache code cache with gc-friendly structure (#26092) This PR replaces fastcache with a pretty simple LRU which does not require explicit closing. (cherry picked from commit 5fded040372784985265f83f33f15cb6a51bebdb) --- common/lru/blob_lru.go | 88 ++++++++++++++++++++++++++ common/lru/blob_lru_test.go | 122 ++++++++++++++++++++++++++++++++++++ core/state/database.go | 14 ++--- 3 files changed, 217 insertions(+), 7 deletions(-) create mode 100644 common/lru/blob_lru.go create mode 100644 common/lru/blob_lru_test.go diff --git a/common/lru/blob_lru.go b/common/lru/blob_lru.go new file mode 100644 index 000000000000..3138f422ccfa --- /dev/null +++ b/common/lru/blob_lru.go @@ -0,0 +1,88 @@ +// Copyright 2022 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package lru + +import ( + "math" + "sync" + + "github.com/ethereum/go-ethereum/common" + "github.com/hashicorp/golang-lru/simplelru" +) + +// SizeConstrainedLRU is a wrapper around simplelru.LRU. The simplelru.LRU is capable +// of item-count constraints, but is not capable of enforcing a byte-size constraint, +// hence this wrapper. +// OBS: This cache assumes that items are content-addressed: keys are unique per content. +// In other words: two Add(..) with the same key K, will always have the same value V. +type SizeConstrainedLRU struct { + size uint64 + maxSize uint64 + lru *simplelru.LRU + lock sync.RWMutex +} + +// NewSizeConstrainedLRU creates a new SizeConstrainedLRU. +func NewSizeConstrainedLRU(max uint64) *SizeConstrainedLRU { + lru, err := simplelru.NewLRU(math.MaxInt, nil) + if err != nil { + panic(err) + } + return &SizeConstrainedLRU{ + size: 0, + maxSize: max, + lru: lru, + } +} + +// Add adds a value to the cache. Returns true if an eviction occurred. +// OBS: This cache assumes that items are content-addressed: keys are unique per content. +// In other words: two Add(..) with the same key K, will always have the same value V. +// OBS: The value is _not_ copied on Add, so the caller must not modify it afterwards. +func (c *SizeConstrainedLRU) Add(key common.Hash, value []byte) (evicted bool) { + c.lock.Lock() + defer c.lock.Unlock() + + // Unless it is already present, might need to evict something. + // OBS: If it is present, we still call Add internally to bump the recentness. + if !c.lru.Contains(key) { + targetSize := c.size + uint64(len(value)) + for targetSize > c.maxSize { + evicted = true + _, v, ok := c.lru.RemoveOldest() + if !ok { + // list is now empty. Break + break + } + targetSize -= uint64(len(v.([]byte))) + } + c.size = targetSize + } + c.lru.Add(key, value) + return evicted +} + +// Get looks up a key's value from the cache. +func (c *SizeConstrainedLRU) Get(key common.Hash) []byte { + c.lock.RLock() + defer c.lock.RUnlock() + + if v, ok := c.lru.Get(key); ok { + return v.([]byte) + } + return nil +} diff --git a/common/lru/blob_lru_test.go b/common/lru/blob_lru_test.go new file mode 100644 index 000000000000..4b5e69340222 --- /dev/null +++ b/common/lru/blob_lru_test.go @@ -0,0 +1,122 @@ +// Copyright 2022 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package lru + +import ( + "encoding/binary" + "fmt" + "testing" + + "github.com/ethereum/go-ethereum/common" +) + +func mkHash(i int) common.Hash { + h := make([]byte, 32) + binary.LittleEndian.PutUint64(h, uint64(i)) + return common.BytesToHash(h) +} + +func TestBlobLru(t *testing.T) { + lru := NewSizeConstrainedLRU(100) + var want uint64 + // Add 11 items of 10 byte each. First item should be swapped out + for i := 0; i < 11; i++ { + k := mkHash(i) + v := fmt.Sprintf("value-%04d", i) + lru.Add(k, []byte(v)) + want += uint64(len(v)) + if want > 100 { + want = 100 + } + if have := lru.size; have != want { + t.Fatalf("size wrong, have %d want %d", have, want) + } + } + // Zero:th should be evicted + { + k := mkHash(0) + if val := lru.Get(k); val != nil { + t.Fatalf("should be evicted: %v", k) + } + } + // Elems 1-11 should be present + for i := 1; i < 11; i++ { + k := mkHash(i) + want := fmt.Sprintf("value-%04d", i) + have := lru.Get(k) + if have == nil { + t.Fatalf("missing key %v", k) + } + if string(have) != want { + t.Fatalf("wrong value, have %v want %v", have, want) + } + } +} + +// TestBlobLruOverflow tests what happens when inserting an element exceeding +// the max size +func TestBlobLruOverflow(t *testing.T) { + lru := NewSizeConstrainedLRU(100) + // Add 10 items of 10 byte each, filling the cache + for i := 0; i < 10; i++ { + k := mkHash(i) + v := fmt.Sprintf("value-%04d", i) + lru.Add(k, []byte(v)) + } + // Add one single large elem. We expect it to swap out all entries. + { + k := mkHash(1337) + v := make([]byte, 200) + lru.Add(k, v) + } + // Elems 0-9 should be missing + for i := 1; i < 10; i++ { + k := mkHash(i) + if val := lru.Get(k); val != nil { + t.Fatalf("should be evicted: %v", k) + } + } + // The size should be accurate + if have, want := lru.size, uint64(200); have != want { + t.Fatalf("size wrong, have %d want %d", have, want) + } + // Adding one small item should swap out the large one + { + i := 0 + k := mkHash(i) + v := fmt.Sprintf("value-%04d", i) + lru.Add(k, []byte(v)) + if have, want := lru.size, uint64(10); have != want { + t.Fatalf("size wrong, have %d want %d", have, want) + } + } +} + +// TestBlobLruSameItem tests what happens when inserting the same k/v multiple times. +func TestBlobLruSameItem(t *testing.T) { + lru := NewSizeConstrainedLRU(100) + // Add one 10 byte-item 10 times + k := mkHash(0) + v := fmt.Sprintf("value-%04d", 0) + for i := 0; i < 10; i++ { + lru.Add(k, []byte(v)) + } + // The size should be accurate + if have, want := lru.size, uint64(10); have != want { + t.Fatalf("size wrong, have %d want %d", have, want) + } +} diff --git a/core/state/database.go b/core/state/database.go index 96b6bcfe6551..28de45261db4 100644 --- a/core/state/database.go +++ b/core/state/database.go @@ -20,8 +20,8 @@ import ( "errors" "fmt" - "github.com/VictoriaMetrics/fastcache" "github.com/ethereum/go-ethereum/common" + lru2 "github.com/ethereum/go-ethereum/common/lru" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/ethdb" @@ -131,14 +131,14 @@ func NewDatabaseWithConfig(db ethdb.Database, config *trie.Config) Database { return &cachingDB{ db: trie.NewDatabaseWithConfig(db, config), codeSizeCache: csc, - codeCache: fastcache.New(codeCacheSize), + codeCache: lru2.NewSizeConstrainedLRU(codeCacheSize), } } type cachingDB struct { db *trie.Database codeSizeCache *lru.Cache - codeCache *fastcache.Cache + codeCache *lru2.SizeConstrainedLRU } // OpenTrie opens the main account trie at a specific root hash. @@ -171,12 +171,12 @@ func (db *cachingDB) CopyTrie(t Trie) Trie { // ContractCode retrieves a particular contract's code. func (db *cachingDB) ContractCode(addrHash, codeHash common.Hash) ([]byte, error) { - if code := db.codeCache.Get(nil, codeHash.Bytes()); len(code) > 0 { + if code := db.codeCache.Get(codeHash); len(code) > 0 { return code, nil } code := rawdb.ReadCode(db.db.DiskDB(), codeHash) if len(code) > 0 { - db.codeCache.Set(codeHash.Bytes(), code) + db.codeCache.Add(codeHash, code) db.codeSizeCache.Add(codeHash, len(code)) return code, nil } @@ -187,12 +187,12 @@ func (db *cachingDB) ContractCode(addrHash, codeHash common.Hash) ([]byte, error // code can't be found in the cache, then check the existence with **new** // db scheme. func (db *cachingDB) ContractCodeWithPrefix(addrHash, codeHash common.Hash) ([]byte, error) { - if code := db.codeCache.Get(nil, codeHash.Bytes()); len(code) > 0 { + if code := db.codeCache.Get(codeHash); len(code) > 0 { return code, nil } code := rawdb.ReadCodeWithPrefix(db.db.DiskDB(), codeHash) if len(code) > 0 { - db.codeCache.Set(codeHash.Bytes(), code) + db.codeCache.Add(codeHash, code) db.codeSizeCache.Add(codeHash, len(code)) return code, nil } From 354b6a797acbab480844f2a6d948dd52a8854b51 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 11 Nov 2022 19:48:36 +0100 Subject: [PATCH 2/4] common/lru: fix race in lru (#26164) This fixes a problem in the SizeConstrainedLRU. The SCLRU uses an underlying simple lru which is not thread safe. During the Get operation, the recentness of the accessed item is updated, so it is not a pure read-operation. Therefore, the mutex we need is a full mutex, not RLock. This PR changes the mutex to be a regular Mutex, instead of RWMutex, so a reviewer can at a glance see that all affected locations are fixed. (cherry picked from commit 8334b5f51a16b72cf2b3ebdc9e131a442c5289d7) --- common/lru/blob_lru.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lru/blob_lru.go b/common/lru/blob_lru.go index 3138f422ccfa..b24684256c88 100644 --- a/common/lru/blob_lru.go +++ b/common/lru/blob_lru.go @@ -33,7 +33,7 @@ type SizeConstrainedLRU struct { size uint64 maxSize uint64 lru *simplelru.LRU - lock sync.RWMutex + lock sync.Mutex } // NewSizeConstrainedLRU creates a new SizeConstrainedLRU. @@ -78,8 +78,8 @@ func (c *SizeConstrainedLRU) Add(key common.Hash, value []byte) (evicted bool) { // Get looks up a key's value from the cache. func (c *SizeConstrainedLRU) Get(key common.Hash) []byte { - c.lock.RLock() - defer c.lock.RUnlock() + c.lock.Lock() + defer c.lock.Unlock() if v, ok := c.lru.Get(key); ok { return v.([]byte) From 283fb4ec13f7a140311c1bf77c7b7406dd2436de Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Fri, 25 Nov 2022 16:10:31 +0800 Subject: [PATCH 3/4] core/rawdb: fix freezer validation (#26251) * core/rawdb: fix freezer validation * core/rawdb: address comment (cherry picked from commit add1bff13fff722acbf1bd06b57245361ff82359) --- core/rawdb/freezer.go | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/core/rawdb/freezer.go b/core/rawdb/freezer.go index 6dea98c3d3c4..03093e7b1dd7 100644 --- a/core/rawdb/freezer.go +++ b/core/rawdb/freezer.go @@ -318,30 +318,35 @@ func (f *Freezer) Sync() error { return nil } -// validate checks that every table has the same length. +// validate checks that every table has the same boundary. // Used instead of `repair` in readonly mode. func (f *Freezer) validate() error { if len(f.tables) == 0 { return nil } var ( - length uint64 - name string + head uint64 + tail uint64 + name string ) - // Hack to get length of any table + // Hack to get boundary of any table for kind, table := range f.tables { - length = atomic.LoadUint64(&table.items) + head = atomic.LoadUint64(&table.items) + tail = atomic.LoadUint64(&table.itemHidden) name = kind break } - // Now check every table against that length + // Now check every table against those boundaries. 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", kind, name, items, length) + if head != atomic.LoadUint64(&table.items) { + return fmt.Errorf("freezer tables %s and %s have differing head: %d != %d", kind, name, atomic.LoadUint64(&table.items), head) + } + if tail != atomic.LoadUint64(&table.itemHidden) { + return fmt.Errorf("freezer tables %s and %s have differing tail: %d != %d", kind, name, atomic.LoadUint64(&table.itemHidden), tail) } } - atomic.StoreUint64(&f.frozen, length) + atomic.StoreUint64(&f.frozen, head) + atomic.StoreUint64(&f.tail, tail) return nil } From 31ed80298dc683ae8e11eba0af9961165cc05890 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 24 Nov 2022 10:50:28 +0100 Subject: [PATCH 4/4] core/rawdb: improve freezerTable.Sync (#26245) While investigating #22374, I noticed that the Sync operation of the freezer does not take the table lock. It also doesn't call sync for all files if there is an error with one of them. I doubt this will fix anything, but didn't want to drop the fix on the floor either. (cherry picked from commit 193f350eb911c9d8a93577d619986e97f490b700) --- core/rawdb/freezer_table.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/core/rawdb/freezer_table.go b/core/rawdb/freezer_table.go index 3fe691cf6d2a..e9998fa463c6 100644 --- a/core/rawdb/freezer_table.go +++ b/core/rawdb/freezer_table.go @@ -875,13 +875,20 @@ func (t *freezerTable) advanceHead() error { // Sync pushes any pending data from memory out to disk. This is an expensive // operation, so use it with care. func (t *freezerTable) Sync() error { - if err := t.index.Sync(); err != nil { - return err - } - if err := t.meta.Sync(); err != nil { - return err + t.lock.Lock() + defer t.lock.Unlock() + + var err error + trackError := func(e error) { + if e != nil && err == nil { + err = e + } } - return t.head.Sync() + + trackError(t.index.Sync()) + trackError(t.meta.Sync()) + trackError(t.head.Sync()) + return err } func (t *freezerTable) dumpIndexStdout(start, stop int64) {