WIP adds new highly concurrent tablet location cache #5311
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
This is a draft change that contains the basic incomplete structure of a
new highly concurrent tablet location cache. For 2.1 this new
implementation could be added along side the existing battle tested
implementation with an option to use the new one. The current structure
with an abstract class for TabletLocator would make it easy to switch the
implementation based on a client config property or java property.
The current tablet location cache has read/write locks that accomplish
two goals.
1. Protect the integrity of an in memory tree map from concurrent
updates.
2. Avoids N threads doing N metadata lookups on a cache miss.
The write lock in the code will usually result in a single metadata
lookup in the case where many threads show up and want info from the
same area of the metadata table.
This changes replaces the tree map with a concurrent map, removing the
need to have locks protecting the data structure. Then it proposes a
mechanism in `ConcurrentTabletLocator.requestLookup()` to attempt to
avoid many concurrent metadata lookups for the same information.
If this implementation were fleshed out, it seems like it may end up
being much simpler than the existing cache. The existing code has a lot
of complexity related to first trying with a read lock and then
switching to a write lock on cache miss, these changes do not have any of
that complexity.
|
Was able to run |
|
Tried running all ITs w/ these changes and that passed. |
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 draft change that contains the basic incomplete structure of a new highly concurrent tablet location cache. For 2.1 this new implementation could be added along side the existing battle tested implementation with an option to use the new one. The current structure with an abstract class for TabletLocator would make it easy to switch the implementation based on a client config property or java property.
The current tablet location cache has read/write locks that accomplish two goals.
This changes replaces the tree map with a concurrent map, removing the need to have locks protecting the data structure. Then it proposes a mechanism in
ConcurrentTabletLocator.requestLookup()to attempt to avoid many concurrent metadata lookups for the same information.If this implementation were fleshed out, it seems like it may end up being much simpler than the existing cache. The existing code has a lot of complexity related to first trying with a read lock and then switching to a write lock on cache miss, these changes do not have any of that complexity.