tendermint: Mempool: do not store tx in cache if mempool full

Tendermint version (use tendermint version or git rev-parse --verify HEAD if installed from source):

0.35

ABCI app (name for built-in, URL for self-written if it’s publicly available):

What happened:

After executing some benchmarks on our tendermint based application using the last 0.35 release, I observed a weird behavior on the mempool. When sending a huge number of transactions to the network distributing them through multiple nodes (some validators and some just full nodes) at the end only 60-70% of them become included in some block while the rest got lost.

The miners have a mempool of 5000 transactions while the full-nodes (receiving the transactions) have a mempool 10 times bigger. We use KeppInvalidTxsInCache=true since in our logic a transaction cannot be valid after being invalid once. Our Cache size is 100k transactions.

In the miner logs I can see the following message appearing many times:

2022-01-28T18:07:08Z ERROR rejected incoming good transaction; mempool full err="mempool is full: number of txs 5000 (max: 5000), total txs bytes 3512534 (max: 512000000)" module=mempool tx=749FC7E3ED60924BD60B109ED30B161EC921D34246C06D1CD47886D936C41E44 version=v1

So my guess is that eventually the mempool of the miners becomes full (5000 txs) and the rest of transactions (sent by other nodes in the network) are considered invalid (they are rejected). Since we use KeppInvalidTxsInCache=true, the transactions are not removed from Cache, thus they will not be accepted anymore. On the origin node (holding the transaction) the TTL is reached (15 minutes) and the transaction are finally removed.

What you expected to happen:

A mempool TX should only be considered invalid (and stay in cache if KeppInvalidTxsInCache=true) if-and-only-if CheckTx() returns non zero. But if the TX is rejected because of mempool full, it should not be stored in the cache and should be accepted afterwars, even if KeppInvalidTxsInCache=true

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 18 (18 by maintainers)

Commits related to this issue

mempool: fix benchmark CheckTx for hitting the GetEvictableTxs call (#7796) Based on the discussion in #7723, make the CheckTx benchmark exercise GetEvictableTxs which is one of the critical paths ... — committed to tendermint/tendermint by JayT106 2 years ago

Most upvoted comments

So I managed to chart the block size using the following query: delta(tendermint_consensus_block_size_bytes_sum{instance="foo.bar"}[2m])/delta(tendermint_consensus_latest_block_height{instance="foo.bar"}[2m])/(1024)

And I confirm blocks are full (5 MiB).

p4u on Jan 31, 2022

You’d just need to set tconfig.Mempool.Version to "v0"

cmwaters on Feb 15, 2022

iiuc, proxyAppConn.CheckTxAsync returns the priority in the result. When we store a Tx in the mempool we also keep a heap (priorityIndex) with highest priority at the top. This is useful for the ReapMax... cases but not for GetEvictableTxs when we want the lowest priority item. Having another heap (with lowest prio at top) or a different datastructure would help with performance here.

ancazamfir on Feb 10, 2022