From 50dc1d2249df966c7cd01603014a11f2dc9ccbee Mon Sep 17 00:00:00 2001 From: yihuang Date: Thu, 13 Jul 2023 15:54:11 +0800 Subject: [PATCH] Problem: memiavl is not protected against concurrent writing (#1100) * Problem: memiavl is not protected against concurrent writing Solution: - introduce exclusive file lock. - introduce read-only mode which can work concurrently. * Update CHANGELOG.md Signed-off-by: yihuang * cleanup * fix same process lock * file permission * Update memiavl/db.go Co-authored-by: mmsqe Signed-off-by: yihuang * fix lint * fix unit test * cross-platform * absolute path --------- Signed-off-by: yihuang Co-authored-by: mmsqe --- CHANGELOG.md | 1 + go.mod | 1 + go.sum | 2 + gomod2nix.toml | 3 + memiavl/db.go | 126 ++++++++++++++++++++++++++++++++------- memiavl/db_test.go | 43 ++++++++++++- memiavl/export.go | 1 + memiavl/filelock.go | 27 +++++++++ memiavl/go.mod | 1 + memiavl/go.sum | 2 + store/go.mod | 1 + store/go.sum | 2 + store/rootmulti/store.go | 2 +- versiondb/go.mod | 1 + versiondb/go.sum | 2 + 15 files changed, 190 insertions(+), 25 deletions(-) create mode 100644 memiavl/filelock.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 74f91ef957..6f236df803 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ - [#1042](https://github.com/crypto-org-chain/cronos/pull/1042) call Close method on app to cleanup resource on graceful shutdown ([ethermint commit](https://github.com/crypto-org-chain/ethermint/commit/0ea7b86532a1144f229961f94b4524d5889e874d)). - [#1083](https://github.com/crypto-org-chain/cronos/pull/1083) memiavl support both sdk 46 and 47 root hash rules. - [#1091](https://github.com/crypto-org-chain/cronos/pull/1091) memiavl support rollback. +- [#1100](https://github.com/crypto-org-chain/cronos/pull/1100) memiavl support read-only mode, and grab exclusive lock for write mode. ### Improvements diff --git a/go.mod b/go.mod index 22db0c86d9..586609e3e2 100644 --- a/go.mod +++ b/go.mod @@ -190,6 +190,7 @@ require ( github.com/tklauser/numcpus v0.4.0 // indirect github.com/tyler-smith/go-bip39 v1.1.0 // indirect github.com/ulikunitz/xz v0.5.10 // indirect + github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d // indirect github.com/zondax/hid v0.9.1 // indirect github.com/zondax/ledger-go v0.14.1 // indirect go.etcd.io/bbolt v1.3.7 // indirect diff --git a/go.sum b/go.sum index 4ddfa8b28b..742744f522 100644 --- a/go.sum +++ b/go.sum @@ -1307,6 +1307,8 @@ github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= +github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d h1:XQyeLr7N9iY9mi+TGgsBFkj54+j3fdoo8e2u6zrGP5A= +github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d/go.mod h1:hoMeDjlNXTNqVwrCk8YDyaBS2g5vFfEX2ezMi4vb6CY= github.com/zondax/hid v0.9.1 h1:gQe66rtmyZ8VeGFcOpbuH3r7erYtNEAezCAYu8LdkJo= github.com/zondax/hid v0.9.1/go.mod h1:l5wttcP0jwtdLjqjMMWFVEE7d1zO0jvSPA9OPZxWpEM= github.com/zondax/ledger-go v0.14.1 h1:Pip65OOl4iJ84WTpA4BKChvOufMhhbxED3BaihoZN4c= diff --git a/gomod2nix.toml b/gomod2nix.toml index 38f6a815f0..2456e3850f 100644 --- a/gomod2nix.toml +++ b/gomod2nix.toml @@ -538,6 +538,9 @@ schema = 3 [mod."github.com/ulikunitz/xz"] version = "v0.5.10" hash = "sha256-bogOwQNmQVS7W+C7wci7XEUeYm9TB7PnxnyBIXKYbm0=" + [mod."github.com/zbiljic/go-filelock"] + version = "v0.0.0-20170914061330-1dbf7103ab7d" + hash = "sha256-JqNj/Wg8nGFSmndgYC7+FZzL2zG7rwOQMjlqYs3ZGvw=" [mod."github.com/zondax/hid"] version = "v0.9.1" hash = "sha256-hSVmN/f/lQHFhF60o6ej78ELC0MMoqQgqIX2hHjdTXg=" diff --git a/memiavl/db.go b/memiavl/db.go index 63baad14e0..b2496d73d3 100644 --- a/memiavl/db.go +++ b/memiavl/db.go @@ -14,7 +14,12 @@ import ( "github.com/tidwall/wal" ) -const DefaultSnapshotInterval = 1000 +const ( + DefaultSnapshotInterval = 1000 + LockFileName = "LOCK" +) + +var errReadOnly = errors.New("db is read-only") // DB implements DB-like functionalities on top of MultiTree: // - async snapshot rewriting @@ -34,8 +39,10 @@ const DefaultSnapshotInterval = 1000 // ``` type DB struct { MultiTree - dir string - logger log.Logger + dir string + logger log.Logger + fileLock FileLock + readOnly bool // result channel of snapshot rewrite goroutine snapshotRewriteChan chan snapshotResult @@ -72,6 +79,7 @@ type Options struct { Logger log.Logger CreateIfMissing bool InitialVersion uint32 + ReadOnly bool // the initial stores when initialize the empty instance InitialStores []string SnapshotKeepRecent uint32 @@ -94,12 +102,56 @@ type Options struct { LoadForOverwriting bool } +func (opts Options) Validate() error { + if opts.ReadOnly && opts.CreateIfMissing { + return errors.New("can't create db in read-only mode") + } + + if opts.ReadOnly && opts.LoadForOverwriting { + return errors.New("can't rollback db in read-only mode") + } + + return nil +} + +func (opts *Options) FillDefaults() { + if opts.Logger == nil { + opts.Logger = log.NewNopLogger() + } + + if opts.SnapshotInterval == 0 { + opts.SnapshotInterval = DefaultSnapshotInterval + } +} + const ( SnapshotPrefix = "snapshot-" SnapshotDirLen = len(SnapshotPrefix) + 20 ) func Load(dir string, opts Options) (*DB, error) { + if err := opts.Validate(); err != nil { + return nil, fmt.Errorf("invalid options: %w", err) + } + opts.FillDefaults() + + if opts.CreateIfMissing { + if err := createDBIfNotExist(dir, opts.InitialVersion); err != nil { + return nil, fmt.Errorf("fail to load db: %w", err) + } + } + + var ( + err error + fileLock FileLock + ) + if !opts.ReadOnly { + fileLock, err = LockFile(filepath.Join(dir, LockFileName)) + if err != nil { + return nil, fmt.Errorf("fail to lock db: %w", err) + } + } + snapshot := "current" if opts.TargetVersion > 0 { // find the biggest snapshot version that's less than or equal to the target version @@ -113,15 +165,7 @@ func Load(dir string, opts Options) (*DB, error) { path := filepath.Join(dir, snapshot) mtree, err := LoadMultiTree(path, opts.ZeroCopy, opts.CacheSize) if err != nil { - if opts.CreateIfMissing && os.IsNotExist(err) { - if err := initEmptyDB(dir, opts.InitialVersion); err != nil { - return nil, err - } - mtree, err = LoadMultiTree(path, opts.ZeroCopy, opts.CacheSize) - } - if err != nil { - return nil, err - } + return nil, err } wal, err := OpenWAL(walPath(dir), &wal.Options{NoCopy: true, NoSync: true}) @@ -176,6 +220,8 @@ func Load(dir string, opts Options) (*DB, error) { MultiTree: *mtree, logger: opts.Logger, dir: dir, + fileLock: fileLock, + readOnly: opts.ReadOnly, wal: wal, walChanSize: opts.AsyncCommitBuffer, snapshotKeepRecent: opts.SnapshotKeepRecent, @@ -184,15 +230,7 @@ func Load(dir string, opts Options) (*DB, error) { triggerStateSyncExport: opts.TriggerStateSyncExport, } - if db.logger == nil { - db.logger = log.NewNopLogger() - } - - if db.snapshotInterval == 0 { - db.snapshotInterval = DefaultSnapshotInterval - } - - if db.Version() == 0 && len(opts.InitialStores) > 0 { + if !db.readOnly && db.Version() == 0 && len(opts.InitialStores) > 0 { // do the initial upgrade with the `opts.InitialStores` var upgrades []*TreeNameUpgrade for _, name := range opts.InitialStores { @@ -206,6 +244,11 @@ func Load(dir string, opts Options) (*DB, error) { return db, nil } +// ReadOnly returns whether the DB is opened in read-only mode. +func (db *DB) ReadOnly() bool { + return db.readOnly +} + // SetInitialVersion wraps `MultiTree.SetInitialVersion`. // it do an immediate snapshot rewrite, because we can't use wal log to record this change, // because we need it to convert versions to wal index in the first place. @@ -230,6 +273,10 @@ func (db *DB) ApplyUpgrades(upgrades []*TreeNameUpgrade) error { db.mtx.Lock() defer db.mtx.Unlock() + if db.readOnly { + return errReadOnly + } + if err := db.MultiTree.ApplyUpgrades(upgrades); err != nil { return err } @@ -351,6 +398,10 @@ func (db *DB) Commit(changeSets []*NamedChangeSet) ([]byte, int64, error) { db.mtx.Lock() defer db.mtx.Unlock() + if db.readOnly { + return nil, 0, errReadOnly + } + if err := db.checkAsyncTasks(); err != nil { return nil, 0, err } @@ -457,6 +508,10 @@ func (db *DB) RewriteSnapshot() error { db.mtx.Lock() defer db.mtx.Unlock() + if db.readOnly { + return errReadOnly + } + snapshotDir := snapshotName(db.lastCommitInfo.Version) tmpDir := snapshotDir + "-tmp" path := filepath.Join(db.dir, tmpDir) @@ -525,6 +580,10 @@ func (db *DB) RewriteSnapshotBackground() error { db.mtx.Lock() defer db.mtx.Unlock() + if db.readOnly { + return errReadOnly + } + return db.rewriteSnapshotBackground() } @@ -571,7 +630,17 @@ func (db *DB) Close() error { db.mtx.Lock() defer db.mtx.Unlock() - return errors.Join(db.waitAsyncCommit(), db.MultiTree.Close(), db.wal.Close()) + errs := []error{ + db.waitAsyncCommit(), db.MultiTree.Close(), db.wal.Close(), + } + db.wal = nil + + if db.fileLock != nil { + errs = append(errs, db.fileLock.Unlock()) + db.fileLock = nil + } + + return errors.Join(errs...) } // TreeByName wraps MultiTree.TreeByName to add a lock. @@ -611,6 +680,10 @@ func (db *DB) ApplyChangeSet(changeSets []*NamedChangeSet, updateCommitInfo bool db.mtx.Lock() defer db.mtx.Unlock() + if db.readOnly { + return nil, 0, errReadOnly + } + return db.MultiTree.ApplyChangeSet(changeSets, updateCommitInfo) } @@ -793,6 +866,15 @@ func atomicRemoveDir(path string) error { return os.RemoveAll(tmpPath) } +// createDBIfNotExist detects if db does not exist and try to initialize an empty one. +func createDBIfNotExist(dir string, initialVersion uint32) error { + _, err := os.Stat(filepath.Join(dir, "current", MetadataFileName)) + if err != nil && os.IsNotExist(err) { + return initEmptyDB(dir, initialVersion) + } + return nil +} + type walEntry struct { index uint64 data *WALEntry diff --git a/memiavl/db_test.go b/memiavl/db_test.go index 71c2f50388..22a1c7ad40 100644 --- a/memiavl/db_test.go +++ b/memiavl/db_test.go @@ -72,8 +72,8 @@ func TestRewriteSnapshotBackground(t *testing.T) { entries, err := os.ReadDir(db.dir) require.NoError(t, err) - // three files: snapshot, current link, wal - require.Equal(t, 3, len(entries)) + // three files: snapshot, current link, wal, LOCK + require.Equal(t, 4, len(entries)) } func TestWAL(t *testing.T) { @@ -230,6 +230,7 @@ func TestLoadVersion(t *testing.T) { } tmp, err := Load(dir, Options{ TargetVersion: uint32(v), + ReadOnly: true, }) require.NoError(t, err) require.Equal(t, RefHashes[v-1], tmp.TreeByName("test").RootHash()) @@ -313,3 +314,41 @@ func TestEmptyValue(t *testing.T) { require.Equal(t, version, db.Version()) require.Equal(t, hash, db.LastCommitInfo().CommitID().Hash) } + +func TestInvalidOptions(t *testing.T) { + dir := t.TempDir() + + _, err := Load(dir, Options{ReadOnly: true}) + require.Error(t, err) + + _, err = Load(dir, Options{ReadOnly: true, CreateIfMissing: true}) + require.Error(t, err) + + db, err := Load(dir, Options{CreateIfMissing: true}) + require.NoError(t, err) + require.NoError(t, db.Close()) + + _, err = Load(dir, Options{LoadForOverwriting: true, ReadOnly: true}) + require.Error(t, err) + + _, err = Load(dir, Options{ReadOnly: true}) + require.NoError(t, err) +} + +func TestExclusiveLock(t *testing.T) { + dir := t.TempDir() + + db, err := Load(dir, Options{CreateIfMissing: true}) + require.NoError(t, err) + + _, err = Load(dir, Options{}) + require.Error(t, err) + + _, err = Load(dir, Options{ReadOnly: true}) + require.NoError(t, err) + + require.NoError(t, db.Close()) + + _, err = Load(dir, Options{}) + require.NoError(t, err) +} diff --git a/memiavl/export.go b/memiavl/export.go index daa9df34bb..a1ed54a1bf 100644 --- a/memiavl/export.go +++ b/memiavl/export.go @@ -28,6 +28,7 @@ func (db *DB) Snapshot(height uint64, protoWriter protoio.Writer) (returnErr err db, err := Load(db.dir, Options{ TargetVersion: version, ZeroCopy: true, + ReadOnly: true, }) if err != nil { return errors.Wrapf(err, "invalid height: %d", height) diff --git a/memiavl/filelock.go b/memiavl/filelock.go new file mode 100644 index 0000000000..2ee406543d --- /dev/null +++ b/memiavl/filelock.go @@ -0,0 +1,27 @@ +package memiavl + +import ( + "path/filepath" + + "github.com/zbiljic/go-filelock" +) + +type FileLock interface { + Unlock() error +} + +func LockFile(fname string) (FileLock, error) { + path, err := filepath.Abs(fname) + if err != nil { + return nil, err + } + fl, err := filelock.New(path) + if err != nil { + return nil, err + } + if _, err := fl.TryLock(); err != nil { + return nil, err + } + + return fl, nil +} diff --git a/memiavl/go.mod b/memiavl/go.mod index 00db7cdd9c..476607a473 100644 --- a/memiavl/go.mod +++ b/memiavl/go.mod @@ -17,6 +17,7 @@ require ( github.com/tidwall/btree v1.5.0 github.com/tidwall/gjson v1.10.2 github.com/tidwall/wal v1.1.7 + github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d golang.org/x/exp v0.0.0-20230310171629-522b1b587ee0 golang.org/x/sync v0.1.0 golang.org/x/sys v0.7.0 diff --git a/memiavl/go.sum b/memiavl/go.sum index 0299ff227e..44268cc1d5 100644 --- a/memiavl/go.sum +++ b/memiavl/go.sum @@ -384,6 +384,8 @@ github.com/yudai/golcs v0.0.0-20170316035057-ecda9a501e82/go.mod h1:lgjkn3NuSvDf github.com/yudai/pp v2.0.1+incompatible/go.mod h1:PuxR/8QJ7cyCkFp/aUDS+JY727OFEZkTdatxwunjIkc= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d h1:XQyeLr7N9iY9mi+TGgsBFkj54+j3fdoo8e2u6zrGP5A= +github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d/go.mod h1:hoMeDjlNXTNqVwrCk8YDyaBS2g5vFfEX2ezMi4vb6CY= github.com/zondax/hid v0.9.1 h1:gQe66rtmyZ8VeGFcOpbuH3r7erYtNEAezCAYu8LdkJo= github.com/zondax/ledger-go v0.14.1 h1:Pip65OOl4iJ84WTpA4BKChvOufMhhbxED3BaihoZN4c= go.etcd.io/bbolt v1.3.6 h1:/ecaJf0sk1l4l6V4awd65v2C3ILy7MSj+s/x1ADCIMU= diff --git a/store/go.mod b/store/go.mod index 76aefda853..69076725ce 100644 --- a/store/go.mod +++ b/store/go.mod @@ -111,6 +111,7 @@ require ( github.com/tidwall/pretty v1.2.0 // indirect github.com/tidwall/tinylru v1.1.0 // indirect github.com/tidwall/wal v1.1.7 // indirect + github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d // indirect github.com/zondax/hid v0.9.1 // indirect github.com/zondax/ledger-go v0.14.1 // indirect go.etcd.io/bbolt v1.3.6 // indirect diff --git a/store/go.sum b/store/go.sum index 0019a9bf9c..e967cc1f13 100644 --- a/store/go.sum +++ b/store/go.sum @@ -607,6 +607,8 @@ github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d h1:XQyeLr7N9iY9mi+TGgsBFkj54+j3fdoo8e2u6zrGP5A= +github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d/go.mod h1:hoMeDjlNXTNqVwrCk8YDyaBS2g5vFfEX2ezMi4vb6CY= github.com/zondax/hid v0.9.1 h1:gQe66rtmyZ8VeGFcOpbuH3r7erYtNEAezCAYu8LdkJo= github.com/zondax/hid v0.9.1/go.mod h1:l5wttcP0jwtdLjqjMMWFVEE7d1zO0jvSPA9OPZxWpEM= github.com/zondax/ledger-go v0.14.1 h1:Pip65OOl4iJ84WTpA4BKChvOufMhhbxED3BaihoZN4c= diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index b529aeb32a..53af53a46a 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -467,7 +467,7 @@ func (rs *Store) Query(req abci.RequestQuery) abci.ResponseQuery { db := rs.db if version != rs.lastCommitInfo.Version { var err error - db, err = memiavl.Load(rs.dir, memiavl.Options{TargetVersion: uint32(version)}) + db, err = memiavl.Load(rs.dir, memiavl.Options{TargetVersion: uint32(version), ReadOnly: true}) if err != nil { return sdkerrors.QueryResult(err, false) } diff --git a/versiondb/go.mod b/versiondb/go.mod index 6418e0f7cf..6d0780cec6 100644 --- a/versiondb/go.mod +++ b/versiondb/go.mod @@ -128,6 +128,7 @@ require ( github.com/tidwall/pretty v1.2.0 // indirect github.com/tidwall/tinylru v1.1.0 // indirect github.com/tidwall/wal v1.1.7 // indirect + github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d // indirect github.com/zondax/hid v0.9.1 // indirect github.com/zondax/ledger-go v0.14.1 // indirect go.etcd.io/bbolt v1.3.7 // indirect diff --git a/versiondb/go.sum b/versiondb/go.sum index f4d7b0adde..7c4d1d08ad 100644 --- a/versiondb/go.sum +++ b/versiondb/go.sum @@ -1028,6 +1028,8 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= +github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d h1:XQyeLr7N9iY9mi+TGgsBFkj54+j3fdoo8e2u6zrGP5A= +github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d/go.mod h1:hoMeDjlNXTNqVwrCk8YDyaBS2g5vFfEX2ezMi4vb6CY= github.com/zondax/hid v0.9.1 h1:gQe66rtmyZ8VeGFcOpbuH3r7erYtNEAezCAYu8LdkJo= github.com/zondax/hid v0.9.1/go.mod h1:l5wttcP0jwtdLjqjMMWFVEE7d1zO0jvSPA9OPZxWpEM= github.com/zondax/ledger-go v0.14.1 h1:Pip65OOl4iJ84WTpA4BKChvOufMhhbxED3BaihoZN4c=