jhungund commented on code in PR #5832:
URL: https://github.com/apache/hbase/pull/5832#discussion_r1569869418


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketProtoUtils.java:
##########
@@ -130,10 +136,30 @@ static Pair<ConcurrentHashMap<BlockCacheKey, 
BucketEntry>, NavigableSet<BlockCac
     ConcurrentHashMap<BlockCacheKey, BucketEntry> result = new 
ConcurrentHashMap<>();
     NavigableSet<BlockCacheKey> resultSet = new 
ConcurrentSkipListSet<>(Comparator
       
.comparing(BlockCacheKey::getHfileName).thenComparingLong(BlockCacheKey::getOffset));
+
+    Map<String, Path> allFilePaths = null;
+    DataTieringManager dataTieringManager;
+    try {
+      dataTieringManager = DataTieringManager.getInstance();
+      allFilePaths = dataTieringManager.getAllFilesList();
+    } catch (IllegalStateException e) {
+      // Data-Tiering manager has not been set up.
+      // Ignore the error and proceed with the normal flow.
+      LOG.warn("Error while getting DataTieringManager instance: {}", 
e.getMessage());
+    }
+
     for (BucketCacheProtos.BackingMapEntry entry : backingMap.getEntryList()) {
       BucketCacheProtos.BlockCacheKey protoKey = entry.getKey();
-      BlockCacheKey key = new BlockCacheKey(protoKey.getHfilename(), 
protoKey.getOffset(),
-        protoKey.getPrimaryReplicaBlock(), fromPb(protoKey.getBlockType()));
+
+      BlockCacheKey key = null;
+      if (allFilePaths != null) {
+        key = new BlockCacheKey(allFilePaths.get(protoKey.getHfilename()), 
protoKey.getOffset(),
+          protoKey.getPrimaryReplicaBlock(), fromPb(protoKey.getBlockType()));
+      } else {
+        key = new BlockCacheKey(new Path(protoKey.getHfilename()), 
protoKey.getOffset(),

Review Comment:
   One way I can think of avoiding incorrect path to be set is by adding the 
following check in the constructor of BlockCacheKey:
   `    if (hfilePath.getParent() != null) {
         this.filePath = hfilePath;
       }
   `
   But with this, path will remain null and during evictions (freespace()), 
either we should skip the data tiering logic for such blocks or take an 
expensive route of going over each file of the region server to determine the 
metadata. If the later case happens even for a single key, then, we are better 
off without the path and use the file names itself. Thoughts?



-- 
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