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


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/util/MemorySizeUtil.java:
##########
@@ -77,34 +82,72 @@ public static MemoryUsage safeGetHeapMemoryUsage() {
   }
 
   /**
-   * Checks whether we have enough heap memory left out after portion for 
Memstore and Block cache.
-   * We need atleast 20% of heap left out for other RS functions.
+   * Validates that heap allocations for memStore and block cache do not 
exceed the allowed limit,
+   * ensuring enough free heap remains for other RegionServer tasks.
+   * @param conf the configuration to validate
+   * @throws RuntimeException if the combined allocation exceeds the threshold
    */
-  public static void checkForClusterFreeHeapMemoryLimit(Configuration conf) {
+  public static void validateRegionServerHeapMemoryAllocation(Configuration 
conf) {
     if (conf.get(MEMSTORE_SIZE_OLD_KEY) != null) {
       LOG.warn(MEMSTORE_SIZE_OLD_KEY + " is deprecated by " + 
MEMSTORE_SIZE_KEY);
     }
-    float globalMemstoreSize = getGlobalMemStoreHeapPercent(conf, false);
-    int gml = (int) (globalMemstoreSize * CONVERT_TO_PERCENTAGE);
-    float blockCacheUpperLimit = getBlockCacheHeapPercent(conf);
-    int bcul = (int) (blockCacheUpperLimit * CONVERT_TO_PERCENTAGE);
-    if (
-      CONVERT_TO_PERCENTAGE - (gml + bcul)
-          < (int) (CONVERT_TO_PERCENTAGE * 
HConstants.HBASE_CLUSTER_MINIMUM_MEMORY_THRESHOLD)
-    ) {
-      throw new RuntimeException("Current heap configuration for MemStore and 
BlockCache exceeds "
-        + "the threshold required for successful cluster operation. "
-        + "The combined value cannot exceed 0.8. Please check " + "the 
settings for "
-        + MEMSTORE_SIZE_KEY + " and either " + 
HConstants.HFILE_BLOCK_CACHE_MEMORY_SIZE_KEY + " or "
-        + HConstants.HFILE_BLOCK_CACHE_SIZE_KEY + " in your configuration. " + 
MEMSTORE_SIZE_KEY
-        + "=" + globalMemstoreSize + ", " + 
HConstants.HFILE_BLOCK_CACHE_MEMORY_SIZE_KEY + "="
-        + conf.get(HConstants.HFILE_BLOCK_CACHE_MEMORY_SIZE_KEY) + ", "
-        + HConstants.HFILE_BLOCK_CACHE_SIZE_KEY + "="
-        + conf.get(HConstants.HFILE_BLOCK_CACHE_SIZE_KEY) + ". (Note: If both "
-        + HConstants.HFILE_BLOCK_CACHE_MEMORY_SIZE_KEY + " and "
-        + HConstants.HFILE_BLOCK_CACHE_SIZE_KEY + " are set, " + "the system 
will use "
-        + HConstants.HFILE_BLOCK_CACHE_MEMORY_SIZE_KEY + ")");
+    float memStoreFraction = getGlobalMemStoreHeapPercent(conf, false);
+    float blockCacheFraction = getBlockCacheHeapPercent(conf);
+    float minFreeHeapFraction = getRegionServerMinFreeHeapFraction(conf);
+
+    int memStorePercent = (int) (memStoreFraction * 100);
+    int blockCachePercent = (int) (blockCacheFraction * 100);
+    int minFreeHeapPercent = (int) (minFreeHeapFraction * 100);
+    int usedPercent = memStorePercent + blockCachePercent;
+    int maxAllowedUsed = 100 - minFreeHeapPercent;
+
+    if (usedPercent > maxAllowedUsed) {
+      throw new RuntimeException(String.format(
+        "RegionServer heap memory allocation is invalid: "
+          + "memStore=%d%%, blockCache=%d%%, requiredFreeHeap=%d%%. "
+          + "Combined usage %d%% exceeds allowed maximum %d%%. "

Review Comment:
   Sorry for late reply and thank you for catching this message.
   I simplified the error message.
   ```
   java.lang.RuntimeException: RegionServer heap memory allocation is invalid: 
total memory usage exceeds 100% (memStore + blockCache + requiredFreeHeap). 
Check the following configuration values:
     - hbase.regionserver.global.memstore.size = 0.50
     - hfile.block.cache.memory.size = null
     - hfile.block.cache.size = 0.6
     - hbase.regionserver.free.heap.min.memory.size = 100g
   ```
   
   Please check e583c2b9e414a76ad16bcee41c9c822d5f9eaada when you have time 🙇  
Thank you!



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/util/MemorySizeUtil.java:
##########
@@ -77,34 +82,72 @@ public static MemoryUsage safeGetHeapMemoryUsage() {
   }
 
   /**
-   * Checks whether we have enough heap memory left out after portion for 
Memstore and Block cache.
-   * We need atleast 20% of heap left out for other RS functions.
+   * Validates that heap allocations for memStore and block cache do not 
exceed the allowed limit,

Review Comment:
   Thank you. Fixed in e583c2b9e414a76ad16bcee41c9c822d5f9eaada



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