Skip to content

Commit 3811802

Browse files
StephenButtolphDan Laine
andauthored
Change merkledb caches to be size based (#1947)
Co-authored-by: Dan Laine <daniel.laine@avalabs.org>
1 parent 0944233 commit 3811802

File tree

8 files changed

+114
-59
lines changed

8 files changed

+114
-59
lines changed

x/merkledb/cache.go

Lines changed: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,40 +4,40 @@
44
package merkledb
55

66
import (
7+
"errors"
78
"sync"
89

910
"github.com/ava-labs/avalanchego/utils/linkedhashmap"
1011
"github.com/ava-labs/avalanchego/utils/wrappers"
1112
)
1213

14+
var errEmptyCacheTooLarge = errors.New("cache is empty yet still too large")
15+
1316
// A cache that calls [onEviction] on the evicted element.
1417
type onEvictCache[K comparable, V any] struct {
15-
lock sync.RWMutex
16-
maxSize int
17-
fifo linkedhashmap.LinkedHashmap[K, V]
18+
lock sync.RWMutex
19+
maxSize int
20+
currentSize int
21+
fifo linkedhashmap.LinkedHashmap[K, V]
22+
size func(K, V) int
1823
// Must not call any method that grabs [c.lock]
1924
// because this would cause a deadlock.
2025
onEviction func(K, V) error
2126
}
2227

23-
func newOnEvictCache[K comparable, V any](maxSize int, onEviction func(K, V) error) onEvictCache[K, V] {
28+
func newOnEvictCache[K comparable, V any](
29+
maxSize int,
30+
size func(K, V) int,
31+
onEviction func(K, V) error,
32+
) onEvictCache[K, V] {
2433
return onEvictCache[K, V]{
2534
maxSize: maxSize,
2635
fifo: linkedhashmap.New[K, V](),
36+
size: size,
2737
onEviction: onEviction,
2838
}
2939
}
3040

31-
// removeOldest returns and removes the oldest element from this cache.
32-
// Assumes [c.lock] is held.
33-
func (c *onEvictCache[K, V]) removeOldest() (K, V, bool) {
34-
k, v, exists := c.fifo.Oldest()
35-
if exists {
36-
c.fifo.Delete(k)
37-
}
38-
return k, v, exists
39-
}
40-
4141
// Get an element from this cache.
4242
func (c *onEvictCache[K, V]) Get(key K) (V, bool) {
4343
c.lock.RLock()
@@ -53,14 +53,14 @@ func (c *onEvictCache[K, V]) Put(key K, value V) error {
5353
c.lock.Lock()
5454
defer c.lock.Unlock()
5555

56+
if oldValue, replaced := c.fifo.Get(key); replaced {
57+
c.currentSize -= c.size(key, oldValue)
58+
}
59+
60+
c.currentSize += c.size(key, value)
5661
c.fifo.Put(key, value) // Mark as MRU
5762

58-
if c.fifo.Len() > c.maxSize {
59-
oldestKey, oldestVal, _ := c.fifo.Oldest()
60-
c.fifo.Delete(oldestKey)
61-
return c.onEviction(oldestKey, oldestVal)
62-
}
63-
return nil
63+
return c.resize(c.maxSize)
6464
}
6565

6666
// Flush removes all elements from the cache.
@@ -74,16 +74,37 @@ func (c *onEvictCache[K, V]) Flush() error {
7474
c.lock.Unlock()
7575
}()
7676

77+
return c.resize(0)
78+
}
79+
80+
// removeOldest returns and removes the oldest element from this cache.
81+
//
82+
// Assumes [c.lock] is held.
83+
func (c *onEvictCache[K, V]) removeOldest() (K, V, bool) {
84+
k, v, exists := c.fifo.Oldest()
85+
if exists {
86+
c.currentSize -= c.size(k, v)
87+
c.fifo.Delete(k)
88+
}
89+
return k, v, exists
90+
}
91+
92+
// resize removes the oldest elements from the cache until the cache is not
93+
// larger than the provided target.
94+
//
95+
// Assumes [c.lock] is held.
96+
func (c *onEvictCache[K, V]) resize(target int) error {
7797
// Note that we can't use [c.fifo]'s iterator because [c.onEviction]
7898
// modifies [c.fifo], which violates the iterator's invariant.
7999
var errs wrappers.Errs
80-
for {
81-
key, value, exists := c.removeOldest()
100+
for c.currentSize > target {
101+
k, v, exists := c.removeOldest()
82102
if !exists {
83-
// The cache is empty.
84-
return errs.Err
103+
// This should really never happen unless the size of an entry
104+
// changed or the target size is negative.
105+
return errEmptyCacheTooLarge
85106
}
86-
87-
errs.Add(c.onEviction(key, value))
107+
errs.Add(c.onEviction(k, v))
88108
}
109+
return errs.Err
89110
}

x/merkledb/cache_test.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,16 @@ func TestNewOnEvictCache(t *testing.T) {
1616
require := require.New(t)
1717

1818
called := false
19+
size := func(int, int) int {
20+
return 1
21+
}
1922
onEviction := func(int, int) error {
2023
called = true
2124
return nil
2225
}
2326
maxSize := 10
2427

25-
cache := newOnEvictCache[int](maxSize, onEviction)
28+
cache := newOnEvictCache(maxSize, size, onEviction)
2629
require.Equal(maxSize, cache.maxSize)
2730
require.NotNil(cache.fifo)
2831
require.Zero(cache.fifo.Len())
@@ -40,14 +43,17 @@ func TestOnEvictCacheNoOnEvictionError(t *testing.T) {
4043

4144
evictedKey := []int{}
4245
evictedValue := []int{}
46+
size := func(int, int) int {
47+
return 1
48+
}
4349
onEviction := func(k, n int) error {
4450
evictedKey = append(evictedKey, k)
4551
evictedValue = append(evictedValue, n)
4652
return nil
4753
}
4854
maxSize := 3
4955

50-
cache := newOnEvictCache[int](maxSize, onEviction)
56+
cache := newOnEvictCache(maxSize, size, onEviction)
5157

5258
// Get non-existent key
5359
_, ok := cache.Get(0)
@@ -162,8 +168,11 @@ func TestOnEvictCacheNoOnEvictionError(t *testing.T) {
162168
// Note this test assumes the cache is FIFO.
163169
func TestOnEvictCacheOnEvictionError(t *testing.T) {
164170
var (
165-
require = require.New(t)
166-
evicted = []int{}
171+
require = require.New(t)
172+
evicted = []int{}
173+
size = func(int, int) int {
174+
return 1
175+
}
167176
onEviction = func(_, n int) error {
168177
// Evicting even keys errors
169178
evicted = append(evicted, n)
@@ -175,7 +184,7 @@ func TestOnEvictCacheOnEvictionError(t *testing.T) {
175184
maxSize = 2
176185
)
177186

178-
cache := newOnEvictCache[int](maxSize, onEviction)
187+
cache := newOnEvictCache(maxSize, size, onEviction)
179188

180189
// Fill the cache
181190
for i := 0; i < maxSize; i++ {

x/merkledb/db.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@ import (
3131
)
3232

3333
const (
34-
DefaultEvictionBatchSize = 100
35-
RootPath = EmptyPath
34+
RootPath = EmptyPath
3635

3736
// TODO: name better
3837
rebuildViewSizeFractionOfCacheSize = 50
@@ -129,13 +128,15 @@ type Config struct {
129128
// If 0 is specified, [runtime.NumCPU] will be used. If -1 is specified,
130129
// no limit will be used.
131130
RootGenConcurrency int
132-
// The number of nodes that are evicted from the cache and written to
133-
// disk at a time.
131+
// The number of bytes to write to disk when intermediate nodes are evicted
132+
// from their cache and written to disk.
134133
EvictionBatchSize int
135134
// The number of changes to the database that we store in memory in order to
136135
// serve change proofs.
137-
HistoryLength int
138-
ValueNodeCacheSize int
136+
HistoryLength int
137+
// The number of bytes to cache nodes with values.
138+
ValueNodeCacheSize int
139+
// The number of bytes to cache nodes without values.
139140
IntermediateNodeCacheSize int
140141
// If [Reg] is nil, metrics are collected locally but not exported through
141142
// Prometheus.
@@ -1209,3 +1210,10 @@ func addPrefixToKey(bufferPool *sync.Pool, prefix []byte, key []byte) []byte {
12091210
copy(prefixedKey[prefixLen:], key)
12101211
return prefixedKey
12111212
}
1213+
1214+
func cacheEntrySize(p path, n *node) int {
1215+
if n == nil {
1216+
return len(p)
1217+
}
1218+
return len(p) + len(n.marshal())
1219+
}

x/merkledb/db_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ func newDefaultConfig() Config {
3838
return Config{
3939
EvictionBatchSize: 10,
4040
HistoryLength: defaultHistoryLength,
41-
ValueNodeCacheSize: 1_000,
42-
IntermediateNodeCacheSize: 1_000,
41+
ValueNodeCacheSize: units.MiB,
42+
IntermediateNodeCacheSize: units.MiB,
4343
Reg: prometheus.NewRegistry(),
4444
Tracer: trace.Noop,
4545
}

x/merkledb/intermediate_node_db.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ func newIntermediateNodeDB(
4545
bufferPool: bufferPool,
4646
evictionBatchSize: evictionBatchSize,
4747
}
48-
result.nodeCache = newOnEvictCache[path](size, result.onEviction)
48+
result.nodeCache = newOnEvictCache(
49+
size,
50+
cacheEntrySize,
51+
result.onEviction,
52+
)
4953
return result
5054
}
5155

@@ -62,8 +66,7 @@ func (db *intermediateNodeDB) onEviction(key path, n *node) error {
6266
// and write them to disk. We write a batch of them, rather than
6367
// just [n], so that we don't immediately evict and write another
6468
// node, because each time this method is called we do a disk write.
65-
// we have already removed the passed n, so the remove count starts at 1
66-
for removedCount := 1; removedCount < db.evictionBatchSize; removedCount++ {
69+
for writeBatch.Size() < db.evictionBatchSize {
6770
key, n, exists := db.nodeCache.removeOldest()
6871
if !exists {
6972
// The cache is empty.

x/merkledb/intermediate_node_db_test.go

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
func TestIntermediateNodeDB(t *testing.T) {
2424
require := require.New(t)
2525

26-
cacheSize := 10
26+
cacheSize := 100
2727
evictionBatchSize := cacheSize
2828
baseDB := memdb.New()
2929
db := newIntermediateNodeDB(
@@ -70,22 +70,33 @@ func TestIntermediateNodeDB(t *testing.T) {
7070
// Assert the key-node pair was deleted
7171
require.Equal(database.ErrNotFound, err)
7272

73-
// Put [cacheSize] elements in the cache
74-
for i := byte(0); i < byte(cacheSize); i++ {
75-
key := newPath([]byte{i})
73+
// Put elements in the cache until it is full.
74+
expectedSize := cacheEntrySize(key, nil)
75+
added := 0
76+
for {
77+
key := newPath([]byte{byte(added)})
7678
node := &node{
7779
dbNode: dbNode{
78-
value: maybe.Some([]byte{i}),
80+
value: maybe.Some([]byte{byte(added)}),
7981
},
8082
}
83+
newExpectedSize := expectedSize + cacheEntrySize(key, node)
84+
if newExpectedSize > cacheSize {
85+
// Don't trigger eviction.
86+
break
87+
}
88+
8189
require.NoError(db.Put(key, node))
90+
expectedSize = newExpectedSize
91+
added++
8292
}
8393

8494
// Assert cache has expected number of elements
85-
require.Equal(cacheSize, db.nodeCache.fifo.Len())
95+
require.Equal(added, db.nodeCache.fifo.Len())
8696

8797
// Put one more element in the cache, which should trigger an eviction
88-
// of all but 1 element
98+
// of all but 2 elements. 2 elements remain rather than 1 element because of
99+
// the added key prefix increasing the size tracked by the batch.
89100
key = newPath([]byte{byte(cacheSize)})
90101
node := &node{
91102
dbNode: dbNode{
@@ -95,11 +106,10 @@ func TestIntermediateNodeDB(t *testing.T) {
95106
require.NoError(db.Put(key, node))
96107

97108
// Assert cache has expected number of elements
98-
require.Equal(1, db.nodeCache.fifo.Len())
99-
gotKey, gotNode, ok := db.nodeCache.fifo.Oldest()
109+
require.Equal(2, db.nodeCache.fifo.Len())
110+
gotKey, _, ok := db.nodeCache.fifo.Oldest()
100111
require.True(ok)
101-
require.Equal(key, gotKey)
102-
require.Equal(node, gotNode)
112+
require.Equal(newPath([]byte{byte(added - 1)}), gotKey)
103113

104114
// Get a node from the base database (not cache)
105115
nodeRead, err := db.Get(newPath([]byte{0x03}))
@@ -112,7 +122,7 @@ func TestIntermediateNodeDB(t *testing.T) {
112122
// Assert the cache is empty
113123
require.Zero(db.nodeCache.fifo.Len())
114124

115-
// Assert all [cacheSize]+1 elements evicted were written to disk with prefix.
125+
// Assert the evicted cache elements were written to disk with prefix.
116126
it := baseDB.NewIteratorWithPrefix(intermediateNodePrefix)
117127
defer it.Release()
118128

@@ -121,5 +131,5 @@ func TestIntermediateNodeDB(t *testing.T) {
121131
count++
122132
}
123133
require.NoError(it.Error())
124-
require.Equal(cacheSize+1, count)
134+
require.Equal(added+1, count)
125135
}

x/merkledb/proof.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"context"
99
"errors"
1010
"fmt"
11+
"math"
1112

1213
"github.com/ava-labs/avalanchego/database"
1314
"github.com/ava-labs/avalanchego/database/memdb"
@@ -19,7 +20,10 @@ import (
1920
pb "github.com/ava-labs/avalanchego/proto/pb/sync"
2021
)
2122

22-
const verificationCacheSize = 2_000
23+
const (
24+
verificationEvictionBatchSize = 0
25+
verificationCacheSize = math.MaxInt
26+
)
2327

2428
var (
2529
ErrInvalidProof = errors.New("proof obtained an invalid root ID")
@@ -870,7 +874,7 @@ func getStandaloneTrieView(ctx context.Context, ops []database.BatchOp) (*trieVi
870874
ctx,
871875
memdb.New(),
872876
Config{
873-
EvictionBatchSize: DefaultEvictionBatchSize,
877+
EvictionBatchSize: verificationEvictionBatchSize,
874878
Tracer: trace.Noop,
875879
ValueNodeCacheSize: verificationCacheSize,
876880
IntermediateNodeCacheSize: verificationCacheSize,

x/merkledb/value_node_db.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ type valueNodeDB struct {
2424

2525
// If a value is nil, the corresponding key isn't in the trie.
2626
// Paths in [nodeCache] aren't prefixed with [valueNodePrefix].
27-
nodeCache cache.LRU[path, *node]
27+
nodeCache cache.Cacher[path, *node]
2828
metrics merkleMetrics
2929

3030
closed utils.Atomic[bool]
@@ -35,7 +35,7 @@ func newValueNodeDB(db database.Database, bufferPool *sync.Pool, metrics merkleM
3535
metrics: metrics,
3636
baseDB: db,
3737
bufferPool: bufferPool,
38-
nodeCache: cache.LRU[path, *node]{Size: size},
38+
nodeCache: cache.NewSizedLRU(size, cacheEntrySize),
3939
}
4040
}
4141

0 commit comments

Comments
 (0)