[GitHub] [hbase] imbajin commented on a change in pull request #1940: HBASE-24601: Change default Hfile storage policy from HOT to NONE for HDFS

[GitBox <gi...@apache.org>]

imbajin commented on a change in pull request #1940:
URL: https://github.com/apache/hbase/pull/1940#discussion_r443187352



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -139,8 +139,8 @@
       "hbase.server.compactchecker.interval.multiplier";
   public static final String BLOCKING_STOREFILES_KEY = "hbase.hstore.blockingStoreFiles";
   public static final String BLOCK_STORAGE_POLICY_KEY = "hbase.hstore.block.storage.policy";
-  // keep in accordance with HDFS default storage policy
-  public static final String DEFAULT_BLOCK_STORAGE_POLICY = "HOT";
+  // "NONE" is not a valid storage policy and means we defer the policy to HDFS
+  public static final String DEFAULT_BLOCK_STORAGE_POLICY = "NONE";

Review comment:
       1. Using the `HOT` or `NONE` strategy **currently** won't throw an unexpected exception because [HBASE-20691](https://issues.apache.org/jira/browse/HBASE-20691) has already covered the different situation. And they invoke the same method, that's why we can use `NONE` directly.
   2. It makes sense to allow Hbase users to set the storage strategy corresponding to the Hfile/WAL directory, because most users may not be familiar with how to operate the **heterogeneous** commands of HDFS
   3. However, if the default value stored in Hfile is still `HOT`, there will be potential semantic **conflicts**. HDFS managers and Hbase users may not know this default setting and will not modify it, but it actually changes HDFS Storage strategy. 
   This is a omission point of [HBASE-20691](https://issues.apache.org/jira/browse/HBASE-20691), It only modified the default policy of **WAL**, but forgot to modify **Hfile**.
   
   TBH, it's' a legacy issue to decide if we should avoid/ban `setStoragePolicy()` in Hbase, seems that keeping the status quo has the least impact (ban the whole reflect API for HDFS should be done in a separate patch)




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org