junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2215282195
##########
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:
When the `hbase.regionserver.free.heap.min.memory.size` is set to an
exceedingly large value, the message becomes a little strange.
```java
conf.setFloat(MemorySizeUtil.MEMSTORE_SIZE_KEY, 0.5f);
conf.setFloat(HConstants.HFILE_BLOCK_CACHE_SIZE_KEY, 0.6f);
conf.set(MemorySizeUtil.HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY,
"100g");
MemorySizeUtil.validateRegionServerHeapMemoryAllocation(conf);
```
```
RegionServer heap memory allocation is invalid: memStore=50%,
blockCache=60%, requiredFreeHeap=4654%. Combined usage 110% exceeds allowed
maximum -4554%.
```
Maybe it's better to simplify the message to something like `Sum exceeds
100%`.
##########
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:
When the `hbase.regionserver.free.heap.min.memory.size` is set to an
exceedingly large value, the message becomes a little strange.
```java
conf.setFloat(MemorySizeUtil.MEMSTORE_SIZE_KEY, 0.5f);
conf.setFloat(HConstants.HFILE_BLOCK_CACHE_SIZE_KEY, 0.6f);
conf.set(MemorySizeUtil.HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY,
"100g");
MemorySizeUtil.validateRegionServerHeapMemoryAllocation(conf);
```
```
RegionServer heap memory allocation is invalid: memStore=50%,
blockCache=60%, requiredFreeHeap=4654%.
Combined usage 110% exceeds allowed maximum -4554%.
```
Maybe it's better to simplify the message to something like `Sum exceeds
100%`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
