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