junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2134536158


##########
hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java:
##########
@@ -1040,10 +1040,21 @@ public enum OperationStatusCode {
   public static final String HFILE_PREAD_ALL_BYTES_ENABLED_KEY = 
"hfile.pread.all.bytes.enabled";
   public static final boolean HFILE_PREAD_ALL_BYTES_ENABLED_DEFAULT = false;
 
-  /*
-   * Minimum percentage of free heap necessary for a successful cluster 
startup.
+  /**
+   * Configuration key for the minimum fraction of heap memory that must 
remain free for a
+   * RegionServer to start
+   */
+  public static final String HBASE_REGION_SERVER_FREE_HEAP_MIN_SIZE_KEY =
+    "hbase.regionserver.free.heap.min.size";
+
+  public static final float HBASE_REGION_SERVER_FREE_HEAP_MIN_SIZE_DEFAULT = 
0.1f;
+
+  /**
+   * Configuration key for the absolute amount of heap memory that must remain 
free for a
+   * RegionServer to start
    */
-  public static final float HBASE_CLUSTER_MINIMUM_MEMORY_THRESHOLD = 0.2f;
+  public static final String HBASE_REGION_SERVER_FREE_HEAP_MEMORY_MIN_SIZE_KEY 
=
+    "hbase.regionserver.free.heap.memory.min.size";

Review Comment:
   No problem. Thanks for clarification.
   
   > * `hfile.block.cache.size`
   > * `hfile.block.cache.memory.size`
   
   Maybe it's just me, but when I see this pair, I get the impression that 
`size` expects a fraction, whereas `memory.size` expects an actual value. And 
that made me wonder whether the absolute version of 
`hbase.regionserver.free.heap.min.size` should be named 
`hbase.regionserver.free.heap.min.memory.size` to follow the same `size` vs 
`memory.size` convention. But maybe there was no such convention in the first 
place?
   
   > I believe supporting both styles (fraction and absolute size) makes sense 
for the near future. Would love to hear your thoughts!
   
   I'm not the one to make the call here, but personally, I'm not a fan of 
adding options just for completeness' sake, because increasing number of 
options tends to increase cognitive burden of the users, and as the number of 
possible combinations of options increases, it gets harder to reason about 
their combined effects. So I wanted to make sure if we definitely needed both. 
However, we have to admit that the number of configuration parameters of HBase 
is already out of control, so my point here is probably moot, because not 
adding a single parameter wouldn't make much difference to the project.
   
   Anyway, the reason you and the initial reporter of the issue wanted this to 
be configurable was to reserve as much memory as possible for memstores and 
block cache when using large heap sizes. And I imagine you are more likely to 
use the actual size version than the fraction one, to better deal with the 
systems with different heap sizes and keep the safety net.
   
   For example, let's say you have these:
   
   * memstore = 0.6
   * blockcache = 0.3
   * minimum unreserved size = 10GB
   
   You can use these values across systems with different heap sizes and still 
be sure that at least 10GB of the heap is unreserved. However, if you use the 
fraction version instead and set it to 0.9, there is no guarantee of the actual 
unreserved size, and that is not very different from just removing the check.
   
   So could you explain how you are going to use these parameters on your 
clusters?



-- 
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: issues-unsubscr...@hbase.apache.org

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

Reply via email to