HBASE-28535: Addressed the review comments.

Change-Id: I95ec8691f5d511a8bd452c1492de7ff1222980b6

Author: jhungund

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

@@ -23,7 +23,6 @@
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hbase.io.FSDataInputStreamWrapper;
-import org.apache.hadoop.hbase.regionserver.DataTieringManager;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -41,13 +40,8 @@ public HFilePreadReader(ReaderContext context, HFileInfo fileInfo, CacheConfig c
 
     final MutableBoolean shouldCache = new MutableBoolean(true);
 
-    DataTieringManager dataTieringManager = DataTieringManager.getInstance();
-    if (dataTieringManager != null) {
-      // Initialize HFileInfo object with metadata for caching decisions.
-      // Initialize the metadata only if the data-tiering is enabled.
-      // If not, the metadata will be initialized later.
-      fileInfo.initMetaAndIndex(this);
-    }
+    // Initialize HFileInfo object with metadata for caching decisions
+    fileInfo.initMetaAndIndex(this);
 
     cacheConf.getBlockCache().ifPresent(cache -> {
       Optional<Boolean> result = cache.shouldCacheFile(fileInfo, conf);

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

@@ -2195,18 +2195,11 @@ public Optional<Boolean> blockFitsIntoTheCache(HFileBlock block) {
   public Optional<Boolean> shouldCacheFile(HFileInfo hFileInfo, Configuration conf) {
     String fileName = hFileInfo.getHFileContext().getHFileName();
     DataTieringManager dataTieringManager = DataTieringManager.getInstance();
-    if (dataTieringManager != null) {
-      if (!dataTieringManager.isHotData(hFileInfo, conf)) {
-        LOG.debug("Data tiering is enabled for file: '{}' and it is not hot data", fileName);
-        return Optional.of(false);
-      } else {
-        LOG.debug("Data tiering is enabled for file: '{}' and it is hot data", fileName);
-      }
-    } else {
-      LOG.debug("Data tiering feature is not enabled. "
-        + " The file: '{}' will be loaded if not already loaded", fileName);
+    if (dataTieringManager != null && !dataTieringManager.isHotData(hFileInfo, conf)) {
+      LOG.debug("Data tiering is enabled for file: '{}' and it is not hot data", fileName);
+      return Optional.of(false);
     }
-    // if we don't have the file in fullyCachedFiles, we should cache it.
+    // if we don't have the file in fullyCachedFiles, we should cache it
     return Optional.of(!fullyCachedFiles.containsKey(fileName));
   }
 

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DataTieringManager.java

@@ -45,8 +45,9 @@
 @InterfaceAudience.Private
 public class DataTieringManager {
   private static final Logger LOG = LoggerFactory.getLogger(DataTieringManager.class);
-  public static final String DATA_TIERING_ENABLED_KEY = "hbase.hstore.datatiering.enable";
-  public static final boolean DEFAULT_DATA_TIERING_ENABLED = false; // disabled by default
+  public static final String GLOBAL_DATA_TIERING_ENABLED_KEY =
+    "hbase.regionserver.datatiering.enable";
+  public static final boolean DEFAULT_GLOBAL_DATA_TIERING_ENABLED = false; // disabled by default
   public static final String DATATIERING_KEY = "hbase.hstore.datatiering.type";
   public static final String DATATIERING_HOT_DATA_AGE_KEY =
     "hbase.hstore.datatiering.hot.age.millis";
@@ -60,26 +61,25 @@ private DataTieringManager(Map<String, HRegion> onlineRegions) {
   }
 
   /**
-   * Initializes the DataTieringManager instance with the provided map of online regions.
+   * Initializes the DataTieringManager instance with the provided map of online regions, only if
+   * the configuration "hbase.regionserver.datatiering.enable" is enabled.
    * @param onlineRegions A map containing online regions.
    */
-  public static synchronized void instantiate(Configuration conf,
+  public static synchronized boolean instantiate(Configuration conf,
     Map<String, HRegion> onlineRegions) {
-    if (isDataTieringFeatureEnabled(conf)) {
-      if (instance == null) {
-        instance = new DataTieringManager(onlineRegions);
-        LOG.info("DataTieringManager instantiated successfully.");
-      } else {
-        LOG.warn("DataTieringManager is already instantiated.");
-      }
+    if (isDataTieringFeatureEnabled(conf) && instance == null) {
+      instance = new DataTieringManager(onlineRegions);
+      LOG.info("DataTieringManager instantiated successfully.");
+      return true;
     } else {
-      LOG.info("Data-Tiering feature is not enabled.");
+      LOG.warn("DataTieringManager is already instantiated.");
     }
+    return false;
   }
 
   /**
    * Retrieves the instance of DataTieringManager.
-   * @return The instance of DataTieringManager.
+   * @return The instance of DataTieringManager, if instantiated, null otherwise.
    */
   public static synchronized DataTieringManager getInstance() {
     return instance;
@@ -311,8 +311,13 @@ public Map<String, String> getColdFilesList() {
     return coldFiles;
   }
 
-  public static boolean isDataTieringFeatureEnabled(Configuration conf) {
-    return conf.getBoolean(DataTieringManager.DATA_TIERING_ENABLED_KEY,
-      DataTieringManager.DEFAULT_DATA_TIERING_ENABLED);
+  private static boolean isDataTieringFeatureEnabled(Configuration conf) {
+    return conf.getBoolean(DataTieringManager.GLOBAL_DATA_TIERING_ENABLED_KEY,
+      DataTieringManager.DEFAULT_GLOBAL_DATA_TIERING_ENABLED);
+  }
+
+  // Resets the instance to null. To be used only for testing.
+  public static void resetForTestingOnly() {
+    instance = null;
   }
 }

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

@@ -534,9 +534,10 @@ public HRegionServer(final Configuration conf) throws IOException {
       regionServerAccounting = new RegionServerAccounting(conf);
 
       blockCache = BlockCacheFactory.createBlockCache(conf);
-      if (DataTieringManager.isDataTieringFeatureEnabled(conf)) {
-        DataTieringManager.instantiate(conf, onlineRegions);
-      }
+      // The call below, instantiates the DataTieringManager only when
+      // the configuration "hbase.regionserver.datatiering.enable" is set to true.
+      DataTieringManager.instantiate(conf, onlineRegions);
+
       mobFileCache = new MobFileCache(conf);
 
       rsSnapshotVerifier = new RSSnapshotVerifier(conf);