From ae275d791e7e802b6fec9243af56c7a3b53f6a08 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Sat, 15 Dec 2018 23:27:02 +0400 Subject: [PATCH] mempool: notifyTxsAvailable if there're txs left, but recheck=false (#2991) (left after committing a block) Fixes #2961 -------------- ORIGINAL ISSUE Tendermint version : 0.26.4-b771798d ABCI app : kv-store Environment: OS (e.g. from /etc/os-release): macOS 10.14.1 What happened: Set mempool.recheck = false and create empty block = false in config.toml. When transactions get added right between a new empty block is being proposed and committed, the proposer won't propose new block for that transactions immediately after. That transactions are stuck in the mempool until a new transaction is added and trigger the proposer. What you expected to happen: If there is a transaction left in the mempool, new block should be proposed immediately. Have you tried the latest version: yes How to reproduce it (as minimally and precisely as possible): Fire two transaction using broadcast_tx_sync with specific delay between them. (You may need to do it multiple time before the right delay is found, on my machine the delay is 0.98s) Logs (paste a small part showing an error (< 10 lines) or link a pastebin, gist, etc. containing more of the log file): https://pastebin.com/0Wt6uhPF Config (you can paste only the changes you've made): [mempool] recheck = false create_empty_block = false Anything else we need to know: In mempool.go, we found that proposer will immediately propose new block if Last committed block has some transaction (causing AppHash to changed) or mem.notifyTxsAvailable() is called. Our scenario is as followed. A transaction is fired, it will create 1 block with 1 tx (line 1-4 in the log) and 1 empty block. After the empty block is proposed but before it is committed, second transaction is fired and added to mempool. (line 8-16) Now, since the last committed block is empty and mem.notifyTxsAvailable() will be called only if mempool.recheck = true. The proposer won't immediately propose new block, causing the second transaction to stuck in mempool until another transaction is added to mempool and trigger mem.notifyTxsAvailable(). --- CHANGELOG_PENDING.md | 1 + mempool/mempool.go | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 3cc491fe8..bcbeb4963 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -24,5 +24,6 @@ Special thanks to external contributors on this release: ### BUG FIXES: - [kv indexer] \#2912 don't ignore key when executing CONTAINS +- [mempool] \#2961 notifyTxsAvailable if there're txs left after committing a block, but recheck=false - [mempool] \#2994 Don't allow txs with negative gas wanted - [p2p] \#2715 fix a bug where seeds don't disconnect from a peer after 3h diff --git a/mempool/mempool.go b/mempool/mempool.go index 404683158..c5f966c4e 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -556,13 +556,18 @@ func (mem *Mempool) Update( // Remove committed transactions. txsLeft := mem.removeTxs(txs) - // Recheck mempool txs if any txs were committed in the block - if mem.config.Recheck && len(txsLeft) > 0 { - mem.logger.Info("Recheck txs", "numtxs", len(txsLeft), "height", height) - mem.recheckTxs(txsLeft) - // At this point, mem.txs are being rechecked. - // mem.recheckCursor re-scans mem.txs and possibly removes some txs. - // Before mem.Reap(), we should wait for mem.recheckCursor to be nil. + // Either recheck non-committed txs to see if they became invalid + // or just notify there're some txs left. + if len(txsLeft) > 0 { + if mem.config.Recheck { + mem.logger.Info("Recheck txs", "numtxs", len(txsLeft), "height", height) + mem.recheckTxs(txsLeft) + // At this point, mem.txs are being rechecked. + // mem.recheckCursor re-scans mem.txs and possibly removes some txs. + // Before mem.Reap(), we should wait for mem.recheckCursor to be nil. + } else { + mem.notifyTxsAvailable() + } } // Update metrics