From a70c19a03a811244a030f3db4661889021392554 Mon Sep 17 00:00:00 2001 From: yihuang Date: Fri, 1 Nov 2024 14:55:53 +0800 Subject: [PATCH 1/4] Problem: tx is decoded in PrepareProposal when using NopMempool (#884) * Problem: tx is decoded in PrepareProposal when using NopMempool * Update CHANGELOG.md Signed-off-by: yihuang * fix test --------- Signed-off-by: yihuang --- CHANGELOG.md | 1 + baseapp/abci_utils.go | 26 +++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ba20e167f9e..45edecfdfe6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#261](https://github.com/crypto-org-chain/cosmos-sdk/pull/261) `ctx.BlockHeader` don't do protobuf deep copy, shallow copy seems enough, reduce scope of mutex in `PriorityNonceMempool.Remove`. * [#507](https://github.com/crypto-org-chain/cosmos-sdk/pull/507) mempool respect gas wanted returned by ante handler * [#744](https://github.com/crypto-org-chain/cosmos-sdk/pull/744) Pass raw transactions to tx executor so it can do pre-estimations. +* [#884](https://github.com/crypto-org-chain/cosmos-sdk/pull/884) Avoid decoding tx for in PrepareProposal if it's NopMempool. ## [Unreleased] diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 1bbd500bd705..dc532c191b0c 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -212,6 +212,9 @@ type ( txVerifier ProposalTxVerifier txSelector TxSelector signerExtAdapter mempool.SignerExtractionAdapter + + // fastPrepareProposal together with NoOpMempool will bypass tx selector + fastPrepareProposal bool } ) @@ -224,6 +227,12 @@ func NewDefaultProposalHandler(mp mempool.Mempool, txVerifier ProposalTxVerifier } } +func NewDefaultProposalHandlerFast(mp mempool.Mempool, txVerifier ProposalTxVerifier) *DefaultProposalHandler { + h := NewDefaultProposalHandler(mp, txVerifier) + h.fastPrepareProposal = true + return h +} + // SetTxSelector sets the TxSelector function on the DefaultProposalHandler. func (h *DefaultProposalHandler) SetTxSelector(ts TxSelector) { h.txSelector = ts @@ -264,6 +273,11 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan // Note, we still need to ensure the transactions returned respect req.MaxTxBytes. _, isNoOp := h.mempool.(mempool.NoOpMempool) if h.mempool == nil || isNoOp { + if h.fastPrepareProposal { + txs := h.txSelector.SelectTxForProposalFast(ctx, req.Txs) + return &abci.ResponsePrepareProposal{Txs: txs}, nil + } + for _, txBz := range req.Txs { tx, err := h.txVerifier.TxDecode(txBz) if err != nil { @@ -464,6 +478,12 @@ type TxSelector interface { // return if the caller should halt the transaction selection loop // (typically over a mempool) or otherwise. SelectTxForProposal(ctx context.Context, maxTxBytes, maxBlockGas uint64, memTx sdk.Tx, txBz []byte, gasWanted uint64) bool + + // SelectTxForProposalFast is called in the case of NoOpMempool, + // where cometbft already checked the block gas/size limit, + // so the tx selector should simply accept them all to the proposal. + // But extra validations on the tx are still possible though. + SelectTxForProposalFast(ctx context.Context, txs [][]byte) [][]byte } type defaultTxSelector struct { @@ -485,7 +505,7 @@ func (ts *defaultTxSelector) SelectedTxs(_ context.Context) [][]byte { func (ts *defaultTxSelector) Clear() { ts.totalTxBytes = 0 ts.totalTxGas = 0 - ts.selectedTxs = nil + ts.selectedTxs = ts.selectedTxs[:0] // keep the allocated memory } func (ts *defaultTxSelector) SelectTxForProposal(_ context.Context, maxTxBytes, maxBlockGas uint64, memTx sdk.Tx, txBz []byte, gasWanted uint64) bool { @@ -510,3 +530,7 @@ func (ts *defaultTxSelector) SelectTxForProposal(_ context.Context, maxTxBytes, // check if we've reached capacity; if so, we cannot select any more transactions return ts.totalTxBytes >= maxTxBytes || (maxBlockGas > 0 && (ts.totalTxGas >= maxBlockGas)) } + +func (ts *defaultTxSelector) SelectTxForProposalFast(ctx context.Context, txs [][]byte) [][]byte { + return txs +} From e790d2346678dd299758a816262a57d0296b4b10 Mon Sep 17 00:00:00 2001 From: yihuang Date: Wed, 13 Nov 2024 10:19:18 +0800 Subject: [PATCH 2/4] Problem: iavl v1 pruning is slow (#923) * Problem: iavl v1 pruning is slow Solution: - enable async pruning * Update CHANGELOG.md Signed-off-by: yihuang --------- Signed-off-by: yihuang --- CHANGELOG.md | 1 + store/iavl/store.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45edecfdfe6b..eb23b60027af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements * (server) [#21941](https://github.com/cosmos/cosmos-sdk/pull/21941) Regenerate addrbook.json for in place testnet. +* (store) [#923](https://github.com/crypto-org-chain/cosmos-sdk/pull/923) Enable iavl async pruning. ### Bug Fixes diff --git a/store/iavl/store.go b/store/iavl/store.go index e73dcd6c1fdd..8e091260fe40 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -50,7 +50,7 @@ func LoadStore(db dbm.DB, logger log.Logger, key types.StoreKey, id types.Commit // provided DB. An error is returned if the version fails to load, or if called with a positive // version on an empty tree. func LoadStoreWithInitialVersion(db dbm.DB, logger log.Logger, key types.StoreKey, id types.CommitID, initialVersion uint64, cacheSize int, disableFastNode bool, metrics metrics.StoreMetrics) (types.CommitKVStore, error) { - tree := iavl.NewMutableTree(wrapper.NewDBWrapper(db), cacheSize, disableFastNode, logger, iavl.InitialVersionOption(initialVersion)) + tree := iavl.NewMutableTree(wrapper.NewDBWrapper(db), cacheSize, disableFastNode, logger, iavl.InitialVersionOption(initialVersion), iavl.AsyncPruningOption(true)) isUpgradeable, err := tree.IsUpgradeable() if err != nil { From 2d4f901700176a65ae63840b64c8e1bb7adadbbe Mon Sep 17 00:00:00 2001 From: mmsqe Date: Wed, 13 Nov 2024 10:36:35 +0800 Subject: [PATCH 3/4] Problem: async prune test is failed (#924) --- store/rootmulti/store_test.go | 49 ++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index 3d7dca809d2f..dcb5d8482af7 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -532,14 +532,18 @@ func TestMultiStore_Pruning(t *testing.T) { ms.Commit() } - for _, v := range tc.saved { - _, err := ms.CacheMultiStoreWithVersion(v) - require.NoError(t, err, "expected no error when loading height: %d", v) + for _, v := range tc.deleted { + // Ensure async pruning is done + checkErr := func() bool { + _, err := ms.CacheMultiStoreWithVersion(v) + return err != nil + } + require.Eventually(t, checkErr, 1*time.Second, 10*time.Millisecond, "expected error when loading height: %d", v) } - for _, v := range tc.deleted { + for _, v := range tc.saved { _, err := ms.CacheMultiStoreWithVersion(v) - require.Error(t, err, "expected error when loading height: %d", v) + require.NoError(t, err, "expected no error when loading height: %d", v) } }) } @@ -564,16 +568,6 @@ func TestMultiStore_Pruning_SameHeightsTwice(t *testing.T) { require.Equal(t, numVersions, lastCommitInfo.Version) - for v := int64(1); v < numVersions-int64(keepRecent); v++ { - err := ms.LoadVersion(v) - require.Error(t, err, "expected error when loading pruned height: %d", v) - } - - for v := (numVersions - int64(keepRecent)); v < numVersions; v++ { - err := ms.LoadVersion(v) - require.NoError(t, err, "expected no error when loading height: %d", v) - } - // Get latest err := ms.LoadVersion(numVersions - 1) require.NoError(t, err) @@ -588,6 +582,17 @@ func TestMultiStore_Pruning_SameHeightsTwice(t *testing.T) { // Ensure that can commit one more height with no panic lastCommitInfo = ms.Commit() require.Equal(t, numVersions+1, lastCommitInfo.Version) + isPruned := func() bool { + ls := ms.Commit() // to flush the batch with the pruned heights + for v := int64(1); v < numVersions-int64(keepRecent); v++ { + if err := ms.LoadVersion(v); err == nil { + require.NoError(t, ms.LoadVersion(ls.Version)) // load latest + return false + } + } + return true + } + require.Eventually(t, isPruned, 1000*time.Second, 10*time.Millisecond, "expected error when loading pruned heights") } func TestMultiStore_PruningRestart(t *testing.T) { @@ -618,10 +623,18 @@ func TestMultiStore_PruningRestart(t *testing.T) { actualHeightToPrune = ms.pruningManager.GetPruningHeight(ms.LatestVersion()) require.Equal(t, int64(8), actualHeightToPrune) - for v := int64(1); v <= actualHeightToPrune; v++ { - _, err := ms.CacheMultiStoreWithVersion(v) - require.Error(t, err, "expected error when loading height: %d", v) + // Ensure async pruning is done + isPruned := func() bool { + ms.Commit() // to flush the batch with the pruned heights + for v := int64(1); v <= actualHeightToPrune; v++ { + if _, err := ms.CacheMultiStoreWithVersion(v); err == nil { + return false + } + } + return true } + + require.Eventually(t, isPruned, 1*time.Second, 10*time.Millisecond, "expected error when loading pruned heights") } // TestUnevenStoresHeightCheck tests if loading root store correctly errors when From 3300cc8f38369e2dcf6bd9aefac677309325f065 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 14 Nov 2024 10:00:21 +0800 Subject: [PATCH 4/4] Problem: pause pruning is not included (#934) --- CHANGELOG.md | 1 + store/iavl/store.go | 9 +++++++++ store/iavl/tree.go | 10 ++++++++++ store/rootmulti/store.go | 19 ++++++++++++++++++- store/rootmulti/store_test.go | 24 ++++++++++++++++++++++++ store/types/store.go | 8 ++++++++ 6 files changed, 70 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb23b60027af..15046e02b0b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (server) [#21941](https://github.com/cosmos/cosmos-sdk/pull/21941) Regenerate addrbook.json for in place testnet. * (store) [#923](https://github.com/crypto-org-chain/cosmos-sdk/pull/923) Enable iavl async pruning. +* (store) [#934](https://github.com/crypto-org-chain/cosmos-sdk/pull/934) Add pause pruning. ### Bug Fixes diff --git a/store/iavl/store.go b/store/iavl/store.go index 8e091260fe40..7066891cda19 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -145,6 +145,15 @@ func (st *Store) LastCommitID() types.CommitID { } } +// PausePruning implements CommitKVStore interface. +func (st *Store) PausePruning(pause bool) { + if pause { + st.tree.SetCommitting() + } else { + st.tree.UnsetCommitting() + } +} + // SetPruning panics as pruning options should be provided at initialization // since IAVl accepts pruning options directly. func (st *Store) SetPruning(_ pruningtypes.PruningOptions) { diff --git a/store/iavl/tree.go b/store/iavl/tree.go index 889fc1d5a07f..f6a2db1ffc79 100644 --- a/store/iavl/tree.go +++ b/store/iavl/tree.go @@ -22,6 +22,8 @@ type ( Get(key []byte) ([]byte, error) Set(key, value []byte) (bool, error) Remove(key []byte) ([]byte, bool, error) + SetCommitting() + UnsetCommitting() SaveVersion() ([]byte, int64, error) Version() int64 Hash() []byte @@ -53,6 +55,14 @@ func (it *immutableTree) Remove(_ []byte) ([]byte, bool, error) { panic("cannot call 'Remove' on an immutable IAVL tree") } +func (it *immutableTree) SetCommitting() { + panic("cannot call 'SetCommitting' on an immutable IAVL tree") +} + +func (it *immutableTree) UnsetCommitting() { + panic("cannot call 'UnsetCommitting' on an immutable IAVL tree") +} + func (it *immutableTree) SaveVersion() ([]byte, int64, error) { panic("cannot call 'SaveVersion' on an immutable IAVL tree") } diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index bb3bc04ac4f2..540814b51415 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -464,6 +464,16 @@ func (rs *Store) LastCommitID() types.CommitID { return rs.lastCommitInfo.CommitID() } +// PausePruning temporarily pauses the pruning of all individual stores which implement +// the PausablePruner interface. +func (rs *Store) PausePruning(pause bool) { + for _, store := range rs.stores { + if pauseable, ok := store.(types.PausablePruner); ok { + pauseable.PausePruning(pause) + } + } +} + // Commit implements Committer/CommitStore. func (rs *Store) Commit() types.CommitID { var previousHeight, version int64 @@ -485,7 +495,14 @@ func (rs *Store) Commit() types.CommitID { rs.logger.Debug("commit header and version mismatch", "header_height", rs.commitHeader.Height, "version", version) } - rs.lastCommitInfo = commitStores(version, rs.stores, rs.removalMap) + func() { // ensure unpause + // set the committing flag on all stores to block the pruning + rs.PausePruning(true) + // unset the committing flag on all stores to continue the pruning + defer rs.PausePruning(false) + rs.lastCommitInfo = commitStores(version, rs.stores, rs.removalMap) + }() + rs.lastCommitInfo.Timestamp = rs.commitHeader.Time defer rs.flushMetadata(rs.db, version, rs.lastCommitInfo) diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index dcb5d8482af7..23669c82a9a0 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -637,6 +637,30 @@ func TestMultiStore_PruningRestart(t *testing.T) { require.Eventually(t, isPruned, 1*time.Second, 10*time.Millisecond, "expected error when loading pruned heights") } +var _ types.PausablePruner = &pauseableCommitKVStoreStub{} + +type pauseableCommitKVStoreStub struct { + types.CommitKVStore + pauseCalled []bool +} + +func (p *pauseableCommitKVStoreStub) PausePruning(b bool) { + p.pauseCalled = append(p.pauseCalled, b) +} + +func TestPausePruningOnCommit(t *testing.T) { + store := NewStore(dbm.NewMemDB(), log.NewNopLogger(), metrics.NewNoOpMetrics()) + store.SetPruning(pruningtypes.NewCustomPruningOptions(2, 11)) + store.MountStoreWithDB(testStoreKey1, types.StoreTypeIAVL, nil) + require.NoError(t, store.LoadLatestVersion()) + myStub := &pauseableCommitKVStoreStub{CommitKVStore: store.stores[testStoreKey1].(types.CommitKVStore)} + store.stores[testStoreKey1] = myStub + // when + store.Commit() + // then + require.Equal(t, []bool{true, false}, myStub.pauseCalled) +} + // TestUnevenStoresHeightCheck tests if loading root store correctly errors when // there's any module store with the wrong height func TestUnevenStoresHeightCheck(t *testing.T) { diff --git a/store/types/store.go b/store/types/store.go index 67bd140f5e75..cd92fbd0ca07 100644 --- a/store/types/store.go +++ b/store/types/store.go @@ -29,6 +29,14 @@ type Committer interface { GetPruning() pruningtypes.PruningOptions } +type PausablePruner interface { + // PausePruning let the pruning handler know that the store is being committed + // or not, so the handler can decide to prune or not the store. + // + // NOTE: PausePruning(true) should be called before Commit() and PausePruning(false) + PausePruning(bool) +} + // Stores of MultiStore must implement CommitStore. type CommitStore interface { Committer