Problem: memiavl minQueryState logic is unnesserary (#1064)

* Problem: memiavl minQueryState logic is unnesserary

Solution:
- simplify the logic, make snapshot-keep-recent defaults to 1, which is enough for ibc relayers to work.

* Update CHANGELOG.md

Signed-off-by: yihuang <huang@crypto.com>

* simpliy ApplyChangeSet

* Update memiavl/multitree.go

Co-authored-by: mmsqe <mavis@crypto.com>
Signed-off-by: yihuang <huang@crypto.com>

* fix

---------

Signed-off-by: yihuang <huang@crypto.com>
Co-authored-by: mmsqe <mavis@crypto.com>

CHANGELOG.md

@@ -46,6 +46,7 @@
 - [#1034](https://github.com/crypto-org-chain/cronos/pull/1034) Support memiavl snapshot strategy configuration.
 - [#1035](https://github.com/crypto-org-chain/cronos/pull/1035) Support caching in memiavl directly, ignore inter-block cache silently.
 - [#1050](https://github.com/crypto-org-chain/cronos/pull/1050) nativebyteorder mode will check endianness on startup, binaries are built with nativebyteorder by default.
+- [#1064](https://github.com/crypto-org-chain/cronos/pull/1064) Simplify memiavl snapshot switching.
 
 *April 13, 2023*
 

app/config/config.go

@@ -22,7 +22,8 @@ type MemIAVLConfig struct {
 	// AsyncCommitBuffer defines the size of asynchronous commit queue, this greatly improve block catching-up
 	// performance, -1 means synchronous commit.
 	AsyncCommitBuffer int `mapstructure:"async-commit-buffer"`
-	// SnapshotKeepRecent defines what many old snapshots (excluding the latest one) to keep after new snapshots are taken.
+	// SnapshotKeepRecent defines what many old snapshots (excluding the latest one) to keep after new snapshots are
+	// taken, defaults to 1 to make sure ibc relayers work.
 	SnapshotKeepRecent uint32 `mapstructure:"snapshot-keep-recent"`
 	// SnapshotInterval defines the block interval the memiavl snapshot is taken, default to 1000.
 	SnapshotInterval uint32 `mapstructure:"snapshot-interval"`
@@ -32,8 +33,9 @@ type MemIAVLConfig struct {
 
 func DefaultMemIAVLConfig() MemIAVLConfig {
 	return MemIAVLConfig{
-		CacheSize:        DefaultCacheSize,
-		SnapshotInterval: memiavl.DefaultSnapshotInterval,
-		ZeroCopy:         true,
+		CacheSize:          DefaultCacheSize,
+		SnapshotInterval:   memiavl.DefaultSnapshotInterval,
+		ZeroCopy:           true,
+		SnapshotKeepRecent: 1,
 	}
 }

app/config/toml.go

@@ -19,7 +19,8 @@ zero-copy = {{ .MemIAVL.ZeroCopy }}
 # performance, -1 means synchronous commit.
 async-commit-buffer = {{ .MemIAVL.AsyncCommitBuffer }}
 
-# SnapshotKeepRecent defines what many old snapshots (excluding the latest one) to keep after new snapshots are taken.
+# SnapshotKeepRecent defines what many old snapshots (excluding the latest one) to keep after new snapshots are
+# taken, defaults to 1 to make sure ibc relayers work.
 snapshot-keep-recent = {{ .MemIAVL.SnapshotKeepRecent }}
 
 # SnapshotInterval defines the block interval the memiavl snapshot is taken, default to 1000.

app/memiavl.go

@@ -33,9 +33,7 @@ func SetupMemIAVL(logger log.Logger, homePath string, appOpts servertypes.AppOpt
 			ZeroCopy:           cast.ToBool(appOpts.Get(FlagZeroCopy)),
 			SnapshotKeepRecent: cast.ToUint32(appOpts.Get(FlagSnapshotKeepRecent)),
 			SnapshotInterval:   cast.ToUint32(appOpts.Get(FlagSnapshotInterval)),
-			// make sure a few queryable states even with pruning="nothing", needed for ibc relayer to work.
-			MinQueryStates: 3,
-			CacheSize:      cast.ToInt(appOpts.Get(FlagCacheSize)),
+			CacheSize:          cast.ToInt(appOpts.Get(FlagCacheSize)),
 		})
 		baseAppOptions = append([]func(*baseapp.BaseApp){setCMS(cms)}, baseAppOptions...)
 	}

memiavl/db.go

@@ -45,9 +45,6 @@ type DB struct {
 	snapshotInterval uint32
 	// make sure only one snapshot rewrite is running
 	pruneSnapshotLock sync.Mutex
-	// might need to wait for a few blocks before actual snapshot switching.
-	minQueryStates   int
-	pendingForSwitch *MultiTree
 
 	// invariant: the LastIndex always match the current version of MultiTree
 	wal         *wal.Log
@@ -83,8 +80,6 @@ type Options struct {
 	AsyncCommitBuffer int
 	// ZeroCopy if true, the get and iterator methods could return a slice pointing to mmaped blob files.
 	ZeroCopy bool
-	// for ibc relayer to work, we need to make sure at least a few queryable states exists even with pruning="nothing" setting.
-	MinQueryStates int
 	// CacheSize defines the cache's max entry size for each memiavl store.
 	CacheSize int
 }
@@ -147,7 +142,6 @@ func Load(dir string, opts Options) (*DB, error) {
 		walChanSize:        opts.AsyncCommitBuffer,
 		snapshotKeepRecent: opts.SnapshotKeepRecent,
 		snapshotInterval:   opts.SnapshotInterval,
-		minQueryStates:     opts.MinQueryStates,
 	}
 
 	if db.logger == nil {
@@ -236,32 +230,21 @@ func (db *DB) checkBackgroundSnapshotRewrite() error {
 			return fmt.Errorf("background snapshot rewriting failed: %w", result.err)
 		}
 
-		db.pendingForSwitch = result.mtree
-	default:
-	}
-
-	if db.pendingForSwitch == nil {
-		return nil
-	}
-
-	// catchup the remaining wal
-	if err := db.pendingForSwitch.CatchupWAL(db.wal, 0); err != nil {
-		return fmt.Errorf("catchup failed: %w", err)
-	}
+		// catchup the remaining wal
+		if err := result.mtree.CatchupWAL(db.wal, 0); err != nil {
+			return fmt.Errorf("catchup failed: %w", err)
+		}
 
-	// make sure there are at least a few queryable states after switch
-	if db.pendingForSwitch.Version() < db.pendingForSwitch.SnapshotVersion()+int64(db.minQueryStates) {
-		return nil
-	}
+		// do the switch
+		if err := db.reloadMultiTree(result.mtree); err != nil {
+			return fmt.Errorf("switch multitree failed: %w", err)
+		}
+		db.logger.Info("switched to new snapshot", "version", db.MultiTree.Version())
 
-	// do the switch
-	if err := db.reloadMultiTree(db.pendingForSwitch); err != nil {
-		return fmt.Errorf("switch multitree failed: %w", err)
+		db.pruneSnapshots()
+	default:
 	}
-	db.logger.Info("switched to new snapshot", "version", db.MultiTree.Version())
 
-	db.pendingForSwitch = nil
-	db.pruneSnapshots()
 	return nil
 }
 

memiavl/multitree.go

@@ -12,7 +12,6 @@ import (
 	"cosmossdk.io/errors"
 
 	storetypes "github.com/cosmos/cosmos-sdk/store/types"
-	"github.com/cosmos/iavl"
 	"github.com/tidwall/wal"
 	"golang.org/x/exp/slices"
 	"golang.org/x/sync/errgroup"
@@ -253,47 +252,28 @@ func (t *MultiTree) ApplyUpgrades(upgrades []*TreeNameUpgrade) error {
 func (t *MultiTree) ApplyChangeSet(changeSets []*NamedChangeSet, updateCommitInfo bool) ([]byte, int64, error) {
 	version := nextVersion(t.lastCommitInfo.Version, t.initialVersion)
 
-	var (
-		infos   []storetypes.StoreInfo
-		csIndex int
-	)
-	for _, entry := range t.trees {
-		var changeSet iavl.ChangeSet
+	for _, cs := range changeSets {
+		tree := t.trees[t.treesByName[cs.Name]].tree
 
-		if csIndex < len(changeSets) && entry.name == changeSets[csIndex].Name {
-			changeSet = changeSets[csIndex].Changeset
-			csIndex++
-		}
-		hash, v, err := entry.tree.ApplyChangeSet(changeSet, updateCommitInfo)
+		_, v, err := tree.ApplyChangeSet(cs.Changeset, updateCommitInfo)
 		if err != nil {
 			return nil, 0, err
 		}
 		if v != version {
 			return nil, 0, fmt.Errorf("multi tree version don't match(%d != %d)", v, version)
 		}
-		if updateCommitInfo {
-			infos = append(infos, storetypes.StoreInfo{
-				Name: entry.name,
-				CommitId: storetypes.CommitID{
-					Version: v,
-					Hash:    hash,
-				},
-			})
-		}
-	}
-
-	if csIndex != len(changeSets) {
-		return nil, 0, fmt.Errorf("non-exhaustive change sets")
 	}
 
 	t.lastCommitInfo.Version = version
-	t.lastCommitInfo.StoreInfos = infos
 
 	var hash []byte
 	if updateCommitInfo {
-		hash = t.lastCommitInfo.Hash()
+		hash = t.UpdateCommitInfo()
+	} else {
+		t.lastCommitInfo.StoreInfos = []storetypes.StoreInfo{}
 	}
-	return hash, t.lastCommitInfo.Version, nil
+
+	return hash, version, nil
 }
 
 // UpdateCommitInfo update lastCommitInfo based on current status of trees.