From 9c2af677afc2e8318b8a7e356e37633fbf10cfa6 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 11 Oct 2024 20:47:31 +0200 Subject: [PATCH 1/5] fix(server/v2/cometbft): close app --- server/v2/cometbft/abci.go | 3 +++ server/v2/cometbft/abci_test.go | 6 +++--- server/v2/cometbft/server.go | 1 + server/v2/types.go | 1 + simapp/v2/app_di.go | 10 ++++++++++ 5 files changed, 18 insertions(+), 3 deletions(-) diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index 95eeec9bb5da..75618e296022 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -43,6 +43,7 @@ type Consensus[T transaction.Tx] struct { logger log.Logger appName, version string app *appmanager.AppManager[T] + appCloser func() error txCodec transaction.Codec[T] store types.Store streaming streaming.Manager @@ -77,6 +78,7 @@ func NewConsensus[T transaction.Tx]( logger log.Logger, appName string, app *appmanager.AppManager[T], + appCloser func() error, mp mempool.Mempool[T], indexedEvents map[string]struct{}, queryHandlersMap map[string]appmodulev2.Handler, @@ -89,6 +91,7 @@ func NewConsensus[T transaction.Tx]( appName: appName, version: getCometBFTServerVersion(), app: app, + appCloser: appCloser, cfg: cfg, store: store, logger: logger, diff --git a/server/v2/cometbft/abci_test.go b/server/v2/cometbft/abci_test.go index 3af3fbec8f29..9a42948c6c61 100644 --- a/server/v2/cometbft/abci_test.go +++ b/server/v2/cometbft/abci_test.go @@ -19,7 +19,7 @@ import ( "cosmossdk.io/core/store" "cosmossdk.io/core/transaction" "cosmossdk.io/log" - am "cosmossdk.io/server/v2/appmanager" + "cosmossdk.io/server/v2/appmanager" "cosmossdk.io/server/v2/cometbft/handlers" cometmock "cosmossdk.io/server/v2/cometbft/internal/mock" "cosmossdk.io/server/v2/cometbft/mempool" @@ -672,7 +672,7 @@ func setUpConsensus(t *testing.T, gasLimit uint64, mempool mempool.Mempool[mock. sc := cometmock.NewMockCommiter(log.NewNopLogger(), string(actorName), "stf") mockStore := cometmock.NewMockStore(ss, sc) - b := am.Builder[mock.Tx]{ + b := appmanager.Builder[mock.Tx]{ STF: s, DB: mockStore, ValidateTxGasLimit: gasLimit, @@ -688,7 +688,7 @@ func setUpConsensus(t *testing.T, gasLimit uint64, mempool mempool.Mempool[mock. am, err := b.Build() require.NoError(t, err) - return NewConsensus[mock.Tx](log.NewNopLogger(), "testing-app", am, mempool, map[string]struct{}{}, nil, mockStore, Config{AppTomlConfig: DefaultAppTomlConfig()}, mock.TxCodec{}, "test") + return NewConsensus[mock.Tx](log.NewNopLogger(), "testing-app", am, func() error { return nil }, mempool, map[string]struct{}{}, nil, mockStore, Config{AppTomlConfig: DefaultAppTomlConfig()}, mock.TxCodec{}, "test") } // Check target version same with store's latest version diff --git a/server/v2/cometbft/server.go b/server/v2/cometbft/server.go index 8bd2935149e7..20cd63d07ca4 100644 --- a/server/v2/cometbft/server.go +++ b/server/v2/cometbft/server.go @@ -107,6 +107,7 @@ func (s *CometBFTServer[T]) Init(appI serverv2.AppI[T], cfg map[string]any, logg s.logger, appI.Name(), appI.GetAppManager(), + appI.Close, s.serverOptions.Mempool(cfg), indexEvents, appI.GetQueryHandlers(), diff --git a/server/v2/types.go b/server/v2/types.go index e628fb30c90f..5406313b080e 100644 --- a/server/v2/types.go +++ b/server/v2/types.go @@ -21,4 +21,5 @@ type AppI[T transaction.Tx] interface { GetQueryHandlers() map[string]appmodulev2.Handler GetStore() store.RootStore GetSchemaDecoderResolver() decoding.DecoderResolver + Close() error } diff --git a/simapp/v2/app_di.go b/simapp/v2/app_di.go index b1dfc26f99ff..3c9933721f12 100644 --- a/simapp/v2/app_di.go +++ b/simapp/v2/app_di.go @@ -218,6 +218,16 @@ func (app *SimApp[T]) TxConfig() client.TxConfig { return app.txConfig } +// GetStore returns the root store. func (app *SimApp[T]) GetStore() store.RootStore { return app.store } + +// Close overwrites the base Close method to close the stores. +func (app *SimApp[T]) Close() error { + if err := app.store.Close(); err != nil { + return err + } + + return app.App.Close() +} From 2afd304977b2a99d3888f8f254cc5053196f61c1 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 11 Oct 2024 22:03:38 +0200 Subject: [PATCH 2/5] close main db from store/v2 --- store/v2/commitment/metadata.go | 1 + store/v2/commitment/store.go | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/store/v2/commitment/metadata.go b/store/v2/commitment/metadata.go index 642e6b630dac..a054acf26e89 100644 --- a/store/v2/commitment/metadata.go +++ b/store/v2/commitment/metadata.go @@ -17,6 +17,7 @@ const ( ) // MetadataStore is a store for metadata related to the commitment store. +// It isn't metadata store role to close the underlying KVStore. type MetadataStore struct { kv corestore.KVStoreWithBatch } diff --git a/store/v2/commitment/store.go b/store/v2/commitment/store.go index da0440987524..98765dfb06be 100644 --- a/store/v2/commitment/store.go +++ b/store/v2/commitment/store.go @@ -38,6 +38,7 @@ type MountTreeFn func(storeKey string) (Tree, error) // and trees. type CommitStore struct { logger corelog.Logger + db corestore.KVStoreWithBatch // holds the db instance for closing it metadata *MetadataStore multiTrees map[string]Tree // oldTrees is a map of store keys to old trees that have been deleted or renamed. @@ -49,6 +50,7 @@ type CommitStore struct { func NewCommitStore(trees, oldTrees map[string]Tree, db corestore.KVStoreWithBatch, logger corelog.Logger) (*CommitStore, error) { return &CommitStore{ logger: logger, + db: db, multiTrees: trees, oldTrees: oldTrees, metadata: NewMetadataStore(db), @@ -499,5 +501,6 @@ func (c *CommitStore) Close() error { } } - return nil + // close the db + return c.db.Close() } From 029d19f25749502e9aaace4f3dfdca688be5465e Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 11 Oct 2024 22:06:12 +0200 Subject: [PATCH 3/5] typo --- server/v2/testdata/app.toml | 2 +- store/v2/root/factory.go | 2 +- tools/confix/data/v2-app.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/v2/testdata/app.toml b/server/v2/testdata/app.toml index 094ac068d94a..3d32be7e57ef 100644 --- a/server/v2/testdata/app.toml +++ b/server/v2/testdata/app.toml @@ -25,7 +25,7 @@ minimum-gas-prices = '0stake' app-db-backend = 'goleveldb' [store.options] -# SState storage database type. Currently we support: "sqlite", "pebble" and "rocksdb" +# State storage database type. Currently we support: "sqlite", "pebble" and "rocksdb" ss-type = 'sqlite' # State commitment database type. Currently we support: "iavl" and "iavl-v2" sc-type = 'iavl' diff --git a/store/v2/root/factory.go b/store/v2/root/factory.go index 9e1f76a36ceb..83f6b85125aa 100644 --- a/store/v2/root/factory.go +++ b/store/v2/root/factory.go @@ -35,7 +35,7 @@ const ( // Options are the options for creating a root store. type Options struct { - SSType SSType `mapstructure:"ss-type" toml:"ss-type" comment:"SState storage database type. Currently we support: \"sqlite\", \"pebble\" and \"rocksdb\""` + SSType SSType `mapstructure:"ss-type" toml:"ss-type" comment:"State storage database type. Currently we support: \"sqlite\", \"pebble\" and \"rocksdb\""` SCType SCType `mapstructure:"sc-type" toml:"sc-type" comment:"State commitment database type. Currently we support: \"iavl\" and \"iavl-v2\""` SSPruningOption *store.PruningOption `mapstructure:"ss-pruning-option" toml:"ss-pruning-option" comment:"Pruning options for state storage"` SCPruningOption *store.PruningOption `mapstructure:"sc-pruning-option" toml:"sc-pruning-option" comment:"Pruning options for state commitment"` diff --git a/tools/confix/data/v2-app.toml b/tools/confix/data/v2-app.toml index ef144a0b90dd..8a35d32f543b 100644 --- a/tools/confix/data/v2-app.toml +++ b/tools/confix/data/v2-app.toml @@ -50,7 +50,7 @@ minimum-gas-prices = '0stake' app-db-backend = 'goleveldb' [store.options] -# SState storage database type. Currently we support: "sqlite", "pebble" and "rocksdb" +# State storage database type. Currently we support: "sqlite", "pebble" and "rocksdb" ss-type = 'sqlite' # State commitment database type. Currently we support: "iavl" and "iavl-v2" sc-type = 'iavl' From 6e737e637a1be543b3c4d9ca18349f79c84c0fd0 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 11 Oct 2024 22:40:12 +0200 Subject: [PATCH 4/5] updates --- store/v2/commitment/store.go | 5 +---- store/v2/root/factory.go | 2 +- store/v2/root/migrate_test.go | 2 +- store/v2/root/store.go | 6 ++++++ store/v2/root/store_test.go | 6 +++--- store/v2/root/upgrade_test.go | 4 ++-- 6 files changed, 14 insertions(+), 11 deletions(-) diff --git a/store/v2/commitment/store.go b/store/v2/commitment/store.go index 98765dfb06be..da0440987524 100644 --- a/store/v2/commitment/store.go +++ b/store/v2/commitment/store.go @@ -38,7 +38,6 @@ type MountTreeFn func(storeKey string) (Tree, error) // and trees. type CommitStore struct { logger corelog.Logger - db corestore.KVStoreWithBatch // holds the db instance for closing it metadata *MetadataStore multiTrees map[string]Tree // oldTrees is a map of store keys to old trees that have been deleted or renamed. @@ -50,7 +49,6 @@ type CommitStore struct { func NewCommitStore(trees, oldTrees map[string]Tree, db corestore.KVStoreWithBatch, logger corelog.Logger) (*CommitStore, error) { return &CommitStore{ logger: logger, - db: db, multiTrees: trees, oldTrees: oldTrees, metadata: NewMetadataStore(db), @@ -501,6 +499,5 @@ func (c *CommitStore) Close() error { } } - // close the db - return c.db.Close() + return nil } diff --git a/store/v2/root/factory.go b/store/v2/root/factory.go index 83f6b85125aa..2511a53b434e 100644 --- a/store/v2/root/factory.go +++ b/store/v2/root/factory.go @@ -177,5 +177,5 @@ func CreateRootStore(opts *FactoryOptions) (store.RootStore, error) { } pm := pruning.NewManager(sc, ss, storeOpts.SCPruningOption, storeOpts.SSPruningOption) - return New(opts.Logger, ss, sc, pm, nil, nil) + return New(opts.SCRawDB, opts.Logger, ss, sc, pm, nil, nil) } diff --git a/store/v2/root/migrate_test.go b/store/v2/root/migrate_test.go index 0cc20ae940d9..9bd8dfdd2ef8 100644 --- a/store/v2/root/migrate_test.go +++ b/store/v2/root/migrate_test.go @@ -80,7 +80,7 @@ func (s *MigrateStoreTestSuite) SetupTest() { pm := pruning.NewManager(sc, ss, nil, nil) // assume no storage store, simulate the migration process - s.rootStore, err = New(testLog, ss, orgSC, pm, migrationManager, nil) + s.rootStore, err = New(dbm.NewMemDB(), testLog, ss, orgSC, pm, migrationManager, nil) s.Require().NoError(err) } diff --git a/store/v2/root/store.go b/store/v2/root/store.go index c39119a9aaf3..443c4ec4b4a7 100644 --- a/store/v2/root/store.go +++ b/store/v2/root/store.go @@ -33,6 +33,9 @@ type Store struct { logger corelog.Logger initialVersion uint64 + // holds the db instance for closing it + db corestore.KVStoreWithBatch + // stateStorage reflects the state storage backend stateStorage store.VersionedDatabase @@ -68,6 +71,7 @@ type Store struct { // // NOTE: The migration manager is optional and can be nil if no migration is required. func New( + db corestore.KVStoreWithBatch, logger corelog.Logger, ss store.VersionedDatabase, sc store.Committer, @@ -76,6 +80,7 @@ func New( m metrics.StoreMetrics, ) (store.RootStore, error) { return &Store{ + db: db, logger: logger, initialVersion: 1, stateStorage: ss, @@ -92,6 +97,7 @@ func New( func (s *Store) Close() (err error) { err = errors.Join(err, s.stateStorage.Close()) err = errors.Join(err, s.stateCommitment.Close()) + err = errors.Join(err, s.db.Close()) s.stateStorage = nil s.stateCommitment = nil diff --git a/store/v2/root/store_test.go b/store/v2/root/store_test.go index e8cf34e014e0..9f925b098b9c 100644 --- a/store/v2/root/store_test.go +++ b/store/v2/root/store_test.go @@ -59,7 +59,7 @@ func (s *RootStoreTestSuite) SetupTest() { s.Require().NoError(err) pm := pruning.NewManager(sc, ss, nil, nil) - rs, err := New(noopLog, ss, sc, pm, nil, nil) + rs, err := New(dbm.NewMemDB(), noopLog, ss, sc, pm, nil, nil) s.Require().NoError(err) s.rootStore = rs @@ -84,7 +84,7 @@ func (s *RootStoreTestSuite) newStoreWithPruneConfig(config *store.PruningOption pm := pruning.NewManager(sc, ss, config, config) - rs, err := New(noopLog, ss, sc, pm, nil, nil) + rs, err := New(dbm.NewMemDB(), noopLog, ss, sc, pm, nil, nil) s.Require().NoError(err) s.rootStore = rs @@ -93,7 +93,7 @@ func (s *RootStoreTestSuite) newStoreWithPruneConfig(config *store.PruningOption func (s *RootStoreTestSuite) newStoreWithBackendMount(ss store.VersionedDatabase, sc store.Committer, pm *pruning.Manager) { noopLog := coretesting.NewNopLogger() - rs, err := New(noopLog, ss, sc, pm, nil, nil) + rs, err := New(dbm.NewMemDB(), noopLog, ss, sc, pm, nil, nil) s.Require().NoError(err) s.rootStore = rs diff --git a/store/v2/root/upgrade_test.go b/store/v2/root/upgrade_test.go index a4aee1d5bcfa..47c2882dc2ef 100644 --- a/store/v2/root/upgrade_test.go +++ b/store/v2/root/upgrade_test.go @@ -50,7 +50,7 @@ func (s *UpgradeStoreTestSuite) SetupTest() { sc, err := commitment.NewCommitStore(multiTrees, nil, s.commitDB, testLog) s.Require().NoError(err) pm := pruning.NewManager(sc, ss, nil, nil) - s.rootStore, err = New(testLog, ss, sc, pm, nil, nil) + s.rootStore, err = New(s.commitDB, testLog, ss, sc, pm, nil, nil) s.Require().NoError(err) // commit changeset @@ -92,7 +92,7 @@ func (s *UpgradeStoreTestSuite) loadWithUpgrades(upgrades *corestore.StoreUpgrad sc, err := commitment.NewCommitStore(multiTrees, oldTrees, s.commitDB, testLog) s.Require().NoError(err) pm := pruning.NewManager(sc, s.rootStore.GetStateStorage().(store.Pruner), nil, nil) - s.rootStore, err = New(testLog, s.rootStore.GetStateStorage(), sc, pm, nil, nil) + s.rootStore, err = New(s.commitDB, testLog, s.rootStore.GetStateStorage(), sc, pm, nil, nil) s.Require().NoError(err) } From 9b6e888c2217fc08f5e15613bd92849d9a1b16f2 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Sat, 12 Oct 2024 10:04:02 +0200 Subject: [PATCH 5/5] updates --- store/v2/root/store.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/store/v2/root/store.go b/store/v2/root/store.go index 443c4ec4b4a7..7447c60ae7b2 100644 --- a/store/v2/root/store.go +++ b/store/v2/root/store.go @@ -5,6 +5,7 @@ import ( "crypto/sha256" "errors" "fmt" + "io" "sync" "time" @@ -34,7 +35,7 @@ type Store struct { initialVersion uint64 // holds the db instance for closing it - db corestore.KVStoreWithBatch + dbCloser io.Closer // stateStorage reflects the state storage backend stateStorage store.VersionedDatabase @@ -71,7 +72,7 @@ type Store struct { // // NOTE: The migration manager is optional and can be nil if no migration is required. func New( - db corestore.KVStoreWithBatch, + dbCloser io.Closer, logger corelog.Logger, ss store.VersionedDatabase, sc store.Committer, @@ -80,7 +81,7 @@ func New( m metrics.StoreMetrics, ) (store.RootStore, error) { return &Store{ - db: db, + dbCloser: dbCloser, logger: logger, initialVersion: 1, stateStorage: ss, @@ -97,7 +98,7 @@ func New( func (s *Store) Close() (err error) { err = errors.Join(err, s.stateStorage.Close()) err = errors.Join(err, s.stateCommitment.Close()) - err = errors.Join(err, s.db.Close()) + err = errors.Join(err, s.dbCloser.Close()) s.stateStorage = nil s.stateCommitment = nil