Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
perf(store): save metadata async
  • Loading branch information
julienrbrt committed Apr 28, 2026
commit f7ea4ad43b6b9da9762e6f3a5a4746281152f5bd
88 changes: 87 additions & 1 deletion pkg/store/cached_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package store

import (
"context"
"sync"

lru "github.com/hashicorp/golang-lru/v2"
"github.com/rs/zerolog"

"github.com/evstack/ev-node/types"
)
Expand All @@ -14,15 +16,32 @@ const (

// DefaultBlockDataCacheSize is the default number of block data entries to cache.
DefaultBlockDataCacheSize = 200_000

asyncWriteBufferSize = 8192
)

type asyncWriteOp struct {
key string
value []byte
isDelete bool
}

// CachedStore wraps a Store with LRU caching for frequently accessed data.
// The underlying LRU cache is thread-safe, so no additional synchronization is needed.
// Metadata writes (SetMetadata, DeleteMetadata) are processed asynchronously via a
// buffered channel to avoid blocking Badger's write pipeline for critical operations
// like block production (batch commits).
type CachedStore struct {
Store

headerCache *lru.Cache[uint64, *types.SignedHeader]
blockDataCache *lru.Cache[uint64, *blockDataEntry]

writeCh chan asyncWriteOp
done chan struct{}
stopMu sync.RWMutex
stopped bool
logger zerolog.Logger
}

type blockDataEntry struct {
Expand Down Expand Up @@ -73,6 +92,9 @@ func NewCachedStore(store Store, opts ...CachedStoreOption) (*CachedStore, error
Store: store,
headerCache: headerCache,
blockDataCache: blockDataCache,
writeCh: make(chan asyncWriteOp, asyncWriteBufferSize),
done: make(chan struct{}),
logger: zerolog.Nop(),
}

for _, opt := range opts {
Expand All @@ -81,9 +103,30 @@ func NewCachedStore(store Store, opts ...CachedStoreOption) (*CachedStore, error
}
}

cs.startWriteLoop()

return cs, nil
}

func (cs *CachedStore) startWriteLoop() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the performance gained through this change? also what is the performance difference with non automatic fsync on the db, we should be able to write to the db fast, it keeps it in cache then it flushes every n blocks. Are we already doing that?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Badger already has non automatic fsync, so that would be the same.
The issue is even to multiple concurrent writes (submitter + executor), writing to WAL requires a short lock.

go func() {
defer close(cs.done)
for op := range cs.writeCh {
var err error
if op.isDelete {
err = cs.Store.DeleteMetadata(context.Background(), op.key)
} else {
err = cs.Store.SetMetadata(context.Background(), op.key, op.value)
}
if err != nil {
cs.logger.Error().Err(err).Str("key", op.key).
Bool("delete", op.isDelete).
Msg("async metadata write failed")
}
}
}()
}

// GetHeader returns the header at the given height, using the cache if available.
func (cs *CachedStore) GetHeader(ctx context.Context, height uint64) (*types.SignedHeader, error) {
// Try cache first
Expand Down Expand Up @@ -173,8 +216,51 @@ func (cs *CachedStore) PruneBlocks(ctx context.Context, height uint64) error {
return nil
}

// Close closes the underlying store.
// SetMetadata queues an asynchronous metadata write. The write is persisted
// by the background goroutine. If the buffer is full or the store has been
// stopped, the write falls back to synchronous execution on the underlying store.
func (cs *CachedStore) SetMetadata(ctx context.Context, key string, value []byte) error {
cs.stopMu.RLock()
if cs.stopped {
cs.stopMu.RUnlock()
return cs.Store.SetMetadata(ctx, key, value)
}
select {
case cs.writeCh <- asyncWriteOp{key: key, value: value}:
cs.stopMu.RUnlock()
return nil
default:
cs.stopMu.RUnlock()
return cs.Store.SetMetadata(ctx, key, value)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Comment on lines +256 to +266

@coderabbitai coderabbitai Bot Apr 28, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Blocking send contradicts PR description's backpressure fallback.

The PR description states "When the buffer is full, operations fall back to synchronous writes (backpressure)". However, the current implementation uses a blocking send (cs.writeCh <- asyncWriteOp{...}) which will block the caller indefinitely if the 8192-slot buffer is full, rather than falling back to synchronous writes.

If the fallback behavior is intended, use a non-blocking send:

Proposed fix for fallback behavior
 func (cs *CachedStore) SetMetadata(ctx context.Context, key string, value []byte) error {
 	cs.stopMu.RLock()
 	defer cs.stopMu.RUnlock()

 	if cs.stopped {
 		return cs.Store.SetMetadata(ctx, key, value)
 	}
 	valueCopy := append([]byte(nil), value...)
-	cs.writeCh <- asyncWriteOp{key: key, value: valueCopy}
-	return nil
+	select {
+	case cs.writeCh <- asyncWriteOp{key: key, value: valueCopy}:
+		return nil
+	default:
+		return cs.Store.SetMetadata(ctx, key, value)
+	}
 }

If blocking is intentional (for guaranteed ordering), update the PR description to reflect this.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/store/cached_store.go` around lines 256 - 266, The current
CachedStore.SetMetadata uses a blocking send to cs.writeCh (asyncWriteOp) which
contradicts the PR description's required backpressure fallback; change the send
to a non-blocking select that attempts to enqueue the asyncWriteOp to writeCh
and, if the channel is full, synchronously call the underlying
Store.SetMetadata(ctx, key, valueCopy) instead (preserve stopMu read lock and
the stopped check, and keep copying value into valueCopy before either enqueuing
or falling back).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is intentional.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@julienrbrt, understood — the blocking send is intentional to ensure ordered and guaranteed delivery of metadata write operations to the background goroutine. I'll note this for future reviews.

( ˘•ω•˘ )


✏️ Learnings added
Learnt from: julienrbrt
Repo: evstack/ev-node PR: 3298
File: pkg/store/cached_store.go:256-266
Timestamp: 2026-04-28T21:47:13.754Z
Learning: In `evstack/ev-node`, `CachedStore.SetMetadata` and `CachedStore.DeleteMetadata` in `pkg/store/cached_store.go` use a **blocking** send to `cs.writeCh` intentionally. Despite the PR description mentioning a "backpressure fallback to synchronous writes", the blocking behavior is a deliberate design choice for guaranteed ordering. Do not flag the blocking channel send as a bug or suggest replacing it with a non-blocking select fallback.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: alpe
Repo: evstack/ev-node PR: 3131
File: block/internal/syncing/syncer_backoff_test.go:241-245
Timestamp: 2026-03-06T09:40:36.029Z
Learning: In evstack/ev-node, the module declares go 1.25.6. Since Go 1.22, loop variables are per-iteration by default, so loop variable capture is not a concern. Do not flag or fix loop variable capture in this codebase for any Go files; this rule applies to all Go files in the repository, not just the specific test file.


// DeleteMetadata queues an asynchronous metadata delete. If the buffer is full
// or the store has been stopped, the delete falls back to synchronous execution.
func (cs *CachedStore) DeleteMetadata(ctx context.Context, key string) error {
cs.stopMu.RLock()
if cs.stopped {
cs.stopMu.RUnlock()
return cs.Store.DeleteMetadata(ctx, key)
}
select {
case cs.writeCh <- asyncWriteOp{key: key, isDelete: true}:
cs.stopMu.RUnlock()
return nil
default:
cs.stopMu.RUnlock()
return cs.Store.DeleteMetadata(ctx, key)
}
}

// Close drains pending async writes, then closes the underlying store.
func (cs *CachedStore) Close() error {
cs.stopMu.Lock()
cs.stopped = true
close(cs.writeCh)
cs.stopMu.Unlock()
<-cs.done

cs.ClearCache()
return cs.Store.Close()
}
84 changes: 84 additions & 0 deletions pkg/store/cached_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package store
import (
"context"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -270,3 +271,86 @@ func TestCachedStore_Close(t *testing.T) {
err = cachedStore.Close()
require.NoError(t, err)
}

func TestCachedStore_AsyncSetMetadata(t *testing.T) {
t.Parallel()
ctx := context.Background()

kv, err := NewTestInMemoryKVStore()
require.NoError(t, err)

base := New(kv)
cs, err := NewCachedStore(base)
require.NoError(t, err)
t.Cleanup(func() { cs.Close() })

require.NoError(t, cs.SetMetadata(ctx, "key1", []byte("value1")))

require.Eventually(t, func() bool {
v, err := base.GetMetadata(ctx, "key1")
return err == nil && string(v) == "value1"
}, time.Second, 10*time.Millisecond)
}

func TestCachedStore_AsyncDeleteMetadata(t *testing.T) {
t.Parallel()
ctx := context.Background()

kv, err := NewTestInMemoryKVStore()
require.NoError(t, err)

base := New(kv)
require.NoError(t, base.SetMetadata(ctx, "key1", []byte("value1")))

cs, err := NewCachedStore(base)
require.NoError(t, err)
t.Cleanup(func() { cs.Close() })

require.NoError(t, cs.DeleteMetadata(ctx, "key1"))

require.Eventually(t, func() bool {
_, err := base.GetMetadata(ctx, "key1")
return err != nil
}, time.Second, 10*time.Millisecond)
}

func TestCachedStore_Close_FlushesPendingWrites(t *testing.T) {
kv, err := NewTestInMemoryKVStore()
require.NoError(t, err)

base := New(kv)
cs, err := NewCachedStore(base)
require.NoError(t, err)

ctx := context.Background()
const n = 100
for i := 0; i < n; i++ {
k := []byte{byte(i)}
require.NoError(t, cs.SetMetadata(ctx, string(k), k))
}

// Wait for the last key to land via pass-through read
require.Eventually(t, func() bool {
v, err := cs.GetMetadata(ctx, string([]byte{byte(n - 1)}))
return err == nil && len(v) == 1 && v[0] == byte(n-1)
}, 2*time.Second, 10*time.Millisecond)

require.NoError(t, cs.Close())
Comment thread
julienrbrt marked this conversation as resolved.
Outdated
}

func TestCachedStore_WriteAfterClose_FallsBack(t *testing.T) {
kv, err := NewTestInMemoryKVStore()
require.NoError(t, err)

base := New(kv)
cs, err := NewCachedStore(base)
require.NoError(t, err)

ctx := context.Background()
require.NoError(t, cs.SetMetadata(ctx, "before", []byte("ok")))

require.NoError(t, cs.Close())

err = cs.SetMetadata(ctx, "after", []byte("sync"))
require.Error(t, err)
}
Loading