From a540b0e84937767919b2c568a8c4e4dbb821f6f3 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 27 Jan 2020 17:20:37 -0800 Subject: [PATCH 01/20] fix commitID along with adding new SnapshotEvery parameter to cleanup disk --- baseapp/baseapp_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 8e2caf466c99..c2af97395e1c 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -83,7 +83,7 @@ func TestMountStores(t *testing.T) { // Test that LoadLatestVersion actually does. func TestLoadVersion(t *testing.T) { logger := defaultLogger() - pruningOpt := SetPruning(store.PruneSyncable) + pruningOpt := SetPruning(store.PruneNothing) db := dbm.NewMemDB() name := t.Name() app := NewBaseApp(name, logger, db, nil, pruningOpt) @@ -293,7 +293,7 @@ func TestAppVersionSetterGetter(t *testing.T) { func TestLoadVersionInvalid(t *testing.T) { logger := log.NewNopLogger() - pruningOpt := SetPruning(store.PruneSyncable) + pruningOpt := SetPruning(store.PruneNothing) db := dbm.NewMemDB() name := t.Name() app := NewBaseApp(name, logger, db, nil, pruningOpt) From 253ad5c13901e09d9ff33d87398ea6fd5d1b16d5 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 27 Jan 2020 17:30:31 -0800 Subject: [PATCH 02/20] fix commitID along with adding new SnapshotEvery parameter to cleanup disk --- simapp/query_test.go | 38 +++++++++++++++++++++++++ store/iavl/store.go | 52 +++++++++++++++++++++++++++++------ store/iavl/store_test.go | 1 + store/rootmulti/store.go | 33 ++++++++++++---------- store/rootmulti/store_test.go | 10 +++---- store/types/pruning.go | 45 ++++++++++++++++++++++++------ 6 files changed, 142 insertions(+), 37 deletions(-) create mode 100644 simapp/query_test.go diff --git a/simapp/query_test.go b/simapp/query_test.go new file mode 100644 index 000000000000..3e4014ee5327 --- /dev/null +++ b/simapp/query_test.go @@ -0,0 +1,38 @@ +package simapp + +import ( + "fmt" + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/supply" + "github.com/stretchr/testify/require" + abci "github.com/tendermint/tendermint/abci/types" +) + +func TestQuery(t *testing.T) { + app := Setup(false) + ctx := app.NewContext(false, abci.Header{Height: app.LastBlockHeight()}) + + chainSupply := supply.Supply{ + Total: sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(10))), + } + + app.SupplyKeeper.SetSupply(ctx, chainSupply) + app.Commit() + app.BeginBlock(abci.RequestBeginBlock{Header: abci.Header{Height: app.LastBlockHeight() + 1}}) + app.Commit() + + retr := app.SupplyKeeper.GetSupply(ctx) + require.Equal(t, &chainSupply, retr, "Get does not work") + + res := app.Query(abci.RequestQuery{ + Path: fmt.Sprintf("store/%s/key", supply.StoreKey), + Data: supply.SupplyKey, + Prove: true, + }) + + fmt.Printf("Res: %#v\n", res) + + require.NotNil(t, res.Value, "Query is nil") +} diff --git a/store/iavl/store.go b/store/iavl/store.go index 66c5e5262209..d1ad3980ecab 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -1,9 +1,11 @@ package iavl import ( + "fmt" "io" "sync" + "github.com/pkg/errors" "github.com/tendermint/iavl" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto/merkle" @@ -29,13 +31,18 @@ var ( // Store Implements types.KVStore and CommitKVStore. type Store struct { - tree Tree + tree Tree + pruning types.PruningOptions + lastCommit types.CommitID } // LoadStore returns an IAVL Store as a CommitKVStore. Internally it will load the // store's version (id) from the provided DB. An error is returned if the version // fails to load. func LoadStore(db dbm.DB, id types.CommitID, pruning types.PruningOptions, lazyLoading bool) (types.CommitKVStore, error) { + if !pruning.IsValid() { + panic(fmt.Sprintf("PruningOptions are invalid: %#v", pruning)) + } tree, err := iavl.NewMutableTreeWithOpts( db, dbm.NewMemDB(), @@ -56,7 +63,18 @@ func LoadStore(db dbm.DB, id types.CommitID, pruning types.PruningOptions, lazyL return nil, err } - return &Store{tree: tree}, nil + lastCommit := types.CommitID{ + Version: tree.Version(), + Hash: tree.Hash(), + } + + store := Store{ + tree: tree, + pruning: pruning, + lastCommit: lastCommit, + } + + return &store, nil } // UnsafeNewStore returns a reference to a new IAVL Store with a given mutable @@ -94,18 +112,34 @@ func (st *Store) Commit() types.CommitID { panic(err) } - return types.CommitID{ - Version: version, - Hash: hash, + flushed := st.pruning.FlushVersion(version) + + // If the version we saved got flushed to disk, check if previous flushed version should be deleted + if flushed { + previous := version - st.pruning.KeepEvery() + // Previous flushed version should only by deleted if previous version is not snapshot version + // OR if snapshotting is disabled (SnapshotEvery == 0) + if !st.pruning.SnapshotVersion(version) { + err := st.tree.DeleteVersion(previous) + if errCause := errors.Cause(err); errCause != nil && errCause != iavl.ErrVersionDoesNotExist { + panic(err) + } + } + } + + if flushed { + st.lastCommit = types.CommitID{ + Version: version, + Hash: hash, + } } + + return st.lastCommit } // Implements Committer. func (st *Store) LastCommitID() types.CommitID { - return types.CommitID{ - Version: st.tree.Version(), - Hash: st.tree.Hash(), - } + return st.lastCommit } // SetPruning panics as pruning options should be provided at initialization diff --git a/store/iavl/store_test.go b/store/iavl/store_test.go index 8f47286e4cb5..4ecd27596936 100644 --- a/store/iavl/store_test.go +++ b/store/iavl/store_test.go @@ -502,6 +502,7 @@ func TestIAVLStoreQuery(t *testing.T) { require.NoError(t, err) iavlStore := UnsafeNewStore(tree) + iavlStore.pruning = types.PruneNothing k1, v1 := []byte("key1"), []byte("val1") k2, v2 := []byte("key2"), []byte("val2") diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index 5239ab8a4e15..96c6fa67277d 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -24,7 +24,7 @@ const ( commitInfoKeyFmt = "s/%d" // s/ ) -// Store is composed of many CommitStores. Name contrasts with +// Store is coNothingmposed of many CommitStores. Name contrasts with // cacheMultiStore which is for cache-wrapping other MultiStores. It implements // the CommitMultiStore interface. type Store struct { @@ -291,20 +291,25 @@ func (rs *Store) Commit() types.CommitID { version := rs.lastCommitID.Version + 1 commitInfo := commitStores(version, rs.stores) - // Need to update atomically. - batch := rs.db.NewBatch() - defer batch.Close() - setCommitInfo(batch, version, commitInfo) - setLatestVersion(batch, version) - batch.Write() - - // Prepare for next version. - commitID := types.CommitID{ - Version: version, - Hash: commitInfo.Hash(), + // update lastCommit only if this version was flushed to disk + if rs.pruningOpts.FlushVersion(version) { + // Need to update atomically. + batch := rs.db.NewBatch() + defer batch.Close() + setCommitInfo(batch, version, commitInfo) + setLatestVersion(batch, version) + batch.Write() + + // Prepare for next version. + commitID := types.CommitID{ + Version: version, + Hash: commitInfo.Hash(), + } + rs.lastCommitID = commitID + return commitID + } else { + return rs.lastCommitID } - rs.lastCommitID = commitID - return commitID } // Implements CacheWrapper/Store/CommitStore. diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index 56111436bd22..8c7d71c37ba4 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -53,7 +53,7 @@ func TestStoreMount(t *testing.T) { func TestCacheMultiStoreWithVersion(t *testing.T) { var db dbm.DB = dbm.NewMemDB() - ms := newMultiStoreWithMounts(db, types.PruneSyncable) + ms := newMultiStoreWithMounts(db, types.PruneNothing) err := ms.LoadLatestVersion() require.Nil(t, err) @@ -90,7 +90,7 @@ func TestCacheMultiStoreWithVersion(t *testing.T) { func TestHashStableWithEmptyCommit(t *testing.T) { var db dbm.DB = dbm.NewMemDB() - ms := newMultiStoreWithMounts(db, types.PruneSyncable) + ms := newMultiStoreWithMounts(db, types.PruneNothing) err := ms.LoadLatestVersion() require.Nil(t, err) @@ -114,7 +114,7 @@ func TestHashStableWithEmptyCommit(t *testing.T) { func TestMultistoreCommitLoad(t *testing.T) { var db dbm.DB = dbm.NewMemDB() - store := newMultiStoreWithMounts(db, types.PruneSyncable) + store := newMultiStoreWithMounts(db, types.PruneNothing) err := store.LoadLatestVersion() require.Nil(t, err) @@ -139,7 +139,7 @@ func TestMultistoreCommitLoad(t *testing.T) { } // Load the latest multistore again and check version. - store = newMultiStoreWithMounts(db, types.PruneSyncable) + store = newMultiStoreWithMounts(db, types.PruneNothing) err = store.LoadLatestVersion() require.Nil(t, err) commitID = getExpectedCommitID(store, nCommits) @@ -152,7 +152,7 @@ func TestMultistoreCommitLoad(t *testing.T) { // Load an older multistore and check version. ver := nCommits - 1 - store = newMultiStoreWithMounts(db, types.PruneSyncable) + store = newMultiStoreWithMounts(db, types.PruneNothing) err = store.LoadVersion(ver) require.Nil(t, err) commitID = getExpectedCommitID(store, ver) diff --git a/store/types/pruning.go b/store/types/pruning.go index cd4f19b61689..c76df4a1f7bb 100644 --- a/store/types/pruning.go +++ b/store/types/pruning.go @@ -3,17 +3,40 @@ package types // PruningStrategy specifies how old states will be deleted over time where // keepRecent can be used with keepEvery to create a pruning "strategy". type PruningOptions struct { - keepRecent int64 - keepEvery int64 + keepRecent int64 + keepEvery int64 + snapshotEvery int64 } -func NewPruningOptions(keepRecent, keepEvery int64) PruningOptions { +func NewPruningOptions(keepRecent, keepEvery, snapshotEvery int64) PruningOptions { return PruningOptions{ - keepRecent: keepRecent, - keepEvery: keepEvery, + keepRecent: keepRecent, + keepEvery: keepEvery, + snapshotEvery: snapshotEvery, } } +func (po PruningOptions) IsValid() bool { + // If we're flushing periodically, we must make in-memory cache less than + // that period to avoid inefficiency and undefined behavior + if po.keepEvery != 0 && po.keepRecent > po.keepEvery { + return false + } + // snapshotEvery must be a multiple of keepEvery + if po.keepEvery == 0 { + return po.snapshotEvery == 0 + } + return po.snapshotEvery%po.keepEvery == 0 +} + +func (po PruningOptions) FlushVersion(ver int64) bool { + return po.keepEvery != 0 && ver%po.keepEvery == 0 +} + +func (po PruningOptions) SnapshotVersion(ver int64) bool { + return po.snapshotEvery != 0 && ver%po.snapshotEvery == 0 +} + // How much recent state will be kept. Older state will be deleted. func (po PruningOptions) KeepRecent() int64 { return po.keepRecent @@ -24,12 +47,16 @@ func (po PruningOptions) KeepEvery() int64 { return po.keepEvery } +func (po PruningOptions) SnapshotEvery() int64 { + return po.snapshotEvery +} + // default pruning strategies var ( // PruneEverything means all saved states will be deleted, storing only the current state - PruneEverything = NewPruningOptions(1, 0) + PruneEverything = NewPruningOptions(0, 1, 0) // PruneNothing means all historic states will be saved, nothing will be deleted - PruneNothing = NewPruningOptions(0, 1) - // PruneSyncable means only those states not needed for state syncing will be deleted (keeps last 100 + every 10000th) - PruneSyncable = NewPruningOptions(100, 10000) + PruneNothing = NewPruningOptions(0, 1, 1) + // PruneSyncable means only those states not needed for state syncing will be deleted (flush every 100 + snapshot every 10000th) + PruneSyncable = NewPruningOptions(1, 100, 10000) ) From f1e5ba950590c5016ade5a74fc65b8763ba43d76 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 27 Jan 2020 17:31:59 -0800 Subject: [PATCH 03/20] remove file from unrelated debugging worl --- simapp/query_test.go | 38 -------------------------------------- 1 file changed, 38 deletions(-) delete mode 100644 simapp/query_test.go diff --git a/simapp/query_test.go b/simapp/query_test.go deleted file mode 100644 index 3e4014ee5327..000000000000 --- a/simapp/query_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package simapp - -import ( - "fmt" - "testing" - - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/supply" - "github.com/stretchr/testify/require" - abci "github.com/tendermint/tendermint/abci/types" -) - -func TestQuery(t *testing.T) { - app := Setup(false) - ctx := app.NewContext(false, abci.Header{Height: app.LastBlockHeight()}) - - chainSupply := supply.Supply{ - Total: sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(10))), - } - - app.SupplyKeeper.SetSupply(ctx, chainSupply) - app.Commit() - app.BeginBlock(abci.RequestBeginBlock{Header: abci.Header{Height: app.LastBlockHeight() + 1}}) - app.Commit() - - retr := app.SupplyKeeper.GetSupply(ctx) - require.Equal(t, &chainSupply, retr, "Get does not work") - - res := app.Query(abci.RequestQuery{ - Path: fmt.Sprintf("store/%s/key", supply.StoreKey), - Data: supply.SupplyKey, - Prove: true, - }) - - fmt.Printf("Res: %#v\n", res) - - require.NotNil(t, res.Value, "Query is nil") -} From 923f1e5947ed0dc089cf01f73f733b1e7f13d706 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 27 Jan 2020 17:34:56 -0800 Subject: [PATCH 04/20] fix cleanup bug --- store/iavl/store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/store/iavl/store.go b/store/iavl/store.go index d1ad3980ecab..9e825bb456a4 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -119,7 +119,7 @@ func (st *Store) Commit() types.CommitID { previous := version - st.pruning.KeepEvery() // Previous flushed version should only by deleted if previous version is not snapshot version // OR if snapshotting is disabled (SnapshotEvery == 0) - if !st.pruning.SnapshotVersion(version) { + if !st.pruning.SnapshotVersion(previous) { err := st.tree.DeleteVersion(previous) if errCause := errors.Cause(err); errCause != nil && errCause != iavl.ErrVersionDoesNotExist { panic(err) From a35591bcc913e552d0fa8b84ac3da07356d31f1d Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 28 Jan 2020 12:18:44 -0800 Subject: [PATCH 05/20] add iavl tests --- store/cache/cache_test.go | 6 +- store/iavl/store.go | 17 +++++- store/iavl/store_test.go | 116 ++++++++++++++++++++++++------------- store/prefix/store_test.go | 2 +- 4 files changed, 93 insertions(+), 48 deletions(-) diff --git a/store/cache/cache_test.go b/store/cache/cache_test.go index 93ece75083ed..3fc5f9157dbc 100644 --- a/store/cache/cache_test.go +++ b/store/cache/cache_test.go @@ -20,7 +20,7 @@ func TestGetOrSetStoreCache(t *testing.T) { sKey := types.NewKVStoreKey("test") tree, err := iavl.NewMutableTree(db, 100) require.NoError(t, err) - store := iavlstore.UnsafeNewStore(tree) + store := iavlstore.UnsafeNewStore(tree, types.PruneNothing) store2 := mngr.GetStoreCache(sKey, store) require.NotNil(t, store2) @@ -34,7 +34,7 @@ func TestUnwrap(t *testing.T) { sKey := types.NewKVStoreKey("test") tree, err := iavl.NewMutableTree(db, 100) require.NoError(t, err) - store := iavlstore.UnsafeNewStore(tree) + store := iavlstore.UnsafeNewStore(tree, types.PruneNothing) _ = mngr.GetStoreCache(sKey, store) require.Equal(t, store, mngr.Unwrap(sKey)) @@ -48,7 +48,7 @@ func TestStoreCache(t *testing.T) { sKey := types.NewKVStoreKey("test") tree, err := iavl.NewMutableTree(db, 100) require.NoError(t, err) - store := iavlstore.UnsafeNewStore(tree) + store := iavlstore.UnsafeNewStore(tree, types.PruneNothing) kvStore := mngr.GetStoreCache(sKey, store) for i := uint(0); i < cache.DefaultCommitKVStoreCacheSize*2; i++ { diff --git a/store/iavl/store.go b/store/iavl/store.go index 9e825bb456a4..5f7a2d2c0d95 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -81,8 +81,19 @@ func LoadStore(db dbm.DB, id types.CommitID, pruning types.PruningOptions, lazyL // IAVL tree reference. // // CONTRACT: The IAVL tree should be fully loaded. -func UnsafeNewStore(tree *iavl.MutableTree) *Store { - return &Store{tree: tree} +// CONTRACT: PruningOptions passed in as argument must be the same as pruning options +// passed into iavl.MutableTree +func UnsafeNewStore(tree *iavl.MutableTree, po types.PruningOptions) *Store { + commit := types.CommitID{ + Version: tree.Version(), + Hash: tree.Hash(), + } + + return &Store{ + tree: tree, + pruning: po, + lastCommit: commit, + } } // GetImmutable returns a reference to a new store backed by an immutable IAVL @@ -119,7 +130,7 @@ func (st *Store) Commit() types.CommitID { previous := version - st.pruning.KeepEvery() // Previous flushed version should only by deleted if previous version is not snapshot version // OR if snapshotting is disabled (SnapshotEvery == 0) - if !st.pruning.SnapshotVersion(previous) { + if previous != 0 && !st.pruning.SnapshotVersion(previous) { err := st.tree.DeleteVersion(previous) if errCause := errors.Cause(err); errCause != nil && errCause != iavl.ErrVersionDoesNotExist { panic(err) diff --git a/store/iavl/store_test.go b/store/iavl/store_test.go index 4ecd27596936..c1158fc71f98 100644 --- a/store/iavl/store_test.go +++ b/store/iavl/store_test.go @@ -52,7 +52,7 @@ func newAlohaTree(t *testing.T, db dbm.DB) (*iavl.MutableTree, types.CommitID) { func TestGetImmutable(t *testing.T) { db := dbm.NewMemDB() tree, cID := newAlohaTree(t, db) - store := UnsafeNewStore(tree) + store := UnsafeNewStore(tree, types.PruneNothing) require.True(t, tree.Set([]byte("hello"), []byte("adios"))) hash, ver, err := tree.SaveVersion() @@ -82,7 +82,7 @@ func TestGetImmutable(t *testing.T) { func TestTestGetImmutableIterator(t *testing.T) { db := dbm.NewMemDB() tree, cID := newAlohaTree(t, db) - store := UnsafeNewStore(tree) + store := UnsafeNewStore(tree, types.PruneNothing) newStore, err := store.GetImmutable(cID.Version) require.NoError(t, err) @@ -105,7 +105,7 @@ func TestTestGetImmutableIterator(t *testing.T) { func TestIAVLStoreGetSetHasDelete(t *testing.T) { db := dbm.NewMemDB() tree, _ := newAlohaTree(t, db) - iavlStore := UnsafeNewStore(tree) + iavlStore := UnsafeNewStore(tree, types.PruneNothing) key := "hello" @@ -130,14 +130,14 @@ func TestIAVLStoreGetSetHasDelete(t *testing.T) { func TestIAVLStoreNoNilSet(t *testing.T) { db := dbm.NewMemDB() tree, _ := newAlohaTree(t, db) - iavlStore := UnsafeNewStore(tree) + iavlStore := UnsafeNewStore(tree, types.PruneNothing) require.Panics(t, func() { iavlStore.Set([]byte("key"), nil) }, "setting a nil value should panic") } func TestIAVLIterator(t *testing.T) { db := dbm.NewMemDB() tree, _ := newAlohaTree(t, db) - iavlStore := UnsafeNewStore(tree) + iavlStore := UnsafeNewStore(tree, types.PruneNothing) iter := iavlStore.Iterator([]byte("aloha"), []byte("hellz")) expected := []string{"aloha", "hello"} var i int @@ -213,7 +213,7 @@ func TestIAVLReverseIterator(t *testing.T) { tree, err := iavl.NewMutableTree(db, cacheSize) require.NoError(t, err) - iavlStore := UnsafeNewStore(tree) + iavlStore := UnsafeNewStore(tree, types.PruneNothing) iavlStore.Set([]byte{0x00}, []byte("0")) iavlStore.Set([]byte{0x00, 0x00}, []byte("0 0")) @@ -246,7 +246,7 @@ func TestIAVLPrefixIterator(t *testing.T) { tree, err := iavl.NewMutableTree(db, cacheSize) require.NoError(t, err) - iavlStore := UnsafeNewStore(tree) + iavlStore := UnsafeNewStore(tree, types.PruneNothing) iavlStore.Set([]byte("test1"), []byte("test1")) iavlStore.Set([]byte("test2"), []byte("test2")) @@ -310,7 +310,7 @@ func TestIAVLReversePrefixIterator(t *testing.T) { tree, err := iavl.NewMutableTree(db, cacheSize) require.NoError(t, err) - iavlStore := UnsafeNewStore(tree) + iavlStore := UnsafeNewStore(tree, types.PruneNothing) iavlStore.Set([]byte("test1"), []byte("test1")) iavlStore.Set([]byte("test2"), []byte("test2")) @@ -375,7 +375,7 @@ func nextVersion(iavl *Store) { func TestIAVLDefaultPruning(t *testing.T) { //Expected stored / deleted version numbers for: - //numRecent = 5, storeEvery = 3 + //numRecent = 5, storeEvery = 3, snapshotEvery = 6 var states = []pruneState{ {[]int64{}, []int64{}}, {[]int64{1}, []int64{}}, @@ -383,23 +383,23 @@ func TestIAVLDefaultPruning(t *testing.T) { {[]int64{1, 2, 3}, []int64{}}, {[]int64{1, 2, 3, 4}, []int64{}}, {[]int64{1, 2, 3, 4, 5}, []int64{}}, - {[]int64{2, 3, 4, 5, 6}, []int64{1}}, - {[]int64{3, 4, 5, 6, 7}, []int64{1, 2}}, - {[]int64{3, 4, 5, 6, 7, 8}, []int64{1, 2}}, - {[]int64{3, 5, 6, 7, 8, 9}, []int64{1, 2, 4}}, - {[]int64{3, 6, 7, 8, 9, 10}, []int64{1, 2, 4, 5}}, - {[]int64{3, 6, 7, 8, 9, 10, 11}, []int64{1, 2, 4, 5}}, - {[]int64{3, 6, 8, 9, 10, 11, 12}, []int64{1, 2, 4, 5, 7}}, - {[]int64{3, 6, 9, 10, 11, 12, 13}, []int64{1, 2, 4, 5, 7, 8}}, - {[]int64{3, 6, 9, 10, 11, 12, 13, 14}, []int64{1, 2, 4, 5, 7, 8}}, - {[]int64{3, 6, 9, 11, 12, 13, 14, 15}, []int64{1, 2, 4, 5, 7, 8, 10}}, - } - testPruning(t, int64(5), int64(3), states) + {[]int64{2, 4, 5, 6}, []int64{1, 3}}, + {[]int64{4, 5, 6, 7}, []int64{1, 2, 3}}, + {[]int64{4, 5, 6, 7, 8}, []int64{1, 2, 3}}, + {[]int64{5, 6, 7, 8, 9}, []int64{1, 2, 3, 4}}, + {[]int64{6, 7, 8, 9, 10}, []int64{1, 2, 3, 4, 5}}, + {[]int64{6, 7, 8, 9, 10, 11}, []int64{1, 2, 3, 4, 5}}, + {[]int64{6, 8, 10, 11, 12}, []int64{1, 2, 3, 4, 5, 7, 9}}, + {[]int64{6, 10, 11, 12, 13}, []int64{1, 2, 3, 4, 5, 7, 8, 9}}, + {[]int64{6, 10, 11, 12, 13, 14}, []int64{1, 2, 3, 4, 5, 7, 8, 9}}, + {[]int64{6, 11, 12, 13, 14, 15}, []int64{1, 2, 3, 4, 5, 7, 8, 9, 10}}, + } + testPruning(t, int64(5), int64(3), int64(6), states) } func TestIAVLAlternativePruning(t *testing.T) { //Expected stored / deleted version numbers for: - //numRecent = 3, storeEvery = 5 + //numRecent = 3, storeEvery = 5, snapshotEvery = 10 var states = []pruneState{ {[]int64{}, []int64{}}, {[]int64{1}, []int64{}}, @@ -411,14 +411,14 @@ func TestIAVLAlternativePruning(t *testing.T) { {[]int64{5, 6, 7}, []int64{1, 2, 3, 4}}, {[]int64{5, 6, 7, 8}, []int64{1, 2, 3, 4}}, {[]int64{5, 7, 8, 9}, []int64{1, 2, 3, 4, 6}}, - {[]int64{5, 8, 9, 10}, []int64{1, 2, 3, 4, 6, 7}}, - {[]int64{5, 9, 10, 11}, []int64{1, 2, 3, 4, 6, 7, 8}}, - {[]int64{5, 10, 11, 12}, []int64{1, 2, 3, 4, 6, 7, 8, 9}}, - {[]int64{5, 10, 11, 12, 13}, []int64{1, 2, 3, 4, 6, 7, 8, 9}}, - {[]int64{5, 10, 12, 13, 14}, []int64{1, 2, 3, 4, 6, 7, 8, 9, 11}}, - {[]int64{5, 10, 13, 14, 15}, []int64{1, 2, 3, 4, 6, 7, 8, 9, 11, 12}}, - } - testPruning(t, int64(3), int64(5), states) + {[]int64{8, 9, 10}, []int64{1, 2, 3, 4, 6, 7}}, + {[]int64{9, 10, 11}, []int64{1, 2, 3, 4, 6, 7, 8}}, + {[]int64{10, 11, 12}, []int64{1, 2, 3, 4, 5, 6, 7, 8, 9}}, + {[]int64{10, 11, 12, 13}, []int64{1, 2, 3, 4, 5, 6, 7, 8, 9}}, + {[]int64{10, 12, 13, 14}, []int64{1, 2, 3, 4, 5, 6, 7, 8, 9, 11}}, + {[]int64{10, 13, 14, 15}, []int64{1, 2, 3, 4, 5, 6, 7, 8, 9, 11, 12}}, + } + testPruning(t, int64(3), int64(5), int64(10), states) } type pruneState struct { @@ -426,26 +426,27 @@ type pruneState struct { deleted []int64 } -func testPruning(t *testing.T, numRecent int64, storeEvery int64, states []pruneState) { +func testPruning(t *testing.T, numRecent int64, storeEvery int64, snapshotEvery int64, states []pruneState) { db := dbm.NewMemDB() + pruningOpts := types.NewPruningOptions(numRecent, storeEvery, snapshotEvery) iavlOpts := iavl.PruningOptions(storeEvery, numRecent) tree, err := iavl.NewMutableTreeWithOpts(db, dbm.NewMemDB(), cacheSize, iavlOpts) require.NoError(t, err) - iavlStore := UnsafeNewStore(tree) + iavlStore := UnsafeNewStore(tree, pruningOpts) for step, state := range states { for _, ver := range state.stored { require.True(t, iavlStore.VersionExists(ver), - "missing version %d with latest version %d; should save last %d and every %d", - ver, step, numRecent, storeEvery) + "missing version %d with latest version %d; should save last %d, store every %d, and snapshot every %d", + ver, step, numRecent, storeEvery, snapshotEvery) } for _, ver := range state.deleted { require.False(t, iavlStore.VersionExists(ver), - "not pruned version %d with latest version %d; should prune all but last %d and every %d", - ver, step, numRecent, storeEvery) + "not pruned version %d with latest version %d; should prune all but last %d and every %d with intermediate flush interval %d", + ver, step, numRecent, snapshotEvery, storeEvery) } nextVersion(iavlStore) @@ -457,7 +458,7 @@ func TestIAVLNoPrune(t *testing.T) { tree, err := iavl.NewMutableTree(db, cacheSize) require.NoError(t, err) - iavlStore := UnsafeNewStore(tree) + iavlStore := UnsafeNewStore(tree, types.PruneNothing) nextVersion(iavlStore) for i := 1; i < 100; i++ { @@ -478,7 +479,7 @@ func TestIAVLPruneEverything(t *testing.T) { tree, err := iavl.NewMutableTreeWithOpts(db, dbm.NewMemDB(), cacheSize, iavlOpts) require.NoError(t, err) - iavlStore := UnsafeNewStore(tree) + iavlStore := UnsafeNewStore(tree, types.PruneEverything) nextVersion(iavlStore) for i := 1; i < 100; i++ { @@ -496,13 +497,46 @@ func TestIAVLPruneEverything(t *testing.T) { } } +func TestIAVLPruneRestart(t *testing.T) { + db := dbm.NewMemDB() + pruningOpts := types.NewPruningOptions(1, 3, 6) + iavlOpts := iavl.PruningOptions(3, 1) + + tree, err := iavl.NewMutableTreeWithOpts(db, dbm.NewMemDB(), cacheSize, iavlOpts) + require.NoError(t, err) + + iavlStore := UnsafeNewStore(tree, pruningOpts) + initialCommitID := iavlStore.LastCommitID() + + // Create versions without triggering flush + for i := 0; i < 2; i++ { + nextVersion(iavlStore) + } + + unflushedCommitID := iavlStore.LastCommitID() + require.Equal(t, initialCommitID, unflushedCommitID, "CommitID changed before flush") + + // trigger flush to disk with new version + nextVersion(iavlStore) + + // IAVLStore should have flushed to disk on last version: 3 + flushedCommit := iavlStore.LastCommitID() + + // Create new versions without triggering flush + for i := 0; i < 2; i++ { + nextVersion(iavlStore) + } + + postFlushCommit := iavlStore.LastCommitID() + require.Equal(t, flushedCommit, postFlushCommit, "CommitID changed after flush after mem-only commits") +} + func TestIAVLStoreQuery(t *testing.T) { db := dbm.NewMemDB() tree, err := iavl.NewMutableTree(db, cacheSize) require.NoError(t, err) - iavlStore := UnsafeNewStore(tree) - iavlStore.pruning = types.PruneNothing + iavlStore := UnsafeNewStore(tree, types.PruneNothing) k1, v1 := []byte("key1"), []byte("val1") k2, v2 := []byte("key2"), []byte("val2") @@ -601,7 +635,7 @@ func BenchmarkIAVLIteratorNext(b *testing.B) { tree.Set(key, value) } - iavlStore := UnsafeNewStore(tree) + iavlStore := UnsafeNewStore(tree, types.PruneNothing) iterators := make([]types.Iterator, b.N/treeSize) for i := 0; i < len(iterators); i++ { diff --git a/store/prefix/store_test.go b/store/prefix/store_test.go index 8431459748d0..427423340d8f 100644 --- a/store/prefix/store_test.go +++ b/store/prefix/store_test.go @@ -90,7 +90,7 @@ func TestIAVLStorePrefix(t *testing.T) { db := dbm.NewMemDB() tree, err := tiavl.NewMutableTree(db, cacheSize) require.NoError(t, err) - iavlStore := iavl.UnsafeNewStore(tree) + iavlStore := iavl.UnsafeNewStore(tree, types.PruneNothing) testPrefixStore(t, iavlStore, []byte("test")) } From 0ae90d848dc4dccb63701faa96e2ef8e66e0e780 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 28 Jan 2020 16:09:03 -0800 Subject: [PATCH 06/20] add rootmulti tests --- store/rootmulti/store.go | 14 ++++--- store/rootmulti/store_test.go | 71 +++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index 96c6fa67277d..02dd3ef7d6d9 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -29,6 +29,7 @@ const ( // the CommitMultiStore interface. type Store struct { db dbm.DB + version int64 lastCommitID types.CommitID pruningOpts types.PruningOptions storesParams map[types.StoreKey]storeParams @@ -198,6 +199,7 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error { } rs.lastCommitID = lastCommitID + rs.version = lastCommitID.Version rs.stores = newStores return nil @@ -288,21 +290,21 @@ func (rs *Store) LastCommitID() types.CommitID { func (rs *Store) Commit() types.CommitID { // Commit stores. - version := rs.lastCommitID.Version + 1 - commitInfo := commitStores(version, rs.stores) + rs.version = rs.version + 1 + commitInfo := commitStores(rs.version, rs.stores) // update lastCommit only if this version was flushed to disk - if rs.pruningOpts.FlushVersion(version) { + if rs.pruningOpts.FlushVersion(rs.version) { // Need to update atomically. batch := rs.db.NewBatch() defer batch.Close() - setCommitInfo(batch, version, commitInfo) - setLatestVersion(batch, version) + setCommitInfo(batch, rs.version, commitInfo) + setLatestVersion(batch, rs.version) batch.Write() // Prepare for next version. commitID := types.CommitID{ - Version: version, + Version: rs.version, Hash: commitInfo.Hash(), } rs.lastCommitID = commitID diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index 8c7d71c37ba4..286c33defd21 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -1,6 +1,7 @@ package rootmulti import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -289,6 +290,76 @@ func TestParsePath(t *testing.T) { } +func TestMultiStoreRestart(t *testing.T) { + db := dbm.NewMemDB() + pruning := types.NewPruningOptions(1, 3, 6) + multi := newMultiStoreWithMounts(db, pruning) + err := multi.LoadLatestVersion() + require.Nil(t, err) + + initCid := multi.LastCommitID() + + k, v := "wind", "blows" + k2, v2 := "water", "flows" + k3, v3 := "fire", "burns" + + for i := 1; i < 3; i++ { + // Set and commit data in one store. + store1 := multi.getStoreByName("store1").(types.KVStore) + store1.Set([]byte(k), []byte(fmt.Sprintf("%s:%d", v, i))) + + // ... and another. + store2 := multi.getStoreByName("store2").(types.KVStore) + store2.Set([]byte(k2), []byte(fmt.Sprintf("%s:%d", v2, i))) + + // ... and another. + store3 := multi.getStoreByName("store3").(types.KVStore) + store3.Set([]byte(k3), []byte(fmt.Sprintf("%s:%d", v3, i))) + + cid := multi.Commit() + require.Equal(t, initCid, cid) + } + + // Set and commit data in one store. + store1 := multi.getStoreByName("store1").(types.KVStore) + store1.Set([]byte(k), []byte(fmt.Sprintf("%s:%d", v, 3))) + + // ... and another. + store2 := multi.getStoreByName("store2").(types.KVStore) + store2.Set([]byte(k2), []byte(fmt.Sprintf("%s:%d", v2, 3))) + + flushedCid := multi.Commit() + require.NotEqual(t, initCid, flushedCid, "CID is different after flush to disk") + + // ... and another. + store3 := multi.getStoreByName("store3").(types.KVStore) + store3.Set([]byte(k3), []byte(fmt.Sprintf("%s:%d", v3, 3))) + + postFlushCid := multi.Commit() + require.Equal(t, flushedCid, postFlushCid, "Commit changed after in-memory commit") + + multi = newMultiStoreWithMounts(db, pruning) + err = multi.LoadLatestVersion() + require.Nil(t, err) + + reloadedCid := multi.LastCommitID() + require.Equal(t, flushedCid, reloadedCid, "Reloaded CID is not the same as last flushed CID") + + // Check that store1 and store2 retained date from 3rd commit + store1 = multi.getStoreByName("store1").(types.KVStore) + val := store1.Get([]byte(k)) + require.Equal(t, []byte(fmt.Sprintf("%s:%d", v, 3)), val, "Reloaded value not the same as last flushed value") + + store2 = multi.getStoreByName("store2").(types.KVStore) + val2 := store2.Get([]byte(k2)) + require.Equal(t, []byte(fmt.Sprintf("%s:%d", v2, 3)), val2, "Reloaded value not the same as last flushed value") + + // Check that store3 still has data from last commit even though update happened on 2nd commit + store3 = multi.getStoreByName("store3").(types.KVStore) + val3 := store3.Get([]byte(k3)) + require.Equal(t, []byte(fmt.Sprintf("%s:%d", v3, 2)), val3, "Reloaded value not the same as last flushed value") +} + func TestMultiStoreQuery(t *testing.T) { db := dbm.NewMemDB() multi := newMultiStoreWithMounts(db, types.PruneNothing) From d612e56d607979b5f68d515f42fb3ec3fa4707a0 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 28 Jan 2020 20:51:58 -0800 Subject: [PATCH 07/20] add baseapp tests along with Version method on CommitStore --- baseapp/baseapp.go | 2 +- baseapp/baseapp_test.go | 79 ++++++++++++++++++++++++++++++++++++ server/mock/store.go | 4 ++ store/iavl/store.go | 23 ++++++++++- store/rootmulti/dbadapter.go | 4 ++ store/rootmulti/store.go | 4 ++ store/transient/store.go | 7 ++++ store/types/store.go | 1 + 8 files changed, 122 insertions(+), 2 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 3837a34ccc9a..058148261e5f 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -299,7 +299,7 @@ func (app *BaseApp) LastCommitID() sdk.CommitID { // LastBlockHeight returns the last committed block height. func (app *BaseApp) LastBlockHeight() int64 { - return app.cms.LastCommitID().Version + return app.cms.Version() } // initializes the remaining logic from app.cms diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index c2af97395e1c..6712ba5ef867 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -326,6 +326,85 @@ func TestLoadVersionInvalid(t *testing.T) { require.Error(t, err) } +func TestLoadVersionPruning(t *testing.T) { + logger := log.NewNopLogger() + pruningOptions := store.NewPruningOptions(1, 2, 6) + pruningOpt := SetPruning(pruningOptions) + db := dbm.NewMemDB() + name := t.Name() + app := NewBaseApp(name, logger, db, nil, pruningOpt) + + // make a cap key and mount the store + capKey := sdk.NewKVStoreKey(MainStoreKey) + app.MountStores(capKey) + err := app.LoadLatestVersion(capKey) // needed to make stores non-nil + require.Nil(t, err) + + emptyCommitID := sdk.CommitID{} + + // fresh store has zero/empty last commit + lastHeight := app.LastBlockHeight() + lastID := app.LastCommitID() + require.Equal(t, int64(0), lastHeight) + require.Equal(t, emptyCommitID, lastID) + + // execute a block + header := abci.Header{Height: 1} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + res := app.Commit() + + // execute a block, collect commit ID + header = abci.Header{Height: 2} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + res = app.Commit() + commitID2 := sdk.CommitID{Version: 2, Hash: res.Data} + + // execute a block + header = abci.Header{Height: 3} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + res = app.Commit() + commitID3 := sdk.CommitID{Version: 3, Hash: res.Data} + + // reload with LoadLatestVersion, check it loads last flushed version + app = NewBaseApp(name, logger, db, nil, pruningOpt) + app.MountStores(capKey) + err = app.LoadLatestVersion(capKey) + require.Nil(t, err) + testLoadVersionHelper(t, app, int64(2), commitID2) + + // re-execute block 3 and check it is same CommitID + header = abci.Header{Height: 3} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + res = app.Commit() + recommitID3 := sdk.CommitID{Version: 3, Hash: res.Data} + require.Equal(t, commitID3, recommitID3, "Commits of identical blocks not equal after reload") + + // execute a block, collect commit ID + header = abci.Header{Height: 4} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + res = app.Commit() + commitID4 := sdk.CommitID{Version: 4, Hash: res.Data} + + // execute a block + header = abci.Header{Height: 5} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + res = app.Commit() + + // reload with LoadLatestVersion, check it loads last flushed version + app = NewBaseApp(name, logger, db, nil, pruningOpt) + app.MountStores(capKey) + err = app.LoadLatestVersion(capKey) + require.Nil(t, err) + testLoadVersionHelper(t, app, int64(4), commitID4) + + // reload with LoadVersion of previous flushed version + // and check it fails since previous flush should be pruned + app = NewBaseApp(name, logger, db, nil, pruningOpt) + app.MountStores(capKey) + err = app.LoadVersion(2, capKey) + require.NotNil(t, err) +} + func testLoadVersionHelper(t *testing.T, app *BaseApp, expectedHeight int64, expectedID sdk.CommitID) { lastHeight := app.LastBlockHeight() lastID := app.LastCommitID() diff --git a/server/mock/store.go b/server/mock/store.go index bf8e66a4dc07..38f0bc03dca4 100644 --- a/server/mock/store.go +++ b/server/mock/store.go @@ -47,6 +47,10 @@ func (ms multiStore) Commit() sdk.CommitID { panic("not implemented") } +func (ms multiStore) Version() int64 { + panic("not implemented") +} + func (ms multiStore) LastCommitID() sdk.CommitID { panic("not implemented") } diff --git a/store/iavl/store.go b/store/iavl/store.go index 5f7a2d2c0d95..8e802b7a436d 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -33,6 +33,7 @@ var ( type Store struct { tree Tree pruning types.PruningOptions + version int64 lastCommit types.CommitID } @@ -71,6 +72,7 @@ func LoadStore(db dbm.DB, id types.CommitID, pruning types.PruningOptions, lazyL store := Store{ tree: tree, pruning: pruning, + version: tree.Version(), lastCommit: lastCommit, } @@ -92,6 +94,7 @@ func UnsafeNewStore(tree *iavl.MutableTree, po types.PruningOptions) *Store { return &Store{ tree: tree, pruning: po, + version: tree.Version(), lastCommit: commit, } } @@ -111,7 +114,19 @@ func (st *Store) GetImmutable(version int64) (*Store, error) { return nil, err } - return &Store{tree: &immutableTree{iTree}}, nil + commit := types.CommitID{ + Version: version, + Hash: iTree.Hash(), + } + + store := &Store{ + tree: &immutableTree{iTree}, + pruning: st.pruning, + version: version, + lastCommit: commit, + } + + return store, nil } // Implements Committer. @@ -123,6 +138,7 @@ func (st *Store) Commit() types.CommitID { panic(err) } + st.version = version flushed := st.pruning.FlushVersion(version) // If the version we saved got flushed to disk, check if previous flushed version should be deleted @@ -148,6 +164,11 @@ func (st *Store) Commit() types.CommitID { return st.lastCommit } +// Implements Committer +func (st *Store) Version() int64 { + return st.version +} + // Implements Committer. func (st *Store) LastCommitID() types.CommitID { return st.lastCommit diff --git a/store/rootmulti/dbadapter.go b/store/rootmulti/dbadapter.go index cda6e62ba721..5f68d5644150 100644 --- a/store/rootmulti/dbadapter.go +++ b/store/rootmulti/dbadapter.go @@ -16,6 +16,10 @@ type commitDBStoreAdapter struct { dbadapter.Store } +func (cdsa commitDBStoreAdapter) Version() int64 { + return -1 +} + func (cdsa commitDBStoreAdapter) Commit() types.CommitID { return types.CommitID{ Version: -1, diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index 02dd3ef7d6d9..2a04eb78983f 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -281,6 +281,10 @@ func (rs *Store) TracingEnabled() bool { //---------------------------------------- // +CommitStore +func (rs *Store) Version() int64 { + return rs.version +} + // Implements Committer/CommitStore. func (rs *Store) LastCommitID() types.CommitID { return rs.lastCommitID diff --git a/store/transient/store.go b/store/transient/store.go index 488965a43ac5..5e570321d0a3 100644 --- a/store/transient/store.go +++ b/store/transient/store.go @@ -14,6 +14,7 @@ var _ types.KVStore = (*Store)(nil) // Store is a wrapper for a MemDB with Commiter implementation type Store struct { dbadapter.Store + version int64 } // Constructs new MemDB adapter @@ -21,9 +22,15 @@ func NewStore() *Store { return &Store{Store: dbadapter.Store{DB: dbm.NewMemDB()}} } +// Implements Committer +func (st *Store) Version() int64 { + return st.version +} + // Implements CommitStore // Commit cleans up Store. func (ts *Store) Commit() (id types.CommitID) { + ts.version += 1 ts.Store = dbadapter.Store{DB: dbm.NewMemDB()} return } diff --git a/store/types/store.go b/store/types/store.go index 377b43909e11..1aa1f99cddaa 100644 --- a/store/types/store.go +++ b/store/types/store.go @@ -16,6 +16,7 @@ type Store interface { //nolint // something that can persist to disk type Committer interface { + Version() int64 Commit() CommitID LastCommitID() CommitID SetPruning(PruningOptions) From 63bd983b19b3b5e7f745eee28403daa85bdfaf27 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 29 Jan 2020 10:16:01 -0800 Subject: [PATCH 08/20] cleanup from review --- store/iavl/store.go | 29 +++++++++++++++-------------- store/rootmulti/store.go | 33 ++++++++++++++++----------------- store/types/pruning.go | 4 ++++ 3 files changed, 35 insertions(+), 31 deletions(-) diff --git a/store/iavl/store.go b/store/iavl/store.go index 8e802b7a436d..5ae51918b52b 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -141,24 +141,25 @@ func (st *Store) Commit() types.CommitID { st.version = version flushed := st.pruning.FlushVersion(version) + // If this is not a flushed version, return saved lastCommit + if !flushed { + return st.lastCommit + } + // If the version we saved got flushed to disk, check if previous flushed version should be deleted - if flushed { - previous := version - st.pruning.KeepEvery() - // Previous flushed version should only by deleted if previous version is not snapshot version - // OR if snapshotting is disabled (SnapshotEvery == 0) - if previous != 0 && !st.pruning.SnapshotVersion(previous) { - err := st.tree.DeleteVersion(previous) - if errCause := errors.Cause(err); errCause != nil && errCause != iavl.ErrVersionDoesNotExist { - panic(err) - } + previous := version - st.pruning.KeepEvery() + // Previous flushed version should only by deleted if previous version is not snapshot version + // OR if snapshotting is disabled (SnapshotEvery == 0) + if previous != 0 && !st.pruning.SnapshotVersion(previous) { + err := st.tree.DeleteVersion(previous) + if errCause := errors.Cause(err); errCause != nil && errCause != iavl.ErrVersionDoesNotExist { + panic(err) } } - if flushed { - st.lastCommit = types.CommitID{ - Version: version, - Hash: hash, - } + st.lastCommit = types.CommitID{ + Version: version, + Hash: hash, } return st.lastCommit diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index 2a04eb78983f..de81bb334956 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -24,7 +24,7 @@ const ( commitInfoKeyFmt = "s/%d" // s/ ) -// Store is coNothingmposed of many CommitStores. Name contrasts with +// Store is composed of many CommitStores. Name contrasts with // cacheMultiStore which is for cache-wrapping other MultiStores. It implements // the CommitMultiStore interface. type Store struct { @@ -298,24 +298,23 @@ func (rs *Store) Commit() types.CommitID { commitInfo := commitStores(rs.version, rs.stores) // update lastCommit only if this version was flushed to disk - if rs.pruningOpts.FlushVersion(rs.version) { - // Need to update atomically. - batch := rs.db.NewBatch() - defer batch.Close() - setCommitInfo(batch, rs.version, commitInfo) - setLatestVersion(batch, rs.version) - batch.Write() - - // Prepare for next version. - commitID := types.CommitID{ - Version: rs.version, - Hash: commitInfo.Hash(), - } - rs.lastCommitID = commitID - return commitID - } else { + if !rs.pruningOpts.FlushVersion(rs.version) { return rs.lastCommitID } + // Need to update atomically. + batch := rs.db.NewBatch() + defer batch.Close() + setCommitInfo(batch, rs.version, commitInfo) + setLatestVersion(batch, rs.version) + batch.Write() + + // Prepare for next version. + commitID := types.CommitID{ + Version: rs.version, + Hash: commitInfo.Hash(), + } + rs.lastCommitID = commitID + return commitID } // Implements CacheWrapper/Store/CommitStore. diff --git a/store/types/pruning.go b/store/types/pruning.go index c76df4a1f7bb..9b84b7cbe58c 100644 --- a/store/types/pruning.go +++ b/store/types/pruning.go @@ -2,6 +2,7 @@ package types // PruningStrategy specifies how old states will be deleted over time where // keepRecent can be used with keepEvery to create a pruning "strategy". +// CONTRACT: snapshotEvery is a multiple of keepEvery type PruningOptions struct { keepRecent int64 keepEvery int64 @@ -16,6 +17,9 @@ func NewPruningOptions(keepRecent, keepEvery, snapshotEvery int64) PruningOption } } +// Checks if PruningOptions is valid +// KeepRecent should be larger than KeepEvery +// SnapshotEvery is a multiple of KeepEvery func (po PruningOptions) IsValid() bool { // If we're flushing periodically, we must make in-memory cache less than // that period to avoid inefficiency and undefined behavior From fbd1a52fff85d80eae85d27be14c4662816f7b8f Mon Sep 17 00:00:00 2001 From: Aditya Date: Wed, 29 Jan 2020 14:25:47 -0800 Subject: [PATCH 09/20] Update store/types/pruning.go Co-Authored-By: Alexander Bezobchuk --- store/types/pruning.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/store/types/pruning.go b/store/types/pruning.go index 9b84b7cbe58c..f0bf22c33798 100644 --- a/store/types/pruning.go +++ b/store/types/pruning.go @@ -22,7 +22,7 @@ func NewPruningOptions(keepRecent, keepEvery, snapshotEvery int64) PruningOption // SnapshotEvery is a multiple of KeepEvery func (po PruningOptions) IsValid() bool { // If we're flushing periodically, we must make in-memory cache less than - // that period to avoid inefficiency and undefined behavior + // that period to avoid inefficiency and undefined behavior. if po.keepEvery != 0 && po.keepRecent > po.keepEvery { return false } From 42fc8ef0273cafb37c282f41d7e09304de084368 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 30 Jan 2020 16:39:21 -0800 Subject: [PATCH 10/20] initial fixes --- store/iavl/store.go | 80 +++++++++++------------------------ store/iavl/store_test.go | 2 + store/rootmulti/proof_test.go | 4 ++ store/rootmulti/store.go | 30 ++++++------- store/types/store.go | 1 - 5 files changed, 43 insertions(+), 74 deletions(-) diff --git a/store/iavl/store.go b/store/iavl/store.go index 5ae51918b52b..fdb2368ac0ce 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -31,10 +31,8 @@ var ( // Store Implements types.KVStore and CommitKVStore. type Store struct { - tree Tree - pruning types.PruningOptions - version int64 - lastCommit types.CommitID + tree Tree + pruning types.PruningOptions } // LoadStore returns an IAVL Store as a CommitKVStore. Internally it will load the @@ -64,16 +62,9 @@ func LoadStore(db dbm.DB, id types.CommitID, pruning types.PruningOptions, lazyL return nil, err } - lastCommit := types.CommitID{ - Version: tree.Version(), - Hash: tree.Hash(), - } - store := Store{ - tree: tree, - pruning: pruning, - version: tree.Version(), - lastCommit: lastCommit, + tree: tree, + pruning: pruning, } return &store, nil @@ -86,16 +77,9 @@ func LoadStore(db dbm.DB, id types.CommitID, pruning types.PruningOptions, lazyL // CONTRACT: PruningOptions passed in as argument must be the same as pruning options // passed into iavl.MutableTree func UnsafeNewStore(tree *iavl.MutableTree, po types.PruningOptions) *Store { - commit := types.CommitID{ - Version: tree.Version(), - Hash: tree.Hash(), - } - return &Store{ - tree: tree, - pruning: po, - version: tree.Version(), - lastCommit: commit, + tree: tree, + pruning: po, } } @@ -114,16 +98,9 @@ func (st *Store) GetImmutable(version int64) (*Store, error) { return nil, err } - commit := types.CommitID{ - Version: version, - Hash: iTree.Hash(), - } - store := &Store{ - tree: &immutableTree{iTree}, - pruning: st.pruning, - version: version, - lastCommit: commit, + tree: &immutableTree{iTree}, + pruning: st.pruning, } return store, nil @@ -137,42 +114,35 @@ func (st *Store) Commit() types.CommitID { // TODO: Do we want to extend Commit to allow returning errors? panic(err) } + fmt.Printf("Hash: %x\n", hash) + fmt.Println("Leaves:", st.tree.(*iavl.MutableTree).Size()) - st.version = version flushed := st.pruning.FlushVersion(version) - - // If this is not a flushed version, return saved lastCommit - if !flushed { - return st.lastCommit - } - - // If the version we saved got flushed to disk, check if previous flushed version should be deleted - previous := version - st.pruning.KeepEvery() - // Previous flushed version should only by deleted if previous version is not snapshot version - // OR if snapshotting is disabled (SnapshotEvery == 0) - if previous != 0 && !st.pruning.SnapshotVersion(previous) { - err := st.tree.DeleteVersion(previous) - if errCause := errors.Cause(err); errCause != nil && errCause != iavl.ErrVersionDoesNotExist { - panic(err) + if flushed { + // If the version we saved got flushed to disk, check if previous flushed version should be deleted + previous := version - st.pruning.KeepEvery() + // Previous flushed version should only by deleted if previous version is not snapshot version + // OR if snapshotting is disabled (SnapshotEvery == 0) + if previous != 0 && !st.pruning.SnapshotVersion(previous) { + err := st.tree.DeleteVersion(previous) + if errCause := errors.Cause(err); errCause != nil && errCause != iavl.ErrVersionDoesNotExist { + panic(err) + } } } - st.lastCommit = types.CommitID{ + return types.CommitID{ Version: version, Hash: hash, } - - return st.lastCommit -} - -// Implements Committer -func (st *Store) Version() int64 { - return st.version } // Implements Committer. func (st *Store) LastCommitID() types.CommitID { - return st.lastCommit + return types.CommitID{ + Version: st.tree.Version(), + Hash: st.tree.Hash(), + } } // SetPruning panics as pruning options should be provided at initialization diff --git a/store/iavl/store_test.go b/store/iavl/store_test.go index c1158fc71f98..a38068f65a6a 100644 --- a/store/iavl/store_test.go +++ b/store/iavl/store_test.go @@ -497,6 +497,7 @@ func TestIAVLPruneEverything(t *testing.T) { } } +/* func TestIAVLPruneRestart(t *testing.T) { db := dbm.NewMemDB() pruningOpts := types.NewPruningOptions(1, 3, 6) @@ -530,6 +531,7 @@ func TestIAVLPruneRestart(t *testing.T) { postFlushCommit := iavlStore.LastCommitID() require.Equal(t, flushedCommit, postFlushCommit, "CommitID changed after flush after mem-only commits") } +*/ func TestIAVLStoreQuery(t *testing.T) { db := dbm.NewMemDB() diff --git a/store/rootmulti/proof_test.go b/store/rootmulti/proof_test.go index 4d316f2c85aa..3075e3229683 100644 --- a/store/rootmulti/proof_test.go +++ b/store/rootmulti/proof_test.go @@ -1,6 +1,7 @@ package rootmulti import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -63,9 +64,11 @@ func TestVerifyMultiStoreQueryProof(t *testing.T) { store.MountStoreWithDB(iavlStoreKey, types.StoreTypeIAVL, nil) require.NoError(t, store.LoadVersion(0)) + fmt.Println("hello") iavlStore := store.GetCommitStore(iavlStoreKey).(*iavl.Store) iavlStore.Set([]byte("MYKEY"), []byte("MYVALUE")) cid := store.Commit() + fmt.Printf("CID: %v\n", cid) // Get Proof res := store.Query(abci.RequestQuery{ @@ -74,6 +77,7 @@ func TestVerifyMultiStoreQueryProof(t *testing.T) { Prove: true, }) require.NotNil(t, res.Proof) + fmt.Println("bye") // Verify proof. prt := DefaultProofRuntime() diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index de81bb334956..1e9c4f1b57d4 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -29,7 +29,6 @@ const ( // the CommitMultiStore interface. type Store struct { db dbm.DB - version int64 lastCommitID types.CommitID pruningOpts types.PruningOptions storesParams map[types.StoreKey]storeParams @@ -199,7 +198,6 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error { } rs.lastCommitID = lastCommitID - rs.version = lastCommitID.Version rs.stores = newStores return nil @@ -281,10 +279,6 @@ func (rs *Store) TracingEnabled() bool { //---------------------------------------- // +CommitStore -func (rs *Store) Version() int64 { - return rs.version -} - // Implements Committer/CommitStore. func (rs *Store) LastCommitID() types.CommitID { return rs.lastCommitID @@ -294,23 +288,22 @@ func (rs *Store) LastCommitID() types.CommitID { func (rs *Store) Commit() types.CommitID { // Commit stores. - rs.version = rs.version + 1 - commitInfo := commitStores(rs.version, rs.stores) + version := rs.lastCommitID.Version + commitInfo := commitStores(version, rs.stores) - // update lastCommit only if this version was flushed to disk - if !rs.pruningOpts.FlushVersion(rs.version) { - return rs.lastCommitID + // write CommitInfo to disk only if this version was flushed to disk + if rs.pruningOpts.FlushVersion(version) { + // Need to update atomically. + batch := rs.db.NewBatch() + defer batch.Close() + setCommitInfo(batch, version, commitInfo) + setLatestVersion(batch, version) + batch.Write() } - // Need to update atomically. - batch := rs.db.NewBatch() - defer batch.Close() - setCommitInfo(batch, rs.version, commitInfo) - setLatestVersion(batch, rs.version) - batch.Write() // Prepare for next version. commitID := types.CommitID{ - Version: rs.version, + Version: version, Hash: commitInfo.Hash(), } rs.lastCommitID = commitID @@ -452,6 +445,7 @@ func (rs *Store) Query(req abci.RequestQuery) abci.ResponseQuery { } commitInfo, errMsg := getCommitInfo(rs.db, res.Height) + fmt.Printf("commitInfo: %v\n", commitInfo) if errMsg != nil { return sdkerrors.QueryResult(err) } diff --git a/store/types/store.go b/store/types/store.go index 1aa1f99cddaa..377b43909e11 100644 --- a/store/types/store.go +++ b/store/types/store.go @@ -16,7 +16,6 @@ type Store interface { //nolint // something that can persist to disk type Committer interface { - Version() int64 Commit() CommitID LastCommitID() CommitID SetPruning(PruningOptions) From aa81b5da0dc984f04ace7ed025401f1f3b55a4ef Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 30 Jan 2020 21:21:32 -0800 Subject: [PATCH 11/20] fix query bugs by replacing lastCommitID with lastCommitInfo --- store/iavl/store.go | 2 -- store/rootmulti/proof_test.go | 4 --- store/rootmulti/store.go | 46 +++++++++++++++++++---------------- store/rootmulti/store_test.go | 23 ++++++++++++------ 4 files changed, 41 insertions(+), 34 deletions(-) diff --git a/store/iavl/store.go b/store/iavl/store.go index fdb2368ac0ce..0c5c5616bed7 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -114,8 +114,6 @@ func (st *Store) Commit() types.CommitID { // TODO: Do we want to extend Commit to allow returning errors? panic(err) } - fmt.Printf("Hash: %x\n", hash) - fmt.Println("Leaves:", st.tree.(*iavl.MutableTree).Size()) flushed := st.pruning.FlushVersion(version) if flushed { diff --git a/store/rootmulti/proof_test.go b/store/rootmulti/proof_test.go index 3075e3229683..4d316f2c85aa 100644 --- a/store/rootmulti/proof_test.go +++ b/store/rootmulti/proof_test.go @@ -1,7 +1,6 @@ package rootmulti import ( - "fmt" "testing" "github.com/stretchr/testify/require" @@ -64,11 +63,9 @@ func TestVerifyMultiStoreQueryProof(t *testing.T) { store.MountStoreWithDB(iavlStoreKey, types.StoreTypeIAVL, nil) require.NoError(t, store.LoadVersion(0)) - fmt.Println("hello") iavlStore := store.GetCommitStore(iavlStoreKey).(*iavl.Store) iavlStore.Set([]byte("MYKEY"), []byte("MYVALUE")) cid := store.Commit() - fmt.Printf("CID: %v\n", cid) // Get Proof res := store.Query(abci.RequestQuery{ @@ -77,7 +74,6 @@ func TestVerifyMultiStoreQueryProof(t *testing.T) { Prove: true, }) require.NotNil(t, res.Proof) - fmt.Println("bye") // Verify proof. prt := DefaultProofRuntime() diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index 1e9c4f1b57d4..c44db08a5d76 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -28,13 +28,13 @@ const ( // cacheMultiStore which is for cache-wrapping other MultiStores. It implements // the CommitMultiStore interface. type Store struct { - db dbm.DB - lastCommitID types.CommitID - pruningOpts types.PruningOptions - storesParams map[types.StoreKey]storeParams - stores map[types.StoreKey]types.CommitKVStore - keysByName map[string]types.StoreKey - lazyLoading bool + db dbm.DB + lastCommitInfo commitInfo + pruningOpts types.PruningOptions + storesParams map[types.StoreKey]storeParams + stores map[types.StoreKey]types.CommitKVStore + keysByName map[string]types.StoreKey + lazyLoading bool traceWriter io.Writer traceContext types.TraceContext @@ -146,11 +146,12 @@ func (rs *Store) LoadVersion(ver int64) error { func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error { infos := make(map[string]storeInfo) - var lastCommitID types.CommitID + var cInfo commitInfo // load old data if we are not version 0 if ver != 0 { - cInfo, err := getCommitInfo(rs.db, ver) + var err error + cInfo, err = getCommitInfo(rs.db, ver) if err != nil { return err } @@ -159,7 +160,6 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error { for _, storeInfo := range cInfo.StoreInfos { infos[storeInfo.Name] = storeInfo } - lastCommitID = cInfo.CommitID() } // load each Store (note this doesn't panic on unmounted keys now) @@ -197,7 +197,7 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error { } } - rs.lastCommitID = lastCommitID + rs.lastCommitInfo = cInfo rs.stores = newStores return nil @@ -281,22 +281,22 @@ func (rs *Store) TracingEnabled() bool { // Implements Committer/CommitStore. func (rs *Store) LastCommitID() types.CommitID { - return rs.lastCommitID + return rs.lastCommitInfo.CommitID() } // Implements Committer/CommitStore. func (rs *Store) Commit() types.CommitID { // Commit stores. - version := rs.lastCommitID.Version - commitInfo := commitStores(version, rs.stores) + version := rs.lastCommitInfo.Version + 1 + rs.lastCommitInfo = commitStores(version, rs.stores) // write CommitInfo to disk only if this version was flushed to disk if rs.pruningOpts.FlushVersion(version) { // Need to update atomically. batch := rs.db.NewBatch() defer batch.Close() - setCommitInfo(batch, version, commitInfo) + setCommitInfo(batch, version, rs.lastCommitInfo) setLatestVersion(batch, version) batch.Write() } @@ -304,9 +304,8 @@ func (rs *Store) Commit() types.CommitID { // Prepare for next version. commitID := types.CommitID{ Version: version, - Hash: commitInfo.Hash(), + Hash: rs.lastCommitInfo.Hash(), } - rs.lastCommitID = commitID return commitID } @@ -444,10 +443,15 @@ func (rs *Store) Query(req abci.RequestQuery) abci.ResponseQuery { return sdkerrors.QueryResult(sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "proof is unexpectedly empty; ensure height has not been pruned")) } - commitInfo, errMsg := getCommitInfo(rs.db, res.Height) - fmt.Printf("commitInfo: %v\n", commitInfo) - if errMsg != nil { - return sdkerrors.QueryResult(err) + var commitInfo commitInfo + if res.Height == rs.lastCommitInfo.Version { + commitInfo = rs.lastCommitInfo + } else { + var errMsg error + commitInfo, errMsg = getCommitInfo(rs.db, res.Height) + if errMsg != nil { + return sdkerrors.QueryResult(err) + } } // Restore origin path and append proof op. diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index 286c33defd21..0440a975925d 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -316,8 +316,11 @@ func TestMultiStoreRestart(t *testing.T) { store3 := multi.getStoreByName("store3").(types.KVStore) store3.Set([]byte(k3), []byte(fmt.Sprintf("%s:%d", v3, i))) - cid := multi.Commit() - require.Equal(t, initCid, cid) + multi.Commit() + + cinfo, err := getCommitInfo(multi.db, int64(i)) + require.NotNil(t, err) + require.Equal(t, commitInfo{}, cinfo) } // Set and commit data in one store. @@ -328,22 +331,28 @@ func TestMultiStoreRestart(t *testing.T) { store2 := multi.getStoreByName("store2").(types.KVStore) store2.Set([]byte(k2), []byte(fmt.Sprintf("%s:%d", v2, 3))) - flushedCid := multi.Commit() - require.NotEqual(t, initCid, flushedCid, "CID is different after flush to disk") + multi.Commit() + + flushedCinfo, err := getCommitInfo(multi.db, 3) + require.Nil(t, err) + require.NotEqual(t, initCid, flushedCinfo, "CID is different after flush to disk") // ... and another. store3 := multi.getStoreByName("store3").(types.KVStore) store3.Set([]byte(k3), []byte(fmt.Sprintf("%s:%d", v3, 3))) - postFlushCid := multi.Commit() - require.Equal(t, flushedCid, postFlushCid, "Commit changed after in-memory commit") + multi.Commit() + + postFlushCinfo, err := getCommitInfo(multi.db, 4) + require.NotNil(t, err) + require.Equal(t, commitInfo{}, postFlushCinfo, "Commit changed after in-memory commit") multi = newMultiStoreWithMounts(db, pruning) err = multi.LoadLatestVersion() require.Nil(t, err) reloadedCid := multi.LastCommitID() - require.Equal(t, flushedCid, reloadedCid, "Reloaded CID is not the same as last flushed CID") + require.Equal(t, flushedCinfo.CommitID(), reloadedCid, "Reloaded CID is not the same as last flushed CID") // Check that store1 and store2 retained date from 3rd commit store1 = multi.getStoreByName("store1").(types.KVStore) From c9c0fd12b0bd4b319c8af64be63bd10e51b4ee4c Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 31 Jan 2020 11:41:37 -0800 Subject: [PATCH 12/20] start addressing review comments --- baseapp/baseapp.go | 2 +- store/iavl/store.go | 2 +- store/rootmulti/dbadapter.go | 4 ---- store/transient/store.go | 7 ------- 4 files changed, 2 insertions(+), 13 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 058148261e5f..3837a34ccc9a 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -299,7 +299,7 @@ func (app *BaseApp) LastCommitID() sdk.CommitID { // LastBlockHeight returns the last committed block height. func (app *BaseApp) LastBlockHeight() int64 { - return app.cms.Version() + return app.cms.LastCommitID().Version } // initializes the remaining logic from app.cms diff --git a/store/iavl/store.go b/store/iavl/store.go index 0c5c5616bed7..1a5503aa00be 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -40,7 +40,7 @@ type Store struct { // fails to load. func LoadStore(db dbm.DB, id types.CommitID, pruning types.PruningOptions, lazyLoading bool) (types.CommitKVStore, error) { if !pruning.IsValid() { - panic(fmt.Sprintf("PruningOptions are invalid: %#v", pruning)) + return nil, fmt.Errorf("pruning options are invalid: %v", pruning) } tree, err := iavl.NewMutableTreeWithOpts( db, diff --git a/store/rootmulti/dbadapter.go b/store/rootmulti/dbadapter.go index 5f68d5644150..cda6e62ba721 100644 --- a/store/rootmulti/dbadapter.go +++ b/store/rootmulti/dbadapter.go @@ -16,10 +16,6 @@ type commitDBStoreAdapter struct { dbadapter.Store } -func (cdsa commitDBStoreAdapter) Version() int64 { - return -1 -} - func (cdsa commitDBStoreAdapter) Commit() types.CommitID { return types.CommitID{ Version: -1, diff --git a/store/transient/store.go b/store/transient/store.go index 5e570321d0a3..488965a43ac5 100644 --- a/store/transient/store.go +++ b/store/transient/store.go @@ -14,7 +14,6 @@ var _ types.KVStore = (*Store)(nil) // Store is a wrapper for a MemDB with Commiter implementation type Store struct { dbadapter.Store - version int64 } // Constructs new MemDB adapter @@ -22,15 +21,9 @@ func NewStore() *Store { return &Store{Store: dbadapter.Store{DB: dbm.NewMemDB()}} } -// Implements Committer -func (st *Store) Version() int64 { - return st.version -} - // Implements CommitStore // Commit cleans up Store. func (ts *Store) Commit() (id types.CommitID) { - ts.version += 1 ts.Store = dbadapter.Store{DB: dbm.NewMemDB()} return } From f0411628a76c9fb0f78046a6f6f02ba6ba508206 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 31 Jan 2020 12:23:33 -0800 Subject: [PATCH 13/20] further cleanup --- server/mock/store.go | 4 ---- store/iavl/store_test.go | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/server/mock/store.go b/server/mock/store.go index 38f0bc03dca4..bf8e66a4dc07 100644 --- a/server/mock/store.go +++ b/server/mock/store.go @@ -47,10 +47,6 @@ func (ms multiStore) Commit() sdk.CommitID { panic("not implemented") } -func (ms multiStore) Version() int64 { - panic("not implemented") -} - func (ms multiStore) LastCommitID() sdk.CommitID { panic("not implemented") } diff --git a/store/iavl/store_test.go b/store/iavl/store_test.go index a38068f65a6a..08e04aa6c7b2 100644 --- a/store/iavl/store_test.go +++ b/store/iavl/store_test.go @@ -375,7 +375,7 @@ func nextVersion(iavl *Store) { func TestIAVLDefaultPruning(t *testing.T) { //Expected stored / deleted version numbers for: - //numRecent = 5, storeEvery = 3, snapshotEvery = 6 + //numRecent = 5, storeEvery = 3, snapshotEvery = 5 var states = []pruneState{ {[]int64{}, []int64{}}, {[]int64{1}, []int64{}}, From d52da2caa5d5d1af588f9957ccc81dcc25421990 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 4 Feb 2020 16:23:22 -0800 Subject: [PATCH 14/20] enforce keepRecent is either 0 or 1 --- store/types/pruning.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/store/types/pruning.go b/store/types/pruning.go index f0bf22c33798..789a3400d6fc 100644 --- a/store/types/pruning.go +++ b/store/types/pruning.go @@ -21,9 +21,13 @@ func NewPruningOptions(keepRecent, keepEvery, snapshotEvery int64) PruningOption // KeepRecent should be larger than KeepEvery // SnapshotEvery is a multiple of KeepEvery func (po PruningOptions) IsValid() bool { - // If we're flushing periodically, we must make in-memory cache less than - // that period to avoid inefficiency and undefined behavior. - if po.keepEvery != 0 && po.keepRecent > po.keepEvery { + // If we're flushing each version, then no need for in-memory cache + if po.keepEvery == 1 && po.keepRecent != 0 { + return false + } + // If we're flushing periodically, we must keep latest version + // in memory to keep track of state changes + if po.keepEvery > 1 && po.keepRecent != 1 { return false } // snapshotEvery must be a multiple of keepEvery From 5fab6cdac536420bfa2615bc83b0cda7672e96db Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 5 Feb 2020 16:24:58 -0800 Subject: [PATCH 15/20] address comments and remove keepRecent from user-facing pruning options --- baseapp/baseapp_test.go | 5 ++- store/iavl/store.go | 13 ++++++- store/iavl/store_test.go | 41 ++------------------ store/rootmulti/store_test.go | 5 ++- store/types/pruning.go | 72 ++++++++++++----------------------- 5 files changed, 48 insertions(+), 88 deletions(-) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 6712ba5ef867..6ee5aabbb0a8 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -328,7 +328,10 @@ func TestLoadVersionInvalid(t *testing.T) { func TestLoadVersionPruning(t *testing.T) { logger := log.NewNopLogger() - pruningOptions := store.NewPruningOptions(1, 2, 6) + pruningOptions := store.PruningOptions{ + KeepEvery: 2, + SnapshotEvery: 6, + } pruningOpt := SetPruning(pruningOptions) db := dbm.NewMemDB() name := t.Name() diff --git a/store/iavl/store.go b/store/iavl/store.go index 1a5503aa00be..098ac67c303b 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -42,11 +42,20 @@ func LoadStore(db dbm.DB, id types.CommitID, pruning types.PruningOptions, lazyL if !pruning.IsValid() { return nil, fmt.Errorf("pruning options are invalid: %v", pruning) } + // If KeepEvery = 1, keepRecent should be 0 since there is no need to keep latest version in in-memory cache + // If KeepEvery > 1, keepRecent should be 1 so that state changes in between flushed blocks can be saved in + // in-memory latest tree. + var keepRecent int64 + if pruning.KeepEvery == 1 { + keepRecent = 0 + } else { + keepRecent = 1 + } tree, err := iavl.NewMutableTreeWithOpts( db, dbm.NewMemDB(), defaultIAVLCacheSize, - iavl.PruningOptions(pruning.KeepEvery(), pruning.KeepRecent()), + iavl.PruningOptions(pruning.KeepEvery, keepRecent), ) if err != nil { return nil, err @@ -118,7 +127,7 @@ func (st *Store) Commit() types.CommitID { flushed := st.pruning.FlushVersion(version) if flushed { // If the version we saved got flushed to disk, check if previous flushed version should be deleted - previous := version - st.pruning.KeepEvery() + previous := version - st.pruning.KeepEvery // Previous flushed version should only by deleted if previous version is not snapshot version // OR if snapshotting is disabled (SnapshotEvery == 0) if previous != 0 && !st.pruning.SnapshotVersion(previous) { diff --git a/store/iavl/store_test.go b/store/iavl/store_test.go index 08e04aa6c7b2..ed138c84964f 100644 --- a/store/iavl/store_test.go +++ b/store/iavl/store_test.go @@ -428,7 +428,10 @@ type pruneState struct { func testPruning(t *testing.T, numRecent int64, storeEvery int64, snapshotEvery int64, states []pruneState) { db := dbm.NewMemDB() - pruningOpts := types.NewPruningOptions(numRecent, storeEvery, snapshotEvery) + pruningOpts := types.PruningOptions{ + KeepEvery: storeEvery, + SnapshotEvery: snapshotEvery, + } iavlOpts := iavl.PruningOptions(storeEvery, numRecent) tree, err := iavl.NewMutableTreeWithOpts(db, dbm.NewMemDB(), cacheSize, iavlOpts) @@ -497,42 +500,6 @@ func TestIAVLPruneEverything(t *testing.T) { } } -/* -func TestIAVLPruneRestart(t *testing.T) { - db := dbm.NewMemDB() - pruningOpts := types.NewPruningOptions(1, 3, 6) - iavlOpts := iavl.PruningOptions(3, 1) - - tree, err := iavl.NewMutableTreeWithOpts(db, dbm.NewMemDB(), cacheSize, iavlOpts) - require.NoError(t, err) - - iavlStore := UnsafeNewStore(tree, pruningOpts) - initialCommitID := iavlStore.LastCommitID() - - // Create versions without triggering flush - for i := 0; i < 2; i++ { - nextVersion(iavlStore) - } - - unflushedCommitID := iavlStore.LastCommitID() - require.Equal(t, initialCommitID, unflushedCommitID, "CommitID changed before flush") - - // trigger flush to disk with new version - nextVersion(iavlStore) - - // IAVLStore should have flushed to disk on last version: 3 - flushedCommit := iavlStore.LastCommitID() - - // Create new versions without triggering flush - for i := 0; i < 2; i++ { - nextVersion(iavlStore) - } - - postFlushCommit := iavlStore.LastCommitID() - require.Equal(t, flushedCommit, postFlushCommit, "CommitID changed after flush after mem-only commits") -} -*/ - func TestIAVLStoreQuery(t *testing.T) { db := dbm.NewMemDB() tree, err := iavl.NewMutableTree(db, cacheSize) diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index 0440a975925d..164f231a2ac2 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -292,7 +292,10 @@ func TestParsePath(t *testing.T) { func TestMultiStoreRestart(t *testing.T) { db := dbm.NewMemDB() - pruning := types.NewPruningOptions(1, 3, 6) + pruning := types.PruningOptions{ + KeepEvery: 3, + SnapshotEvery: 6, + } multi := newMultiStoreWithMounts(db, pruning) err := multi.LoadLatestVersion() require.Nil(t, err) diff --git a/store/types/pruning.go b/store/types/pruning.go index 789a3400d6fc..961a5b03514f 100644 --- a/store/types/pruning.go +++ b/store/types/pruning.go @@ -1,70 +1,48 @@ package types -// PruningStrategy specifies how old states will be deleted over time where -// keepRecent can be used with keepEvery to create a pruning "strategy". -// CONTRACT: snapshotEvery is a multiple of keepEvery +// PruningOptions defines the specific pruning strategy every store in a multi-store will use +// when committing state, where keepEvery determines which committed heights are flushed +// to disk and snapshotEvery determines which of these heights are kept after pruning. +// Note, that the invariant keepEvery % snapshotEvery = 0 must hold type PruningOptions struct { - keepRecent int64 - keepEvery int64 - snapshotEvery int64 -} - -func NewPruningOptions(keepRecent, keepEvery, snapshotEvery int64) PruningOptions { - return PruningOptions{ - keepRecent: keepRecent, - keepEvery: keepEvery, - snapshotEvery: snapshotEvery, - } + KeepEvery int64 + SnapshotEvery int64 } // Checks if PruningOptions is valid -// KeepRecent should be larger than KeepEvery // SnapshotEvery is a multiple of KeepEvery func (po PruningOptions) IsValid() bool { - // If we're flushing each version, then no need for in-memory cache - if po.keepEvery == 1 && po.keepRecent != 0 { - return false - } - // If we're flushing periodically, we must keep latest version - // in memory to keep track of state changes - if po.keepEvery > 1 && po.keepRecent != 1 { + // Must flush at positive block interval + if po.KeepEvery <= 0 { return false } - // snapshotEvery must be a multiple of keepEvery - if po.keepEvery == 0 { - return po.snapshotEvery == 0 - } - return po.snapshotEvery%po.keepEvery == 0 + // SnapshotEvery must be a multiple of KeepEvery + return po.SnapshotEvery%po.KeepEvery == 0 } func (po PruningOptions) FlushVersion(ver int64) bool { - return po.keepEvery != 0 && ver%po.keepEvery == 0 + return po.KeepEvery != 0 && ver%po.KeepEvery == 0 } func (po PruningOptions) SnapshotVersion(ver int64) bool { - return po.snapshotEvery != 0 && ver%po.snapshotEvery == 0 -} - -// How much recent state will be kept. Older state will be deleted. -func (po PruningOptions) KeepRecent() int64 { - return po.keepRecent -} - -// Keeps every N stated, deleting others. -func (po PruningOptions) KeepEvery() int64 { - return po.keepEvery -} - -func (po PruningOptions) SnapshotEvery() int64 { - return po.snapshotEvery + return po.SnapshotEvery != 0 && ver%po.SnapshotEvery == 0 } // default pruning strategies var ( // PruneEverything means all saved states will be deleted, storing only the current state - PruneEverything = NewPruningOptions(0, 1, 0) + PruneEverything = PruningOptions{ + KeepEvery: 1, + SnapshotEvery: 0, + } // PruneNothing means all historic states will be saved, nothing will be deleted - PruneNothing = NewPruningOptions(0, 1, 1) - // PruneSyncable means only those states not needed for state syncing will be deleted (flush every 100 + snapshot every 10000th) - PruneSyncable = NewPruningOptions(1, 100, 10000) + PruneNothing = PruningOptions{ + KeepEvery: 1, + SnapshotEvery: 1, + } + // PruneSyncable means only those states not needed for state syncing will be deleted (flush every 100 + Snapshot every 10000th) + PruneSyncable = PruningOptions{ + KeepEvery: 100, + SnapshotEvery: 10000, + } ) From 4fa7fe7b883b41ff1da4e04cf72e156cb2610f49 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 5 Feb 2020 16:38:07 -0800 Subject: [PATCH 16/20] address last comments on review --- store/rootmulti/store.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index c44db08a5d76..169b971fd26e 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -294,11 +294,7 @@ func (rs *Store) Commit() types.CommitID { // write CommitInfo to disk only if this version was flushed to disk if rs.pruningOpts.FlushVersion(version) { // Need to update atomically. - batch := rs.db.NewBatch() - defer batch.Close() - setCommitInfo(batch, version, rs.lastCommitInfo) - setLatestVersion(batch, version) - batch.Write() + flushCommitInfo(rs.db, version, rs.lastCommitInfo) } // Prepare for next version. @@ -443,6 +439,8 @@ func (rs *Store) Query(req abci.RequestQuery) abci.ResponseQuery { return sdkerrors.QueryResult(sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "proof is unexpectedly empty; ensure height has not been pruned")) } + // If the request's height is the latest height we've committed, then utilize the store's lastCommitInfo + // as this commit info may not be flushed to disk. Otherwise, we query for the commit info from disk. var commitInfo commitInfo if res.Height == rs.lastCommitInfo.Version { commitInfo = rs.lastCommitInfo @@ -684,3 +682,12 @@ func setCommitInfo(batch dbm.Batch, version int64, cInfo commitInfo) { cInfoKey := fmt.Sprintf(commitInfoKeyFmt, version) batch.Set([]byte(cInfoKey), cInfoBytes) } + +// Flush a commitInfo for given version to a db +func flushCommitInfo(db dbm.DB, version int64, cInfo commitInfo) { + batch := db.NewBatch() + defer batch.Close() + setCommitInfo(batch, version, cInfo) + setLatestVersion(batch, version) + batch.Write() +} From e8676aba17cca4ccfd13149da4424f9a02392792 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 6 Feb 2020 10:49:44 -0500 Subject: [PATCH 17/20] Linting and cleanup --- server/start.go | 2 +- store/types/pruning.go | 72 ++++++++++++++++++++++++++---------------- 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/server/start.go b/server/start.go index 8edec4f831c1..5a9cd0500884 100644 --- a/server/start.go +++ b/server/start.go @@ -43,7 +43,7 @@ default, the application will run with Tendermint in process. Pruning options can be provided via the '--pruning' flag. The options are as follows: -syncable: only those states not needed for state syncing will be deleted (keeps last 100 + every 10000th) +syncable: only those states not needed for state syncing will be deleted (flushes every 100th to disk and keeps every 10000th) nothing: all historic states will be saved, nothing will be deleted (i.e. archiving node) everything: all saved states will be deleted, storing only the current state diff --git a/store/types/pruning.go b/store/types/pruning.go index 961a5b03514f..c540443521ff 100644 --- a/store/types/pruning.go +++ b/store/types/pruning.go @@ -1,48 +1,66 @@ package types -// PruningOptions defines the specific pruning strategy every store in a multi-store will use -// when committing state, where keepEvery determines which committed heights are flushed -// to disk and snapshotEvery determines which of these heights are kept after pruning. -// Note, that the invariant keepEvery % snapshotEvery = 0 must hold +var ( + // PruneEverything defines a pruning strategy where all committed states will + // be deleted, persisting only the current state. + PruneEverything = PruningOptions{ + KeepEvery: 1, + SnapshotEvery: 0, + } + + // PruneNothing defines a pruning strategy where all committed states will be + // kept on disk, i.e. no states will be pruned. + PruneNothing = PruningOptions{ + KeepEvery: 1, + SnapshotEvery: 1, + } + + // PruneSyncable defines a pruning strategy where only those states not needed + // for state syncing will be pruned. It flushes every 100th state to disk and + // keeps every 10000th. + PruneSyncable = PruningOptions{ + KeepEvery: 100, + SnapshotEvery: 10000, + } +) + +// PruningOptions defines the specific pruning strategy every store in a multi-store +// will use when committing state, where keepEvery determines which committed +// heights are flushed to disk and snapshotEvery determines which of these heights +// are kept after pruning. type PruningOptions struct { KeepEvery int64 SnapshotEvery int64 } -// Checks if PruningOptions is valid -// SnapshotEvery is a multiple of KeepEvery +// IsValid verifies if the pruning options are valid. It returns false if invalid +// and true otherwise. Pruning options are considered valid iff: +// +// - KeepEvery > 0 +// - SnapshotEvery >= 0 +// - SnapshotEvery % KeepEvery = 0 func (po PruningOptions) IsValid() bool { - // Must flush at positive block interval + // must flush at positive block interval if po.KeepEvery <= 0 { return false } - // SnapshotEvery must be a multiple of KeepEvery + + // cannot snapshot negative intervals + if po.SnapshotEvery < 0 { + return false + } + return po.SnapshotEvery%po.KeepEvery == 0 } +// FlushVersion returns a boolean signaling if the provided version/height should +// be flushed to disk. func (po PruningOptions) FlushVersion(ver int64) bool { return po.KeepEvery != 0 && ver%po.KeepEvery == 0 } +// SnapshotVersion returns a boolean signaling if the provided version/height +// should be snapshotted (kept on disk). func (po PruningOptions) SnapshotVersion(ver int64) bool { return po.SnapshotEvery != 0 && ver%po.SnapshotEvery == 0 } - -// default pruning strategies -var ( - // PruneEverything means all saved states will be deleted, storing only the current state - PruneEverything = PruningOptions{ - KeepEvery: 1, - SnapshotEvery: 0, - } - // PruneNothing means all historic states will be saved, nothing will be deleted - PruneNothing = PruningOptions{ - KeepEvery: 1, - SnapshotEvery: 1, - } - // PruneSyncable means only those states not needed for state syncing will be deleted (flush every 100 + Snapshot every 10000th) - PruneSyncable = PruningOptions{ - KeepEvery: 100, - SnapshotEvery: 10000, - } -) From d8186a6fd7d908dd25098e2a7fe05fb9bcf1fffc Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 6 Feb 2020 11:03:07 -0500 Subject: [PATCH 18/20] More linting --- store/iavl/store.go | 44 ++++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/store/iavl/store.go b/store/iavl/store.go index 098ac67c303b..d1e1d9c4cae0 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -35,22 +35,29 @@ type Store struct { pruning types.PruningOptions } -// LoadStore returns an IAVL Store as a CommitKVStore. Internally it will load the +// LoadStore returns an IAVL Store as a CommitKVStore. Internally, it will load the // store's version (id) from the provided DB. An error is returned if the version // fails to load. func LoadStore(db dbm.DB, id types.CommitID, pruning types.PruningOptions, lazyLoading bool) (types.CommitKVStore, error) { if !pruning.IsValid() { return nil, fmt.Errorf("pruning options are invalid: %v", pruning) } - // If KeepEvery = 1, keepRecent should be 0 since there is no need to keep latest version in in-memory cache - // If KeepEvery > 1, keepRecent should be 1 so that state changes in between flushed blocks can be saved in - // in-memory latest tree. + var keepRecent int64 + + // Determine the value of keepRecent based on the following: + // + // If KeepEvery = 1, keepRecent should be 0 since there is no need to keep + // latest version in a in-memory cache. + // + // If KeepEvery > 1, keepRecent should be 1 so that state changes in between + // flushed states can be saved in the in-memory latest tree. if pruning.KeepEvery == 1 { keepRecent = 0 } else { keepRecent = 1 } + tree, err := iavl.NewMutableTreeWithOpts( db, dbm.NewMemDB(), @@ -71,16 +78,14 @@ func LoadStore(db dbm.DB, id types.CommitID, pruning types.PruningOptions, lazyL return nil, err } - store := Store{ + return &Store{ tree: tree, pruning: pruning, - } - - return &store, nil + }, nil } // UnsafeNewStore returns a reference to a new IAVL Store with a given mutable -// IAVL tree reference. +// IAVL tree reference. It should only be used for testing purposes. // // CONTRACT: The IAVL tree should be fully loaded. // CONTRACT: PruningOptions passed in as argument must be the same as pruning options @@ -107,29 +112,28 @@ func (st *Store) GetImmutable(version int64) (*Store, error) { return nil, err } - store := &Store{ + return &Store{ tree: &immutableTree{iTree}, pruning: st.pruning, - } - - return store, nil + }, nil } -// Implements Committer. +// Commit commits the current store state and returns a CommitID with the new +// version and hash. func (st *Store) Commit() types.CommitID { - // Save a new version. hash, version, err := st.tree.SaveVersion() if err != nil { // TODO: Do we want to extend Commit to allow returning errors? panic(err) } - flushed := st.pruning.FlushVersion(version) - if flushed { - // If the version we saved got flushed to disk, check if previous flushed version should be deleted + // If the version we saved got flushed to disk, check if previous flushed + // version should be deleted. + if st.pruning.FlushVersion(version) { previous := version - st.pruning.KeepEvery - // Previous flushed version should only by deleted if previous version is not snapshot version - // OR if snapshotting is disabled (SnapshotEvery == 0) + + // Previous flushed version should only be pruned if the previous version is + // not a snapshot version OR if snapshotting is disabled (SnapshotEvery == 0). if previous != 0 && !st.pruning.SnapshotVersion(previous) { err := st.tree.DeleteVersion(previous) if errCause := errors.Cause(err); errCause != nil && errCause != iavl.ErrVersionDoesNotExist { From b210d0e5fdb485586e687d0672dc46ebaa6f9819 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 6 Feb 2020 12:13:39 -0500 Subject: [PATCH 19/20] More linting --- store/rootmulti/store.go | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index 169b971fd26e..42c7b05ab589 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -293,7 +293,6 @@ func (rs *Store) Commit() types.CommitID { // write CommitInfo to disk only if this version was flushed to disk if rs.pruningOpts.FlushVersion(version) { - // Need to update atomically. flushCommitInfo(rs.db, version, rs.lastCommitInfo) } @@ -410,7 +409,6 @@ func (rs *Store) getStoreByName(name string) types.Store { // Ie. `req.Path` here is `//`, and trimmed to `/` for the substore. // TODO: add proof for `multistore -> substore`. func (rs *Store) Query(req abci.RequestQuery) abci.ResponseQuery { - // Query just routes this to a substore. path := req.Path storeName, subpath, err := parsePath(path) if err != nil { @@ -439,15 +437,16 @@ func (rs *Store) Query(req abci.RequestQuery) abci.ResponseQuery { return sdkerrors.QueryResult(sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "proof is unexpectedly empty; ensure height has not been pruned")) } - // If the request's height is the latest height we've committed, then utilize the store's lastCommitInfo - // as this commit info may not be flushed to disk. Otherwise, we query for the commit info from disk. + // If the request's height is the latest height we've committed, then utilize + // the store's lastCommitInfo as this commit info may not be flushed to disk. + // Otherwise, we query for the commit info from disk. var commitInfo commitInfo + if res.Height == rs.lastCommitInfo.Version { commitInfo = rs.lastCommitInfo } else { - var errMsg error - commitInfo, errMsg = getCommitInfo(rs.db, res.Height) - if errMsg != nil { + commitInfo, err = getCommitInfo(rs.db, res.Height) + if err != nil { return sdkerrors.QueryResult(err) } } @@ -632,26 +631,22 @@ func commitStores(version int64, storeMap map[types.StoreKey]types.CommitKVStore storeInfos := make([]storeInfo, 0, len(storeMap)) for key, store := range storeMap { - // Commit commitID := store.Commit() if store.GetStoreType() == types.StoreTypeTransient { continue } - // Record CommitID si := storeInfo{} si.Name = key.Name() si.Core.CommitID = commitID - // si.Core.StoreType = store.GetStoreType() storeInfos = append(storeInfos, si) } - ci := commitInfo{ + return commitInfo{ Version: version, StoreInfos: storeInfos, } - return ci } // Gets commitInfo from disk. @@ -683,10 +678,12 @@ func setCommitInfo(batch dbm.Batch, version int64, cInfo commitInfo) { batch.Set([]byte(cInfoKey), cInfoBytes) } -// Flush a commitInfo for given version to a db +// flushCommitInfo flushes a commitInfo for given version to the DB. Note, this +// needs to happen atomically. func flushCommitInfo(db dbm.DB, version int64, cInfo commitInfo) { batch := db.NewBatch() defer batch.Close() + setCommitInfo(batch, version, cInfo) setLatestVersion(batch, version) batch.Write() From 5b00d1a3eb84abd89730ae7e7c5e5726e95c6a13 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 6 Feb 2020 13:05:12 -0500 Subject: [PATCH 20/20] Edit changelog --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc45a176b45d..f69bc97ef8b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,11 +44,23 @@ balances or a single balance by denom when the `denom` query parameter is presen ### API Breaking Changes +* (types) [\#5579](https://github.com/cosmos/cosmos-sdk/pull/5579) The `keepRecent` field has been removed from the `PruningOptions` type. +The `PruningOptions` type now only includes fields `KeepEvery` and `SnapshotEvery`, where `KeepEvery` +determines which committed heights are flushed to disk and `SnapshotEvery` determines which of these +heights are kept after pruning. The `IsValid` method should be called whenever using these options. Methods +`SnapshotVersion` and `FlushVersion` accept a version arugment and determine if the version should be +flushed to disk or kept as a snapshot. Note, `KeepRecent` is automatically inferred from the options +and provided directly the IAVL store. * (modules) [\#5555](https://github.com/cosmos/cosmos-sdk/pull/5555) Move x/auth/client/utils/ types and functions to x/auth/client/. * (modules) [\#5572](https://github.com/cosmos/cosmos-sdk/pull/5572) Move account balance logic and APIs from `x/auth` to `x/bank`. ### Bug Fixes +* (types) [\#5579](https://github.com/cosmos/cosmos-sdk/pull/5579) The IAVL `Store#Commit` method has been refactored to +delete a flushed version if it is not a snapshot version. The root multi-store now keeps track of `commitInfo` instead +of `types.CommitID`. During `Commit` of the root multi-store, `lastCommitInfo` is updated from the saved state +and is only flushed to disk if it is a snapshot version. During `Query` of the root multi-store, if the request height +is the latest height, we'll use the store's `lastCommitInfo`. Otherwise, we fetch `commitInfo` from disk. * (x/bank) [\#5531](https://github.com/cosmos/cosmos-sdk/issues/5531) Added missing amount event to MsgMultiSend, emitted for each output. ### State Machine Breaking