mempool: Make mempool.update release a mutex for Reap earlier

[ValarDragon]

Summary

Make the clist_mempool release a write lock suitable for reaping early once its recheck'd MaxBlockBytes worth of txs.

Currently the clist_mempool has an update mutex that gets write-locked when you receive a new block, and readlocked when receiving a new tx and when proposing a block. This design makes it risky for validators to have large mempool sizes if the application has a notable tx recheck time.

However there is not that much contention between any of these methods. Instead we could make more granular locks to capture where contention is possible. The needs we have are:

• Update rechecks and potentially removes each tx, starting from the beginning and going to the end of the list.
• Reap needs the first MaxBytes worth of txs to be free.
• CheckTx checks a Tx and appends it to the end of the clist.

We can mitigate the contention between update and reap by having Update make a separate write lock on the first max bytes worth of txs until its done with that many bytes. (So the release conditions are recheck finishes the entire mempool, or MaxTxBytes have passed recheck) Then reap only depends on the write lock for the first max bytes worth of txs.

We can still keep CheckTx write locked as before. It is possible that we could deflate the lock contention for CheckTx, but I think approaches to do so for the current clist_mempool will not be useful for future prioritized mempools, hence I don't suggest doing so now. I do think lower the conflation in locks for Update and Reap will be useful even in future prioritized mempool designs. (E.g. the lock free mempool proposed in #2147)

This issue arose from ABCI++ discussions, but is not related to ABCI++.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[alexanderbez]

What's the concrete proposal here @ValarDragon? The Reap* methods obtain read-lock automatically, whereas Update requires the caller to explicitly acquire a write-lock via Lock.

[ValarDragon]

Its to change the write lock for update to be managed internally to the method, and to instead be semantically changed into two different write locks, one for the first MaxBytes worth of txs, and the second for all remaining txs. Then Reap's read lock only needs to get a lock on the first MaxBytes worth of txs, not all txs.

This makes larger mempool sizes then safe for apps with long rechecks

[alexanderbez]

How much of a priority or necessity do you think this is @ValarDragon? Does this help Osmosis? If so, in what ways? Do you have benchmarks?

[ValarDragon]

This is not at all a significant issue, nor really relevant to osmosis / gaia imo beyond a general desire to safely allow for larger mempools.

This is something that came up as a side-idea on one of the ABCI++ calls.