HBASE-26083 In branch-2 & branch-1 L1 miss metric is always 0 when using CombinedBlockCache #3474
Conversation
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
…ing CombinedBlockCache
anoopsjohn
reviewed
Jul 12, 2021
| @@ -79,7 +79,7 @@ public Cacheable getBlock(BlockCacheKey cacheKey, boolean caching, | |||
| boolean repeat, boolean updateCacheMetrics) { | |||
| // TODO: is there a hole here, or just awkwardness since in the lruCache getBlock | |||
| // we end up calling l2Cache.getBlock. | |||
| return lruCache.containsBlock(cacheKey)? | |||
| return cacheKey.getBlockType().getCategory() == BlockCategory.DATA? | |||
There was a problem hiding this comment.
In CombinedBC, its the NON data block (index/bloom) which goes to onheap LRU cache. Data blocks go to bucket cache (l2Cache as per variable here)
There was a problem hiding this comment.
You are right. Have fixed in the last commit.
There was a problem hiding this comment.
As in branch-1 it is also possible that data blocks are cached in L1, so that it is not a good way to check L1 or L2 according to the block type. I have update the logic in the last commit.
Reidddddd
reviewed
Jul 14, 2021
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java
Outdated
Show resolved
Hide resolved
Reidddddd
requested changes
Jul 14, 2021
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java
Outdated
Show resolved
Hide resolved
|
@anoopsjohn any comment about this? |
anoopsjohn
reviewed
Jul 15, 2021
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java
Outdated
Show resolved
Hide resolved
|
🎊 +1 overall
This message was automatically generated. |
Reidddddd
approved these changes
Jul 16, 2021
anoopsjohn
approved these changes
Jul 16, 2021
|
Approved. I had approved another branch-1 PR also yesterday. Thought that was the one :-). NP all looks good. Pls merge. Nice work |
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.
https://issues.apache.org/jira/browse/HBASE-26083