From a8f7024ed226c224b3b22faec0ba52b7750bb800 Mon Sep 17 00:00:00 2001 From: yihuang Date: Mon, 7 Jul 2025 10:56:11 +0800 Subject: [PATCH 1/4] fix: panic: sync: unlock of unlocked mutex Update CHANGELOG.md cleanup --- CHANGELOG.md | 1 + batch.go | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b018c8799..db5bcb2ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Bug Fixes - [#1007](https://github.com/cosmos/iavl/pull/1007) Add the extra check for the reformatted root node in `GetNode` +- [#1106](https://github.com/cosmos/iavl/pull/1106) Fix unlock of unlocked mutex panic. ### Improvements diff --git a/batch.go b/batch.go index 7c898a9c9..3c44fae5a 100644 --- a/batch.go +++ b/batch.go @@ -57,11 +57,9 @@ func (b *BatchWithFlusher) Set(key, value []byte) error { return err } if batchSizeAfter > b.flushThreshold { - b.mtx.Unlock() - if err := b.Write(); err != nil { + if err := b.write(); err != nil { return err } - b.mtx.Lock() } return b.batch.Set(key, value) } @@ -79,19 +77,14 @@ func (b *BatchWithFlusher) Delete(key []byte) error { return err } if batchSizeAfter > b.flushThreshold { - b.mtx.Unlock() - if err := b.Write(); err != nil { + if err := b.write(); err != nil { return err } - b.mtx.Lock() } return b.batch.Delete(key) } -func (b *BatchWithFlusher) Write() error { - b.mtx.Lock() - defer b.mtx.Unlock() - +func (b *BatchWithFlusher) write() error { if err := b.batch.Write(); err != nil { return err } @@ -102,6 +95,13 @@ func (b *BatchWithFlusher) Write() error { return nil } +func (b *BatchWithFlusher) Write() error { + b.mtx.Lock() + defer b.mtx.Unlock() + + return b.write() +} + func (b *BatchWithFlusher) WriteSync() error { b.mtx.Lock() defer b.mtx.Unlock() From 1a0fe3ad4e0060bd5d8f2c867d34d607aae97988 Mon Sep 17 00:00:00 2001 From: yihuang Date: Mon, 7 Jul 2025 10:58:35 +0800 Subject: [PATCH 2/4] fix: nodedb find partial latest version in edge cases `BatchWithFlusher` could separate the commit of the same version into multiple batches, when the batches are not persisted atomically, the current latest version method will return an incomplete version and eventually return error in `MutableTree.Load(0)` method. --- CHANGELOG.md | 1 + batch_flusher_test.go | 85 +++++++++++++++++++++++++++++++++++++++++++ nodedb.go | 12 ++++-- 3 files changed, 94 insertions(+), 4 deletions(-) create mode 100644 batch_flusher_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b018c8799..eb6c259dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Bug Fixes - [#1007](https://github.com/cosmos/iavl/pull/1007) Add the extra check for the reformatted root node in `GetNode` +- [#]() nodedb latest version should not set to a partial committed version. ### Improvements diff --git a/batch_flusher_test.go b/batch_flusher_test.go new file mode 100644 index 000000000..2f8784e2b --- /dev/null +++ b/batch_flusher_test.go @@ -0,0 +1,85 @@ +package iavl + +import ( + "fmt" + "math/rand" + "testing" + + dbm "github.com/cosmos/iavl/db" + "github.com/stretchr/testify/require" +) + +type MockDBBatch struct { + dbm.Batch + + // simulate low level system error + err error +} + +func (b *MockDBBatch) Write() error { + if b.err != nil { + return b.err + } + return b.Batch.Write() +} + +func (b *MockDBBatch) WriteSync() error { + if b.err != nil { + return b.err + } + return b.Batch.WriteSync() +} + +type MockDB struct { + dbm.DB + + batchIndex int + // simulate low level system error at i-th batch write + errors map[int]error +} + +func (db *MockDB) NewBatch() dbm.Batch { + err, _ := db.errors[db.batchIndex] + batch := &MockDBBatch{Batch: db.DB.NewBatch(), err: err} + db.batchIndex++ + return batch +} + +func (db *MockDB) NewBatchWithSize(size int) dbm.Batch { + return db.NewBatch() +} + +func TestBatchFlusher(t *testing.T) { + db := &MockDB{DB: dbm.NewMemDB()} + + { + tree := NewMutableTree(db, 10000, true, NewNopLogger()) + v, err := tree.Load() + require.NoError(t, err) + require.Equal(t, int64(0), v) + + // the batch size exceeds the threshold, and persist into db in two batches + for i := 0; i < 1000; i++ { + // random key value pairs + key := []byte(fmt.Sprintf("key-%064d", rand.Intn(100000000))) + value := []byte(fmt.Sprintf("value-%064d", rand.Intn(100000000))) + _, err := tree.Set(key, value) + require.NoError(t, err) + } + + // the first batch write will success, + // the second batch write will fail + db.errors = map[int]error{ + 1: fmt.Errorf("filesystem failure"), + } + _, _, err = tree.SaveVersion() + require.Error(t, err) + } + + { + tree := NewMutableTree(db, 10000, true, NewNopLogger()) + v, err := tree.Load() + require.NoError(t, err) + require.Equal(t, int64(0), v) + } +} diff --git a/nodedb.go b/nodedb.go index 6e9b1e2a4..ec711cd9c 100644 --- a/nodedb.go +++ b/nodedb.go @@ -913,13 +913,17 @@ func (ndb *nodeDB) getLatestVersion() (bool, int64, error) { } defer itr.Close() - if itr.Valid() { + for ; itr.Valid(); itr.Next() { k := itr.Key() var nk []byte nodeKeyFormat.Scan(k, &nk) - latestVersion = GetNodeKey(nk).version - ndb.resetLatestVersion(latestVersion) - return true, latestVersion, nil + nodeKey := GetNodeKey(nk) + if bytes.Equal(nk, GetRootKey(nodeKey.version)) { + // only find the latest version when we find the root node + latestVersion = nodeKey.version + ndb.resetLatestVersion(latestVersion) + return true, latestVersion, nil + } } if err := itr.Error(); err != nil { From 587d178710f581bbab875d6ea92f9d7a147586c8 Mon Sep 17 00:00:00 2001 From: yihuang Date: Mon, 7 Jul 2025 10:59:54 +0800 Subject: [PATCH 3/4] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb6c259dd..e14a62ab2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ ### Bug Fixes - [#1007](https://github.com/cosmos/iavl/pull/1007) Add the extra check for the reformatted root node in `GetNode` -- [#]() nodedb latest version should not set to a partial committed version. +- [#1107](https://github.com/cosmos/iavl/pull/1107) nodedb latest version should not set to a partial committed version. ### Improvements From fd4c6c9578024e6d3238daecbbbc3db1ec9beb58 Mon Sep 17 00:00:00 2001 From: yihuang Date: Mon, 7 Jul 2025 11:36:08 +0800 Subject: [PATCH 4/4] fix unit test --- batch_flusher_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/batch_flusher_test.go b/batch_flusher_test.go index 2f8784e2b..c0c8ade07 100644 --- a/batch_flusher_test.go +++ b/batch_flusher_test.go @@ -5,12 +5,13 @@ import ( "math/rand" "testing" + corestore "cosmossdk.io/core/store" dbm "github.com/cosmos/iavl/db" "github.com/stretchr/testify/require" ) type MockDBBatch struct { - dbm.Batch + corestore.Batch // simulate low level system error err error @@ -38,14 +39,14 @@ type MockDB struct { errors map[int]error } -func (db *MockDB) NewBatch() dbm.Batch { +func (db *MockDB) NewBatch() corestore.Batch { err, _ := db.errors[db.batchIndex] batch := &MockDBBatch{Batch: db.DB.NewBatch(), err: err} db.batchIndex++ return batch } -func (db *MockDB) NewBatchWithSize(size int) dbm.Batch { +func (db *MockDB) NewBatchWithSize(size int) corestore.Batch { return db.NewBatch() }