wchevreuil commented on code in PR #8128:
URL: https://github.com/apache/hbase/pull/8128#discussion_r3153631186
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketProtoUtils.java:
##########
@@ -100,9 +100,16 @@ private static void
addEntryToBuilder(Map.Entry<BlockCacheKey, BucketEntry> entr
}
private static BucketCacheProtos.BlockCacheKey toPB(BlockCacheKey key) {
- return
BucketCacheProtos.BlockCacheKey.newBuilder().setHfilename(key.getHfileName())
- .setOffset(key.getOffset()).setPrimaryReplicaBlock(key.isPrimary())
- .setBlockType(toPB(key.getBlockType())).build();
+ BucketCacheProtos.BlockCacheKey.Builder builder =
BucketCacheProtos.BlockCacheKey.newBuilder()
+ .setHfilename(key.getHfileName()).setOffset(key.getOffset())
+
.setPrimaryReplicaBlock(key.isPrimary()).setBlockType(toPB(key.getBlockType()));
+ if (key.getCfName() != null) {
+ builder.setFamilyName(key.getCfName());
+ }
+ if (key.getRegionName() != null) {
+ builder.setRegionName(key.getRegionName());
+ }
Review Comment:
Yes. This is required not only by the new "coldDataRatio" metric we are
adding, but also the existing "regionCachedRatio" that is critical for the
CacheAwareLoadBalancer. Without this change here, we cannot calculate these
metrics when recovering the persistent cache. IMO, it's a bug in the current
CacheAwareLoadBalancer implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]