HBASE-28467: Addressed review comments.

Change-Id: Ife4f3a50395877027df717191befc2962deb692d

Author: jhungund

hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java

@@ -275,6 +275,18 @@ public boolean shouldCacheBlockOnRead(BlockCategory category) {
       || (prefetchOnOpen && (category != BlockCategory.META && category != BlockCategory.UNKNOWN));
   }
 
+  public boolean shouldCacheFileBlock(HFileInfo hFileInfo, Configuration conf) {
+    Optional<Boolean> cacheFileBlock = Optional.of(true);
+    // Additionally perform the time-based priority checks to see
+    // whether, or not to cache the block.
+    if (getBlockCache().isPresent()) {
+
+      cacheFileBlock = getBlockCache().get().shouldCacheFile(hFileInfo, conf);
+      LOG.info("BlockCache Present, cacheFileBlock: {}", cacheFileBlock.get());
+    }
+    return cacheFileBlock.get();
+  }
+
   /** Returns true if blocks in this file should be flagged as in-memory */
   public boolean isInMemory() {
     return this.inMemory;

hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java

@@ -1357,13 +1357,7 @@ public HFileBlock readBlock(long dataBlockOffset, long onDiskBlockSize, final bo
         BlockType.BlockCategory category = hfileBlock.getBlockType().getCategory();
         final boolean cacheCompressed = cacheConf.shouldCacheCompressed(category);
         final boolean cacheOnRead = cacheConf.shouldCacheBlockOnRead(category);
-        Optional<Boolean> cacheFileBlock = Optional.of(true);
-        // Additionally perform the time-based priority checks to see
-        // whether, or not to cache the block.
-        if (cacheConf.getBlockCache().isPresent()) {
-          cacheFileBlock = cacheConf.getBlockCache().get().shouldCacheFile(getHFileInfo(), conf);
-        }
-        final boolean shouldCacheFileBlock = cacheFileBlock.get();
+        final boolean shouldCacheFileBlock = cacheConf.shouldCacheFileBlock(getHFileInfo(), conf);
 
         // Don't need the unpacked block back and we're storing the block in the cache compressed
         if (cacheOnly && cacheCompressed && cacheOnRead && shouldCacheFileBlock) {

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDataTieringManager.java

@@ -474,6 +474,31 @@ public void testFeatureKeyDisabled() throws Exception {
     }
   }
 
+  @Test
+  public void testCacheConfigShouldCacheFile() throws Exception {
+    // Evict the files from cache.
+    for (HStoreFile file : hStoreFiles) {
+      file.closeStoreFile(true);
+    }
+    // Verify that the API shouldCacheFileBlock returns the result correctly.
+    // hStoreFiles[0], hStoreFiles[1], hStoreFiles[2] are hot files.
+    // hStoreFiles[3] is a cold file.
+    try {
+      assertTrue(cacheConf.shouldCacheFileBlock(hStoreFiles.get(0).getFileInfo().getHFileInfo(),
+        hStoreFiles.get(0).getFileInfo().getConf()));
+      assertTrue(cacheConf.shouldCacheFileBlock(hStoreFiles.get(1).getFileInfo().getHFileInfo(),
+        hStoreFiles.get(1).getFileInfo().getConf()));
+      assertTrue(cacheConf.shouldCacheFileBlock(hStoreFiles.get(2).getFileInfo().getHFileInfo(),
+        hStoreFiles.get(2).getFileInfo().getConf()));
+      assertFalse(cacheConf.shouldCacheFileBlock(hStoreFiles.get(3).getFileInfo().getHFileInfo(),
+        hStoreFiles.get(3).getFileInfo().getConf()));
+    } finally {
+      for (HStoreFile file : hStoreFiles) {
+        file.initReader();
+      }
+    }
+  }
+
   @Test
   public void testCacheOnReadColdFile() throws Exception {
     // hStoreFiles[3] is a cold file. the blocks should not get loaded after a readBlock call.
@@ -488,7 +513,7 @@ public void testCacheOnReadHotFile() throws Exception {
     HStoreFile hStoreFile = hStoreFiles.get(0);
     BlockCacheKey cacheKey =
       new BlockCacheKey(hStoreFiles.get(0).getPath(), 0, true, BlockType.DATA);
-    testCacheOnRead(hStoreFile, cacheKey, hStoreFile.getFileInfo().getSize(), true);
+    testCacheOnRead(hStoreFile, cacheKey, 23025, true);
   }
 
   private void testCacheOnRead(HStoreFile hStoreFile, BlockCacheKey key, long onDiskBlockSize,