mempool: rework lock discipline to mitigate callback deadlocks #9033
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
creachadair
force-pushed
the
mjf/outer-lock-34
branch
4 times, most recently
from
f3a11d2 to
f2dacf2
Compare
…ort #9030) (manual cherry-pick of commit 22ed610) The priority mempool has a stricter synchronization requirement than the legacy mempool. Under sufficiently-heavy load, exclusive access can lead to deadlocks when processing a large batch of transaction rechecks through an out-of-process application using the socket client. By design, a socket client stalls when its send buffer fills, during which time it holds a lock shared with the receive thread. While blocked in this state, a response read by the receive thread waits for the shared lock so the callback can be invoked. If we're lucky, the server will then read the next request and make enough room in the buffer for the sender to proceed. If not however (e.g., if the next request is bigger than the one just consumed), the receive thread is blocked: It is waiting on the lock and cannot read a response. Once the server's output buffer fills, the system deadlocks. This can happen with any sufficiently-busy workload, but is more likely during a large recheck in the v1 mempool, where the callbacks need exclusive access to mempool state. As a workaround, process rechecks for the priority mempool in their own goroutines outside the mempool mutex. Responses still head-of-line block, but will no longer get pushback due to contention on the mempool itself.
creachadair
force-pushed
the
mjf/outer-lock-34
branch
from
f2dacf2 to
acf4229
Compare
creachadair
requested review from
ebuchman,
cmwaters,
tychoish and
williambanfield
as code owners
williambanfield
approved these changes
Jul 19, 2022
|
The upload-coverage-report failure is not related; it looks like codecov has an expired cert. As far as I can tell nobody ever looks at those reports anyway. |
thanethomson
added a commit
that referenced
this pull request
Aug 16, 2022
Signed-off-by: Thane Thomson <connect@thanethomson.com>
thanethomson
added a commit
that referenced
this pull request
Aug 16, 2022
Signed-off-by: Thane Thomson <connect@thanethomson.com>
thanethomson
added a commit
that referenced
this pull request
Aug 17, 2022
Signed-off-by: Thane Thomson <connect@thanethomson.com>
thanethomson
added a commit
that referenced
this pull request
Aug 18, 2022
* Make reindex-event cmd docs consistent with other commands Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add warning regarding DiscardABCIResponses to BlockResults Go API Signed-off-by: Thane Thomson <connect@thanethomson.com> * Update OpenAPI spec to reflect discard_abci_responses change Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add release highlights to CHANGELOG_PENDING Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add pending changelog entry for #9033 Signed-off-by: Thane Thomson <connect@thanethomson.com> * Format pending changelog entries consistently Signed-off-by: Thane Thomson <connect@thanethomson.com> * Correct and simplify comment wording Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove changelog entry regarding storage section Signed-off-by: Thane Thomson <connect@thanethomson.com> Signed-off-by: Thane Thomson <connect@thanethomson.com>
cmwaters
pushed a commit
to cmwaters/tendermint
that referenced
this pull request
Dec 12, 2022
…ort tendermint#9030) (tendermint#9033) (manual cherry-pick of commit 22ed610)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a manual backport of #9030.