From cd21c0851a29714665c18937df77217b9ba454e9 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Thu, 1 Apr 2021 11:24:23 -0400 Subject: [PATCH 1/3] test: clean up databases in tests (#6304) --- abci/example/kvstore/persistent_kvstore.go | 4 ++++ consensus/replay_test.go | 9 ++++++--- consensus/wal_generator.go | 2 ++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/abci/example/kvstore/persistent_kvstore.go b/abci/example/kvstore/persistent_kvstore.go index 9e259d2d0d..1ec13d353c 100644 --- a/abci/example/kvstore/persistent_kvstore.go +++ b/abci/example/kvstore/persistent_kvstore.go @@ -56,6 +56,10 @@ func NewPersistentKVStoreApplication(dbDir string) *PersistentKVStoreApplication } } +func (app *PersistentKVStoreApplication) Close() error { + return app.app.state.db.Close() +} + func (app *PersistentKVStoreApplication) SetLogger(l log.Logger) { app.logger = l } diff --git a/consensus/replay_test.go b/consensus/replay_test.go index 523c8ee547..15931677a7 100644 --- a/consensus/replay_test.go +++ b/consensus/replay_test.go @@ -1156,6 +1156,7 @@ func testHandshakeReplay( // make a new client creator kvstoreApp := kvstore.NewPersistentKVStoreApplication( filepath.Join(config.DBDir(), fmt.Sprintf("replay_test_%d_%d_a", nBlocks, mode))) + t.Cleanup(func() { require.NoError(t, kvstoreApp.Close()) }) clientCreator2 := proxy.NewLocalClientCreator(kvstoreApp) if nBlocks > 0 { @@ -1328,9 +1329,11 @@ func buildTMStateFromChain( nBlocks int, mode uint) sm.State { // run the whole chain against this client to build up the tendermint state - clientCreator := proxy.NewLocalClientCreator( - kvstore.NewPersistentKVStoreApplication( - filepath.Join(config.DBDir(), fmt.Sprintf("replay_test_%d_%d_t", nBlocks, mode)))) + kvstoreApp := kvstore.NewPersistentKVStoreApplication( + filepath.Join(config.DBDir(), fmt.Sprintf("replay_test_%d_%d_t", nBlocks, mode))) + defer kvstoreApp.Close() + clientCreator := proxy.NewLocalClientCreator(kvstoreApp) + proxyApp := proxy.NewAppConns(clientCreator) if err := proxyApp.Start(); err != nil { panic(err) diff --git a/consensus/wal_generator.go b/consensus/wal_generator.go index 5568ba7963..1369430083 100644 --- a/consensus/wal_generator.go +++ b/consensus/wal_generator.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" db "github.com/tendermint/tm-db" "github.com/tendermint/tendermint/abci/example/kvstore" @@ -31,6 +32,7 @@ func WALGenerateNBlocks(t *testing.T, wr io.Writer, numBlocks int) (err error) { config := getConfig(t) app := kvstore.NewPersistentKVStoreApplication(filepath.Join(config.DBDir(), "wal_generator")) + t.Cleanup(func() { require.NoError(t, app.Close()) }) logger := log.TestingLogger().With("wal_generator", "wal_generator") logger.Info("generating WAL (last height msg excluded)", "numBlocks", numBlocks) From 73cceaf945d6a74c4513a7a2646e432c6f16d212 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Mon, 5 Apr 2021 12:39:04 -0400 Subject: [PATCH 2/3] test: improve cleanup for data and disk use (#6311) --- consensus/common_test.go | 12 ++++++++++++ consensus/replay_file.go | 2 +- node/node.go | 3 ++- node/node_test.go | 4 +++- proxy/client.go | 23 ++++++++++++++++------- rpc/client/main_test.go | 1 + test/maverick/consensus/replay_file.go | 2 +- test/maverick/node/node.go | 3 ++- 8 files changed, 38 insertions(+), 12 deletions(-) diff --git a/consensus/common_test.go b/consensus/common_test.go index c317d2a22c..76bb80b6be 100644 --- a/consensus/common_test.go +++ b/consensus/common_test.go @@ -5,6 +5,7 @@ import ( "bytes" "context" "fmt" + "io" "io/ioutil" "os" "path/filepath" @@ -713,7 +714,10 @@ func randConsensusNet(nValidators int, initialHeight int64, testName string, tic genDoc, privVals := randGenesisDoc(nValidators, false, 30, initialHeight) css := make([]*State, nValidators) logger := consensusLogger() + + closeFuncs := make([]func() error, 0, nValidators) configRootDirs := make([]string, 0, nValidators) + for i := 0; i < nValidators; i++ { stateDB := dbm.NewMemDB() // each state needs its own db stateStore := sm.NewStore(stateDB) @@ -725,6 +729,11 @@ func randConsensusNet(nValidators int, initialHeight int64, testName string, tic } ensureDir(filepath.Dir(thisConfig.Consensus.WalFile()), 0700) // dir for wal app := appFunc() + + if appCloser, ok := app.(io.Closer); ok { + closeFuncs = append(closeFuncs, appCloser.Close) + } + vals := types.TM2PB.ValidatorUpdates(state.Validators) app.InitChain(abci.RequestInitChain{ValidatorSet: &vals}) @@ -733,6 +742,9 @@ func randConsensusNet(nValidators int, initialHeight int64, testName string, tic css[i].SetLogger(logger.With("validator", i, "module", "consensus")) } return css, func() { + for _, closer := range closeFuncs { + _ = closer() + } for _, dir := range configRootDirs { os.RemoveAll(dir) } diff --git a/consensus/replay_file.go b/consensus/replay_file.go index da71a00b0b..0dbb4df008 100644 --- a/consensus/replay_file.go +++ b/consensus/replay_file.go @@ -318,7 +318,7 @@ func newConsensusStateForReplay(config cfg.BaseConfig, csConfig *cfg.ConsensusCo } // Create proxyAppConn connection (consensus, mempool, query) - clientCreator := proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()) + clientCreator, _ := proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()) proxyApp := proxy.NewAppConns(clientCreator) err = proxyApp.Start() if err != nil { diff --git a/node/node.go b/node/node.go index 28c6a8ea97..63fc1e4ffc 100644 --- a/node/node.go +++ b/node/node.go @@ -97,9 +97,10 @@ func DefaultNewNode(config *cfg.Config, logger log.Logger) (*Node, error) { return nil, fmt.Errorf("failed to load or gen node key %s: %w", config.NodeKeyFile(), err) } + appClient, _ := proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()) return NewNode(config, nodeKey, - proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()), + appClient, DefaultGenesisDocProviderFunc(config), DefaultDBProvider, DefaultMetricsProvider(config.Instrumentation), diff --git a/node/node_test.go b/node/node_test.go index b566f47b06..92186a59ed 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -428,9 +428,11 @@ func TestNodeNewNodeCustomReactors(t *testing.T) { nodeKey, err := p2p.LoadOrGenNodeKey(config.NodeKeyFile()) require.NoError(t, err) + appClient, closer := proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()) + t.Cleanup(func() { closer.Close() }) n, err := NewNode(config, nodeKey, - proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()), + appClient, DefaultGenesisDocProviderFunc(config), DefaultDBProvider, DefaultMetricsProvider(config.Instrumentation), diff --git a/proxy/client.go b/proxy/client.go index 27baa9738a..f57c603ca2 100644 --- a/proxy/client.go +++ b/proxy/client.go @@ -2,6 +2,7 @@ package proxy import ( "fmt" + "io" abcicli "github.com/tendermint/tendermint/abci/client" "github.com/tendermint/tendermint/abci/example/counter" @@ -69,20 +70,28 @@ func (r *remoteClientCreator) NewABCIClient() (abcicli.Client, error) { // DefaultClientCreator returns a default ClientCreator, which will create a // local client if addr is one of: 'counter', 'counter_serial', 'kvstore', // 'persistent_kvstore' or 'noop', otherwise - a remote client. -func DefaultClientCreator(addr, transport, dbDir string) ClientCreator { +// +// The Closer is a noop except for persistent_kvstore applications, +// which will clean up the store. +func DefaultClientCreator(addr, transport, dbDir string) (ClientCreator, io.Closer) { switch addr { case "counter": - return NewLocalClientCreator(counter.NewApplication(false)) + return NewLocalClientCreator(counter.NewApplication(false)), noopCloser{} case "counter_serial": - return NewLocalClientCreator(counter.NewApplication(true)) + return NewLocalClientCreator(counter.NewApplication(true)), noopCloser{} case "kvstore": - return NewLocalClientCreator(kvstore.NewApplication()) + return NewLocalClientCreator(kvstore.NewApplication()), noopCloser{} case "persistent_kvstore": - return NewLocalClientCreator(kvstore.NewPersistentKVStoreApplication(dbDir)) + app := kvstore.NewPersistentKVStoreApplication(dbDir) + return NewLocalClientCreator(app), app case "noop": - return NewLocalClientCreator(types.NewBaseApplication()) + return NewLocalClientCreator(types.NewBaseApplication()), noopCloser{} default: mustConnect := false // loop retrying - return NewRemoteClientCreator(addr, transport, mustConnect) + return NewRemoteClientCreator(addr, transport, mustConnect), noopCloser{} } } + +type noopCloser struct{} + +func (noopCloser) Close() error { return nil } diff --git a/rpc/client/main_test.go b/rpc/client/main_test.go index c97311c810..cab8b7cdd4 100644 --- a/rpc/client/main_test.go +++ b/rpc/client/main_test.go @@ -26,6 +26,7 @@ func TestMain(m *testing.M) { // and shut down proper at the end rpctest.StopTendermint(node) + app.Close() _ = os.RemoveAll(dir) os.Exit(code) } diff --git a/test/maverick/consensus/replay_file.go b/test/maverick/consensus/replay_file.go index 9466339ee1..f3624321a7 100644 --- a/test/maverick/consensus/replay_file.go +++ b/test/maverick/consensus/replay_file.go @@ -319,7 +319,7 @@ func newConsensusStateForReplay(config cfg.BaseConfig, csConfig *cfg.ConsensusCo } // Create proxyAppConn connection (consensus, mempool, query) - clientCreator := proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()) + clientCreator, _ := proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()) proxyApp := proxy.NewAppConns(clientCreator) err = proxyApp.Start() if err != nil { diff --git a/test/maverick/node/node.go b/test/maverick/node/node.go index fa9e3f569b..466a176ebc 100644 --- a/test/maverick/node/node.go +++ b/test/maverick/node/node.go @@ -139,9 +139,10 @@ func DefaultNewNode( ) } + appClient, _ := proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()) return NewNode(config, nodeKey, - proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()), + appClient, DefaultGenesisDocProviderFunc(config), DefaultDBProvider, DefaultMetricsProvider(config.Instrumentation), From de0fa2647c9a73e4d36d82630e356b13e222f9ac Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 4 Nov 2021 10:21:00 +0100 Subject: [PATCH 3/3] test: close db in randConsensusNetWithPeers, just as it is in randConsensusNet --- consensus/common_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/consensus/common_test.go b/consensus/common_test.go index 76bb80b6be..3bd30dc138 100644 --- a/consensus/common_test.go +++ b/consensus/common_test.go @@ -933,6 +933,7 @@ func randConsensusNetWithPeers( css := make([]*State, nPeers) logger := consensusLogger() var peer0Config *cfg.Config + closeFuncs := make([]func() error, 0, nValidators) configRootDirs := make([]string, 0, nPeers) for i := 0; i < nPeers; i++ { stateDB := dbm.NewMemDB() // each state needs its own db @@ -965,6 +966,9 @@ func randConsensusNetWithPeers( } app := appFunc(path.Join(config.DBDir(), fmt.Sprintf("%s_%d", testName, i))) + if appCloser, ok := app.(io.Closer); ok { + closeFuncs = append(closeFuncs, appCloser.Close) + } vals := types.TM2PB.ValidatorUpdates(state.Validators) if _, ok := app.(*kvstore.PersistentKVStoreApplication); ok { // simulate handshake, receive app version. If don't do this, replay test will fail @@ -979,6 +983,9 @@ func randConsensusNetWithPeers( css[i].SetLogger(logger.With("validator", i, "proTxHash", proTxHash.ShortString(), "module", "consensus")) } return css, genDoc, peer0Config, func() { + for _, closer := range closeFuncs { + _ = closer() + } for _, dir := range configRootDirs { os.RemoveAll(dir) }