feat: Minimize contention in Bigtable Client initialization. #2923
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
Channel pools for Bigtable clients are lazy initialized, the first thread that needs them creates them. To avoid deadlocks and unbound priority inversions this thread releases the locks while creating the pools, and relocks before saving the newly created pool. In some cases two (or more) threads may create the pool, in this case the pool created by the second thread is discarded. We were discarding this work with the lock held, which resulted in deadlocks for some users inside Google (this should not affect external users).
coryan
added
api: bigtable
googlebot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #2923 +/- ##
==========================================
- Coverage 90.75% 90.68% -0.08%
==========================================
Files 297 297
Lines 19943 20007 +64
==========================================
+ Hits 18100 18144 +44
- Misses 1843 1863 +20
Continue to review full report at Codecov.
|
devbww
reviewed
Jul 31, 2019
| } else { | ||
| // Some other thread created the pool and saved it in `stubs_`. The work | ||
| // in this thread was superfluous. We release the lock while clearing the | ||
| // channels to minimize contention, this seems to workaround other bugs |
There was a problem hiding this comment.
s/contention, this/contention. This/
Although, I'm not sure the "seems to workaround" part is useful. I'm not even sure what "the Google implementation of std::mutex" is referring to.
There was a problem hiding this comment.
I'm still not sure what "bugs inside Google" means, but you can fill me in later.
devbww
approved these changes
Jul 31, 2019
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.
Channel pools for Bigtable clients are lazy initialized, the first
thread that needs them creates them. To avoid deadlocks and unbound
priority inversions this thread releases the locks while creating the
pools, and relocks before saving the newly created pool.
In some cases two (or more) threads may create the pool, in this case
the pool created by the second thread is discarded. We were discarding
this work with the lock held, which resulted in deadlocks for some
users inside Google (this should not affect external users).
This change is