diff --git a/sei-db/state_db/sc/memiavl/db.go b/sei-db/state_db/sc/memiavl/db.go index b342cbfd76..df0fecfe57 100644 --- a/sei-db/state_db/sc/memiavl/db.go +++ b/sei-db/state_db/sc/memiavl/db.go @@ -169,6 +169,15 @@ func OpenDB(targetVersion int64, opts Options) (database *DB, _err error) { err error fileLock FileLock ) + // A failed open hands back no DB, so nothing else can release the lock. The + // process usually exits either way, but a caller that recovers and retries + // would otherwise lock itself out. + defer func() { + if _err != nil && fileLock != nil { + _ = fileLock.Unlock() + _ = fileLock.Destroy() + } + }() if err := opts.Validate(); err != nil { return nil, fmt.Errorf("invalid commit store options: %w", err) } @@ -179,12 +188,14 @@ func OpenDB(targetVersion int64, opts Options) (database *DB, _err error) { } } - if !opts.ReadOnly { + if !opts.ReadOnly || opts.RequireExclusive { fileLock, err = LockFile(filepath.Join(opts.Dir, LockFileName)) if err != nil { return nil, fmt.Errorf("fail to lock db: %w", err) } + } + if !opts.ReadOnly { // cleanup any temporary directories left by interrupted snapshot rewrite if err := removeTmpDirs(opts.Dir); err != nil { return nil, fmt.Errorf("fail to cleanup tmp directories: %w", err) diff --git a/sei-db/state_db/sc/memiavl/filelock.go b/sei-db/state_db/sc/memiavl/filelock.go index 6becb39b82..7ff49cabfd 100644 --- a/sei-db/state_db/sc/memiavl/filelock.go +++ b/sei-db/state_db/sc/memiavl/filelock.go @@ -11,6 +11,10 @@ type FileLock interface { Destroy() error } +// ErrLocked reports that another process holds the lock, which for a database +// directory means a writer has it open. +var ErrLocked = filelock.ErrLocked + func LockFile(fname string) (FileLock, error) { path, err := filepath.Abs(fname) if err != nil { diff --git a/sei-db/state_db/sc/memiavl/opts.go b/sei-db/state_db/sc/memiavl/opts.go index 9c4a6f5d31..27f7b92fb2 100644 --- a/sei-db/state_db/sc/memiavl/opts.go +++ b/sei-db/state_db/sc/memiavl/opts.go @@ -18,6 +18,11 @@ type Options struct { InitialVersion uint32 // ReadOnly opens the database in read-only mode ReadOnly bool + // RequireExclusive takes the directory's lock even under ReadOnly, so the + // open fails with ErrLocked while a writer has it. A read-only tool that + // must not disturb the directory sets it, because the changelog opener + // mutates the directory whether or not the DB API can write. + RequireExclusive bool // InitialStores are the initial store names when initializing an empty instance InitialStores []string // ZeroCopy if true, get and iterator methods return slices pointing to mmaped blob files diff --git a/sei-db/tools/cmd/seidb/operations/evm_logical_digest.go b/sei-db/tools/cmd/seidb/operations/evm_logical_digest.go index 79ecd140c6..ad0c930977 100644 --- a/sei-db/tools/cmd/seidb/operations/evm_logical_digest.go +++ b/sei-db/tools/cmd/seidb/operations/evm_logical_digest.go @@ -84,9 +84,11 @@ const ( // - replay (SLOW): opens a read-only DB, replays the changelog up to // --height, then walks the in-memory/mmap tree. Roughly an order of // magnitude slower than snapshot (changelog replay + per-leaf tree walk -// instead of a sequential file read). Use it only when no snapshot exists -// at the target height — e.g. nodes whose snapshot rewrite lags the tip, so -// an arbitrary comparison height has no snapshot- on disk. +// instead of a sequential file read). It requires the node stopped, because +// opening a changelog its writer still holds lets the opener repair, and +// therefore damage, that changelog. Use it only when no snapshot exists at +// the target height — e.g. nodes whose snapshot rewrite lags the tip, so an +// arbitrary comparison height has no snapshot- on disk. // // The flatkv side is always a pebble WAL-replay-to-height and is fast // regardless. So when comparing across nodes, pick a height that is an existing @@ -1106,11 +1108,17 @@ func digestMemIAVL(dbDir string, height int64, findTarget []byte, normalization func openMemiAVLReplayReadOnly(dbDir string, height int64) (*memiavl.DB, error) { db, err := memiavl.OpenDB(height, memiavl.Options{ - Dir: dbDir, - ReadOnly: true, - ZeroCopy: true, + Dir: dbDir, + ReadOnly: true, + ZeroCopy: true, + RequireExclusive: true, }) if err != nil { + if errors.Is(err, memiavl.ErrLocked) { + return nil, fmt.Errorf("another process has %s open, and replaying the changelog of a "+ + "directory being written lets the changelog opener truncate a record that writer has "+ + "committed; stop seid and rerun, or use --memiavl-open-mode snapshot: %w", dbDir, err) + } return nil, fmt.Errorf("open memiavl read-only replay: %w", err) } return db, nil diff --git a/sei-db/tools/cmd/seidb/operations/memiavl_open_test.go b/sei-db/tools/cmd/seidb/operations/memiavl_open_test.go new file mode 100644 index 0000000000..c87398e115 --- /dev/null +++ b/sei-db/tools/cmd/seidb/operations/memiavl_open_test.go @@ -0,0 +1,65 @@ +package operations + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/sei-protocol/sei-chain/sei-db/common/keys" + "github.com/sei-protocol/sei-chain/sei-db/common/utils" + "github.com/sei-protocol/sei-chain/sei-db/proto" + "github.com/sei-protocol/sei-chain/sei-db/state_db/sc/memiavl" +) + +// TestOpenMemiAVLReplayReadOnlyRefusesADirectoryAWriterHasOpen covers what no +// check of the directory can: the changelog opener repairs a torn tail and +// completes an interrupted truncation, and both conditions occur transiently +// while a writer appends or truncates. Replaying a directory a writer holds is +// refused instead. +func TestOpenMemiAVLReplayReadOnlyRefusesADirectoryAWriterHasOpen(t *testing.T) { + homeDir := t.TempDir() + store := newTestMemiavlStore(t, homeDir) + require.NoError(t, store.ApplyChangeSets([]*proto.NamedChangeSet{{ + Name: keys.EVMStoreKey, + Changeset: proto.ChangeSet{Pairs: []*proto.KVPair{noncePair(addrN(0xA1), 1)}}, + }})) + _, err := store.Commit(store.Version() + 1) + require.NoError(t, err) + + // The store is still open, holding the lock the way a running seid does. + dbDir := utils.GetCosmosSCStorePath(homeDir) + db, err := openMemiAVLReplayReadOnly(dbDir, 0) + require.Nil(t, db) + require.ErrorIs(t, err, memiavl.ErrLocked) + require.Contains(t, err.Error(), "stop seid and rerun") + require.Contains(t, err.Error(), "--memiavl-open-mode snapshot") + + // The same directory opens once that writer releases it, so the refusal + // tracks the writer rather than something the first open left behind. + require.NoError(t, store.Close()) + db, err = openMemiAVLReplayReadOnly(dbDir, 0) + require.NoError(t, err) + require.NoError(t, db.Close()) +} + +// TestOpenMemiAVLReplayReadOnlyReplaysAStoppedNode is the positive control: the +// lock is the only thing the refusal adds, so a released directory replays as +// it did before. +func TestOpenMemiAVLReplayReadOnlyReplaysAStoppedNode(t *testing.T) { + homeDir := t.TempDir() + store := newTestMemiavlStore(t, homeDir) + for nonce := uint64(1); nonce <= 3; nonce++ { + require.NoError(t, store.ApplyChangeSets([]*proto.NamedChangeSet{{ + Name: keys.EVMStoreKey, + Changeset: proto.ChangeSet{Pairs: []*proto.KVPair{noncePair(addrN(0xA1), nonce)}}, + }})) + _, err := store.Commit(store.Version() + 1) + require.NoError(t, err) + } + require.NoError(t, store.Close()) + + db, err := openMemiAVLReplayReadOnly(utils.GetCosmosSCStorePath(homeDir), 3) + require.NoError(t, err) + defer func() { _ = db.Close() }() + require.Equal(t, int64(3), db.Version()) +}