perf(consensus): Make late votes outside of last block commits not get to peerMsgQueue #3157
Conversation
There was a problem hiding this comment.
We should not spend much resources on messages (any type) which are old and useless for the consensus protocol. So, the goal here is perfectly legit.
But adding the solution to the consensus reactor is not really the best approach. In fact, the consensus reactor should not take the consensus state lock for every received vote, which is super inefficient in any case. While this PR hasn't introduced this behavior, if we really want to decouple reactor and state, this solution goes towards the wrong direction.
I would instead add this check to the state implementation. The goal is anyway to prevent writing this useless vote to the WAL and keeping the consensus lock for longer than needed.
.changelog/unreleased/improvements/3157-late-votes-dont-take-cs-mtx.md
Outdated
Show resolved
Hide resolved
My thinking right now is that we can get the lock removed with #3211 , and then have this able to proceed concurrently (not under the single threaded state.go workload). So we would rather it happen in the reactor |
* Backport cometbft#3211 * Fix Race * bp cometbft#3157 * Speedup tests that were hitting timeouts * bp cometbft#3161 * Fix data race * Mempool async processing * Forgot to commit important part * Add changelog
* Backport cometbft#3211 * Fix Race * bp cometbft#3157 * Speedup tests that were hitting timeouts * bp cometbft#3161 * Fix data race * Mempool async processing * Forgot to commit important part * Add changelog
Closes #3154 . Make late votes outside of last blocks precommits not get into the peer msg queue. This lowers the delay for processing more important messages from the peerMsgQueue, and lowers WAL write delays. As noted in the github issue, the number of late votes is actually quite significant.
I don't think theres any test updates needed (all relevant behavior is covered under existing tests. And if you look into the consensus logic, aside from WAL write, all we'd otherwise do is:
We could still do that if we wanted? But doesn't seem that important, since we have the mark late vote still being sent.
PR checklist
.changelog(we use unclog to manage our changelog)docs/orspec/) and code comments