From 9301c894ee946a45a3ec2d43587d030aed6a4d18 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Thu, 16 Jul 2026 16:37:48 +0200 Subject: [PATCH] db/kv/membatchwithdb: restrict overlays to the pure-Go memStore backing Split out of #21293. Shared-tx read views are safe under a concurrent parent Close only because memStore's Rollback/Close are no-ops on the data; typing memTx as *memStore enforces that backing. Remove the unused MDBX-backed constructor, whose write tx pins an OS thread and would invalidate cursors mid-iteration. --- db/kv/membatchwithdb/memory_mutation.go | 44 ++++++------------------- db/state/execctx/domain_shared.go | 5 +++ rpc/rpchelper/filters.go | 9 +++-- 3 files changed, 22 insertions(+), 36 deletions(-) diff --git a/db/kv/membatchwithdb/memory_mutation.go b/db/kv/membatchwithdb/memory_mutation.go index e227c98fef4..d82322982d7 100644 --- a/db/kv/membatchwithdb/memory_mutation.go +++ b/db/kv/membatchwithdb/memory_mutation.go @@ -29,8 +29,6 @@ import ( "github.com/erigontech/erigon/common" "github.com/erigontech/erigon/common/log/v3" "github.com/erigontech/erigon/db/kv" - "github.com/erigontech/erigon/db/kv/dbcfg" - "github.com/erigontech/erigon/db/kv/mdbx" "github.com/erigontech/erigon/db/kv/order" "github.com/erigontech/erigon/db/kv/stream" "github.com/erigontech/erigon/db/snapshotsync/blocksnapshots" @@ -46,7 +44,7 @@ type MemoryMutation struct { // Read views created via NewReadView share this pointer so they synchronize // with the parent's writers. mu *sync.RWMutex - memTx kv.RwTx + memTx *memStore // concrete type is load-bearing — see newReadViewMut memDb kv.RwDB deletedEntries map[string]map[string]struct{} deletedDups map[string]map[string]map[string]struct{} @@ -82,36 +80,6 @@ func NewMemoryBatch(tx kv.TemporalTx, tmpDir string, logger log.Logger) (*Memory }, nil } -// NewMemoryBatchMDBX creates an MDBX-backed in-memory batch. The MDBX write -// transaction pins the goroutine to an OS thread via runtime.LockOSThread(), -// so this variant must not be held across goroutine migrations. -func NewMemoryBatchMDBX(tx kv.TemporalTx, tmpDir string, logger log.Logger) (mm *MemoryMutation, err error) { - tmpDB := mdbx.New(dbcfg.TemporaryDB, logger).InMem(nil, tmpDir).GrowthStep(64 * datasize.MB).MapSize(512 * datasize.GB).MustOpen() - defer func() { - if err != nil { - tmpDB.Close() - } - }() - memTx, err := tmpDB.BeginRw(context.Background()) // nolint:gocritic - if err != nil { - return nil, fmt.Errorf("NewMemoryBatchMDBX: begin tx: %w", err) - } - if err = initSequences(tx, memTx); err != nil { - memTx.Rollback() - return nil, fmt.Errorf("NewMemoryBatchMDBX: init sequences: %w", err) - } - - return &MemoryMutation{ - mu: &sync.RWMutex{}, - db: tx, - memDb: tmpDB, - memTx: memTx, - deletedEntries: make(map[string]map[string]struct{}), - deletedDups: map[string]map[string]map[string]struct{}{}, - clearedTables: make(map[string]struct{}), - }, nil -} - func (m *MemoryMutation) UnderlyingTx() kv.TemporalTx { return m.db } @@ -576,6 +544,8 @@ func (m *MemoryMutation) Commit() error { return nil } +// Safe to close while read views are still iterating: the memStore backing +// makes Rollback a no-op on the data (see newReadViewMut). func (m *MemoryMutation) Rollback() { m.memTx.Rollback() m.memDb.Close() @@ -1042,13 +1012,19 @@ func (m *MemoryMutation) Unwind(ctx context.Context, txNumUnwindTo uint64, chang // // The returned kv.TemporalTx only exposes read methods. Callers cannot write // to the overlay through this view. The caller must not Close the returned -// view (it doesn't own the memDb). +// view (it doesn't own the memDb). Safe under a concurrent parent Close — +// see newReadViewMut. func (m *MemoryMutation) NewReadView(tx kv.Tx) kv.TemporalTx { return m.newReadViewMut(tx) } // newReadViewMut is the internal constructor that returns the full // *MemoryMutation. Used by NewTemporalReadView which needs to embed it. +// +// Read views stay safe under a concurrent parent Close only because the +// pure-Go memStore's Rollback/Close are no-ops on its data — memTx's type +// enforces that backing. A real-DB-backed memTx would invalidate cursors +// mid-iteration and need refcount/drain logic at the parent's Close. func (m *MemoryMutation) newReadViewMut(tx kv.Tx) *MemoryMutation { var dbTx kv.TemporalTx if t, ok := tx.(kv.TemporalTx); ok { diff --git a/db/state/execctx/domain_shared.go b/db/state/execctx/domain_shared.go index 2d994f32aa9..ea429e7749f 100644 --- a/db/state/execctx/domain_shared.go +++ b/db/state/execctx/domain_shared.go @@ -814,6 +814,11 @@ func (sd *SharedDomains) IteratePrefix(domain kv.Domain, prefix []byte, roTx kv. return sd.mem.IteratePrefix(domain, prefix, roTx, it) } +// Close releases this SD's in-memory state. Idempotent. +// +// Safe to call while readers still hold block-overlay views: the overlay's +// memStore backing keeps their data alive (see MemoryMutation.newReadViewMut), +// and sd.mem is never exposed to those views. func (sd *SharedDomains) Close() { if sd.sdCtx == nil { //idempotency return diff --git a/rpc/rpchelper/filters.go b/rpc/rpchelper/filters.go index fa0beceeb6e..52d952a6ba9 100644 --- a/rpc/rpchelper/filters.go +++ b/rpc/rpchelper/filters.go @@ -1079,9 +1079,12 @@ func (ff *Filters) LatestSD() *execctx.SharedDomains { // WithOverlay returns a read view backed by the latest block overlay if one // is available, otherwise returns the given tx unchanged. The read view uses -// the overlay's in-memory data for table lookups, falling back to the caller's tx -// for data not in the overlay. +// the overlay's in-memory data for table lookups, falling back to the caller's +// tx for data not in the overlay. // Safe to call on a nil receiver. +// +// The view stays readable if the publisher closes the SD concurrently (see +// MemoryMutation.newReadViewMut); it must not outlive the caller's tx. func (ff *Filters) WithOverlay(tx kv.Tx) kv.Tx { if ff == nil { return tx @@ -1098,6 +1101,8 @@ func (ff *Filters) WithOverlay(tx kv.Tx) kv.Tx { // WithTemporalOverlay is like WithOverlay but returns kv.TemporalTx directly, // avoiding repeated type assertions at callsites that need temporal access. +// The same concurrent-close safety property as WithOverlay applies (see the +// concurrency note there). func (ff *Filters) WithTemporalOverlay(tx kv.TemporalTx) kv.TemporalTx { if ff == nil { return tx