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]

Reply via email to