From 8eccaf95358124b595659b55c465ff2228c7168b Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Thu, 29 Apr 2021 15:33:59 -0400 Subject: [PATCH] mempool: remove vestigal mempool wal (#6396) --- CHANGELOG_PENDING.md | 1 + config/config.go | 12 ------- config/config_test.go | 3 -- config/toml.go | 1 - mempool/clist_mempool.go | 46 +------------------------ mempool/clist_mempool_test.go | 63 ----------------------------------- mempool/mempool.go | 8 ----- node/node.go | 13 -------- 8 files changed, 2 insertions(+), 145 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 7adb14366..718a7592c 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -52,6 +52,7 @@ Friendly reminder: We have a [bug bounty program](https://hackerone.com/tendermi - Data Storage - [store/state/evidence/light] \#5771 Use an order-preserving varint key encoding (@cmwaters) + - [mempool] \#6396 Remove mempool's write ahead log (WAL), (previously unused by the tendermint code). (@tychoish) ### FEATURES diff --git a/config/config.go b/config/config.go index 8d1a75a81..681f0c9d9 100644 --- a/config/config.go +++ b/config/config.go @@ -720,7 +720,6 @@ type MempoolConfig struct { RootDir string `mapstructure:"home"` Recheck bool `mapstructure:"recheck"` Broadcast bool `mapstructure:"broadcast"` - WalPath string `mapstructure:"wal-dir"` // Maximum number of transactions in the mempool Size int `mapstructure:"size"` // Limit the total size of all txs in the mempool. @@ -747,7 +746,6 @@ func DefaultMempoolConfig() *MempoolConfig { return &MempoolConfig{ Recheck: true, Broadcast: true, - WalPath: "", // Each signature verification takes .5ms, Size reduced until we implement // ABCI Recheck Size: 5000, @@ -764,16 +762,6 @@ func TestMempoolConfig() *MempoolConfig { return cfg } -// WalDir returns the full path to the mempool's write-ahead log -func (cfg *MempoolConfig) WalDir() string { - return rootify(cfg.WalPath, cfg.RootDir) -} - -// WalEnabled returns true if the WAL is enabled. -func (cfg *MempoolConfig) WalEnabled() bool { - return cfg.WalPath != "" -} - // ValidateBasic performs basic validation (checking param bounds, etc.) and // returns an error if any check fails. func (cfg *MempoolConfig) ValidateBasic() error { diff --git a/config/config_test.go b/config/config_test.go index 78657d848..93bbc73bf 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -22,12 +22,9 @@ func TestDefaultConfig(t *testing.T) { cfg.SetRoot("/foo") cfg.Genesis = "bar" cfg.DBPath = "/opt/data" - cfg.Mempool.WalPath = "wal/mem/" assert.Equal("/foo/bar", cfg.GenesisFile()) assert.Equal("/opt/data", cfg.DBDir()) - assert.Equal("/foo/wal/mem", cfg.Mempool.WalDir()) - } func TestConfigValidateBasic(t *testing.T) { diff --git a/config/toml.go b/config/toml.go index 03fe40cf1..709882462 100644 --- a/config/toml.go +++ b/config/toml.go @@ -357,7 +357,6 @@ dial-timeout = "{{ .P2P.DialTimeout }}" recheck = {{ .Mempool.Recheck }} broadcast = {{ .Mempool.Broadcast }} -wal-dir = "{{ js .Mempool.WalPath }}" # Maximum number of transactions in the mempool size = {{ .Mempool.Size }} diff --git a/mempool/clist_mempool.go b/mempool/clist_mempool.go index d1a685b32..7d486ad69 100644 --- a/mempool/clist_mempool.go +++ b/mempool/clist_mempool.go @@ -11,11 +11,9 @@ import ( abci "github.com/tendermint/tendermint/abci/types" cfg "github.com/tendermint/tendermint/config" - auto "github.com/tendermint/tendermint/libs/autofile" "github.com/tendermint/tendermint/libs/clist" "github.com/tendermint/tendermint/libs/log" tmmath "github.com/tendermint/tendermint/libs/math" - tmos "github.com/tendermint/tendermint/libs/os" tmsync "github.com/tendermint/tendermint/libs/sync" "github.com/tendermint/tendermint/p2p" "github.com/tendermint/tendermint/proxy" @@ -25,8 +23,6 @@ import ( // TxKeySize is the size of the transaction key index const TxKeySize = sha256.Size -var newline = []byte("\n") - //-------------------------------------------------------------------------------- // CListMempool is an ordered in-memory pool for transactions before they are @@ -51,8 +47,7 @@ type CListMempool struct { preCheck PreCheckFunc postCheck PostCheckFunc - wal *auto.AutoFile // a log of mempool txs - txs *clist.CList // concurrent linked-list of good txs + txs *clist.CList // concurrent linked-list of good txs proxyAppConn proxy.AppConnMempool // Track whether we're rechecking txs. @@ -137,33 +132,6 @@ func WithMetrics(metrics *Metrics) CListMempoolOption { return func(mem *CListMempool) { mem.metrics = metrics } } -func (mem *CListMempool) InitWAL() error { - var ( - walDir = mem.config.WalDir() - walFile = walDir + "/wal" - ) - - const perm = 0700 - if err := tmos.EnsureDir(walDir, perm); err != nil { - return err - } - - af, err := auto.OpenAutoFile(walFile) - if err != nil { - return fmt.Errorf("can't open autofile %s: %w", walFile, err) - } - - mem.wal = af - return nil -} - -func (mem *CListMempool) CloseWAL() { - if err := mem.wal.Close(); err != nil { - mem.logger.Error("Error closing WAL", "err", err) - } - mem.wal = nil -} - // Safe for concurrent use by multiple goroutines. func (mem *CListMempool) Lock() { mem.updateMtx.Lock() @@ -253,18 +221,6 @@ func (mem *CListMempool) CheckTx(tx types.Tx, cb func(*abci.Response), txInfo Tx } } - // NOTE: writing to the WAL and calling proxy must be done before adding tx - // to the cache. otherwise, if either of them fails, next time CheckTx is - // called with tx, ErrTxInCache will be returned without tx being checked at - // all even once. - if mem.wal != nil { - // TODO: Notify administrators when WAL fails - _, err := mem.wal.Write(append([]byte(tx), newline...)) - if err != nil { - return fmt.Errorf("wal.Write: %w", err) - } - } - // NOTE: proxyAppConn may error if tx buffer is full if err := mem.proxyAppConn.Error(); err != nil { return err diff --git a/mempool/clist_mempool_test.go b/mempool/clist_mempool_test.go index b43888df6..57357b954 100644 --- a/mempool/clist_mempool_test.go +++ b/mempool/clist_mempool_test.go @@ -3,13 +3,10 @@ package mempool import ( "context" "crypto/rand" - "crypto/sha256" "encoding/binary" "fmt" - "io/ioutil" mrand "math/rand" "os" - "path/filepath" "testing" "time" @@ -418,55 +415,6 @@ func TestSerialReap(t *testing.T) { reapCheck(600) } -func TestMempoolCloseWAL(t *testing.T) { - // 1. Create the temporary directory for mempool and WAL testing. - rootDir, err := ioutil.TempDir("", "mempool-test") - require.Nil(t, err, "expecting successful tmpdir creation") - - // 2. Ensure that it doesn't contain any elements -- Sanity check - m1, err := filepath.Glob(filepath.Join(rootDir, "*")) - require.Nil(t, err, "successful globbing expected") - require.Equal(t, 0, len(m1), "no matches yet") - - // 3. Create the mempool - wcfg := cfg.DefaultConfig() - wcfg.Mempool.RootDir = rootDir - app := kvstore.NewApplication() - cc := proxy.NewLocalClientCreator(app) - mempool, cleanup := newMempoolWithAppAndConfig(cc, wcfg) - defer cleanup() - mempool.height = 10 - err = mempool.InitWAL() - require.NoError(t, err) - - // 4. Ensure that the directory contains the WAL file - m2, err := filepath.Glob(filepath.Join(rootDir, "*")) - require.Nil(t, err, "successful globbing expected") - require.Equal(t, 1, len(m2), "expecting the wal match in") - - // 5. Write some contents to the WAL - err = mempool.CheckTx(types.Tx([]byte("foo")), nil, TxInfo{}) - require.NoError(t, err) - walFilepath := mempool.wal.Path - sum1 := checksumFile(walFilepath, t) - - // 6. Sanity check to ensure that the written TX matches the expectation. - require.Equal(t, sum1, checksumIt([]byte("foo\n")), "foo with a newline should be written") - - // 7. Invoke CloseWAL() and ensure it discards the - // WAL thus any other write won't go through. - mempool.CloseWAL() - err = mempool.CheckTx(types.Tx([]byte("bar")), nil, TxInfo{}) - require.NoError(t, err) - sum2 := checksumFile(walFilepath, t) - require.Equal(t, sum1, sum2, "expected no change to the WAL after invoking CloseWAL() since it was discarded") - - // 8. Sanity check to ensure that the WAL file still exists - m3, err := filepath.Glob(filepath.Join(rootDir, "*")) - require.Nil(t, err, "successful globbing expected") - require.Equal(t, 1, len(m3), "expecting the wal match in") -} - func TestMempool_CheckTxChecksTxSize(t *testing.T) { app := kvstore.NewApplication() cc := proxy.NewLocalClientCreator(app) @@ -651,17 +599,6 @@ func newRemoteApp( } return clientCreator, server } -func checksumIt(data []byte) string { - h := sha256.New() - h.Write(data) - return fmt.Sprintf("%x", h.Sum(nil)) -} - -func checksumFile(p string, t *testing.T) string { - data, err := ioutil.ReadFile(p) - require.Nil(t, err, "expecting successful read of %q", p) - return checksumIt(data) -} func abciResponses(n int, code uint32) []*abci.ResponseDeliverTx { responses := make([]*abci.ResponseDeliverTx, 0, n) diff --git a/mempool/mempool.go b/mempool/mempool.go index b7a315d30..74e328bae 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -69,14 +69,6 @@ type Mempool interface { // TxsBytes returns the total size of all txs in the mempool. TxsBytes() int64 - - // InitWAL creates a directory for the WAL file and opens a file itself. If - // there is an error, it will be of type *PathError. - InitWAL() error - - // CloseWAL closes and discards the underlying WAL file. - // Any further writes will not be relayed to disk. - CloseWAL() } //-------------------------------------------------------------------------------- diff --git a/node/node.go b/node/node.go index fbf47d257..57416ca18 100644 --- a/node/node.go +++ b/node/node.go @@ -1322,14 +1322,6 @@ func (n *Node) OnStart() error { n.prometheusSrv = n.startPrometheusServer(n.config.Instrumentation.PrometheusListenAddr) } - // Start the mempool. - if n.config.Mempool.WalEnabled() { - err := n.mempool.InitWAL() - if err != nil { - return fmt.Errorf("init mempool WAL: %w", err) - } - } - // Start the transport. addr, err := p2p.NewNetAddressString(p2p.IDAddressString(n.nodeKey.ID, n.config.P2P.ListenAddress)) if err != nil { @@ -1469,11 +1461,6 @@ func (n *Node) OnStop() { } } - // stop mempool WAL - if n.config.Mempool.WalEnabled() { - n.mempool.CloseWAL() - } - if err := n.transport.Close(); err != nil { n.Logger.Error("Error closing transport", "err", err) }