[#2888] Fix bvar worker starvation with recursive bthread mutex#3414
Open
walterzhaoJR wants to merge 1 commit into
Open
[#2888] Fix bvar worker starvation with recursive bthread mutex#3414walterzhaoJR wants to merge 1 commit into
walterzhaoJR wants to merge 1 commit into
Conversation
Replace the recursive pthread mutex protecting bvar's VarMap with a bthread-aware recursive mutex so yielding metric callbacks do not block worker pthreads. Add regression coverage for worker starvation, recursive ownership across bthread migration, pthread use, and high contention. Related issue: apache#2888
This file contains hidden or 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
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.
Replace the recursive pthread mutex protecting bvar's VarMap with a bthread-aware recursive mutex so yielding metric callbacks do not block worker pthreads. Add regression coverage for worker starvation, recursive ownership across bthread migration, pthread use, and high contention.
Related issue: #2888
What problem does this PR solve?
Issue Number: resolves #2888
Problem Summary:
bvar::Variableprotects its global VarMap with a recursive pthread mutex. If a metric callback, such as aPassiveStatusgetter, yields while holding this lock, other bthreads may block their worker pthreads while waiting for it. Once all workers are blocked, the lock holder cannot resume, causing worker starvation or deadlock.A pthread recursive mutex also identifies its owner by OS pthread, which does not correctly represent a bthread that may migrate between workers.
What is changed and the side effects?
Changed:
bthread_recursive_mutex_tand its C++ wrapper,bthread::RecursiveMutex.bthread_mutex_tfor contention so waiting bthreads are suspended instead of blocking worker pthreads.bthread::RecursiveMutex.test_bvartarget independent of the full bthread runtime.Side effects:
Performance effects:
Breaking backward compatibility:
Test results
bthread_mutex_unittest: 3 recursive mutex tests passed.brpc_bvar_mutex_unittest: 3 bvar regression tests passed.test_bvar: 64/64 tests passed.Check List: