You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "jinhyukify (via GitHub)" <gi...@apache.org> on 2025/06/07 08:40:21 UTC
[PR] HBASE-15625 Make minimum free heap memory percentage configurable and lower default to 0.1 [hbase]
jinhyukify opened a new pull request, #7076:
URL: https://github.com/apache/hbase/pull/7076
**Jira** https://issues.apache.org/jira/browse/HBASE-15625
---
In modern deployments with large heap sizes, there are cases where allocating a very large memstore aggressively can be beneficial for performance. In such scenarios, keeping 20% of the heap free is unnecessarily conservative. To better accommodate these cases, I reduced the default minimum free heap threshold to 10% and made it configurable.
- Two new configuration options added to make the minimum free heap memory requirement configurable:
- `hbase.regionserver.free.heap.min.size`
- `hbase.regionserver.free.heap.memory.min.size`
- Default minimum free heap threshold lowered from 20% to 10%.
- Improved error messages to reflect that the minimum free heap threshold is now configurable.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2146746804
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -241,6 +238,15 @@ public float getHeapOccupancyPercent() {
: this.heapOccupancyPercent;
}
+ private boolean isHeapMemoryUsageExceedingLimit(float memStoreFraction,
+ float blockCacheFraction) {
+ int memStorePercent = (int) (memStoreFraction * CONVERT_TO_PERCENTAGE);
+ int blockCachePercent = (int) (blockCacheFraction * CONVERT_TO_PERCENTAGE);
+ int minFreeHeapPercent = (int) (this.minFreeHeapFraction * CONVERT_TO_PERCENTAGE);
+
+ return memStorePercent + blockCachePercent + minFreeHeapPercent > CONVERT_TO_PERCENTAGE;
Review Comment:
Ah, another case of the [spacebar heating problem](https://xkcd.com/1172/). By not converting to the percent values, we're effectively fixing a bug that allowed slight misconfiguration due to truncation, but then we might break existing setups that relied on that bug.
Though I don't think anyone will be affected by the change, the benefit of simplifying the code and fixing that benign bug does not outweigh the risk, however small the possibility is, so you can leave it as is.
But since you're introducing this method, people might wonder why you did it this way in the future, so I suggest leaving a comment explaining the rationale behind the decision.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2994443249
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 33s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 43s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 37s | | master passed |
| +1 :green_heart: | compile | 1m 21s | | master passed |
| +1 :green_heart: | javadoc | 0m 43s | | master passed |
| +1 :green_heart: | shadedjars | 6m 6s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 5s | | the patch passed |
| +1 :green_heart: | compile | 1m 16s | | the patch passed |
| +1 :green_heart: | javac | 1m 16s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 43s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 2s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 15s | | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 212m 57s | | hbase-server in the patch passed. |
| | | 244m 33s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/12/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 381a7fd320f4 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 87bfbe7f967d50da45421012a763b042c5e16288 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/12/testReport/ |
| Max. process+thread count | 5496 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/12/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2153458092
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -184,26 +184,23 @@ private boolean doInit(Configuration conf) {
tuningEnabled = false;
}
- int gml = (int) (globalMemStorePercentMaxRange * CONVERT_TO_PERCENTAGE);
- int bcul = (int) ((blockCachePercentMinRange) * CONVERT_TO_PERCENTAGE);
- if (CONVERT_TO_PERCENTAGE - (gml + bcul) < CLUSTER_MINIMUM_MEMORY_THRESHOLD) {
+ if (isHeapMemoryUsageExceedingLimit(globalMemStorePercentMaxRange, blockCachePercentMinRange)) {
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_MAX_RANGE_KEY + " and " + BLOCK_CACHE_SIZE_MIN_RANGE_KEY
- + " in your configuration. " + MEMSTORE_SIZE_MAX_RANGE_KEY + " is "
- + globalMemStorePercentMaxRange + " and " + BLOCK_CACHE_SIZE_MIN_RANGE_KEY + " is "
+ + "the allowed heap usage. At least " + minFreeHeapFraction
+ + " of the heap must remain free to ensure stable RegionServer operation. "
+ + "Please check the settings for " + MEMSTORE_SIZE_MAX_RANGE_KEY + " and "
+ + BLOCK_CACHE_SIZE_MIN_RANGE_KEY + " in your configuration. " + MEMSTORE_SIZE_MAX_RANGE_KEY
+ + " is " + globalMemStorePercentMaxRange + " and " + BLOCK_CACHE_SIZE_MIN_RANGE_KEY + " is "
+ blockCachePercentMinRange);
}
- gml = (int) (globalMemStorePercentMinRange * CONVERT_TO_PERCENTAGE);
- bcul = (int) ((blockCachePercentMaxRange) * CONVERT_TO_PERCENTAGE);
- if (CONVERT_TO_PERCENTAGE - (gml + bcul) < CLUSTER_MINIMUM_MEMORY_THRESHOLD) {
+
+ if (isHeapMemoryUsageExceedingLimit(globalMemStorePercentMinRange, blockCachePercentMaxRange)) {
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_MIN_RANGE_KEY + " and " + BLOCK_CACHE_SIZE_MAX_RANGE_KEY
- + " in your configuration. " + MEMSTORE_SIZE_MIN_RANGE_KEY + " is "
- + globalMemStorePercentMinRange + " and " + BLOCK_CACHE_SIZE_MAX_RANGE_KEY + " is "
+ + "the allowed heap usage. At least " + minFreeHeapFraction
+ + " of the heap must remain free to ensure stable RegionServer operation. "
+ + "Please check the settings for " + MEMSTORE_SIZE_MIN_RANGE_KEY + " and "
+ + BLOCK_CACHE_SIZE_MAX_RANGE_KEY + " in your configuration. " + MEMSTORE_SIZE_MIN_RANGE_KEY
+ + " is " + globalMemStorePercentMinRange + " and " + BLOCK_CACHE_SIZE_MAX_RANGE_KEY + " is "
Review Comment:
Nice, much better.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2145217122
##########
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%%. "
+ + "Check the following configuration values:%n" + " - %s = %.2f%n" + " - %s = %s%n"
+ + " - %s = %s%n" + " - %s = %s",
+ memStorePercent, blockCachePercent, minFreeHeapPercent, usedPercent, maxAllowedUsed,
+ MEMSTORE_SIZE_KEY, memStoreFraction, 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),
+ MemorySizeUtil.HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY,
+ conf.get(MemorySizeUtil.HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY)));
+ }
+ }
+
+ /**
+ * Retrieve an explicit minimum required free heap size in bytes in the configuration.
+ * @param conf used to read configs
+ * @return the minimum required free heap size in bytes, or a negative value if not configured.
+ * @throws IllegalArgumentException if HBASE_REGION_SERVER_FREE_HEAP_MEMORY_MIN_SIZE_KEY format is
+ * invalid
+ */
+ private static long getRegionServerMinFreeHeapInBytes(Configuration conf) {
+ final String key = MemorySizeUtil.HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY;
Review Comment:
```suggestion
final String key = HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY;
```
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2965453374
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 33s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 59s | | master passed |
| +1 :green_heart: | compile | 2m 2s | | master passed |
| +1 :green_heart: | javadoc | 1m 0s | | master passed |
| +1 :green_heart: | shadedjars | 7m 31s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 47s | | the patch passed |
| +1 :green_heart: | compile | 1m 26s | | the patch passed |
| +1 :green_heart: | javac | 1m 26s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 52s | | the patch passed |
| +1 :green_heart: | shadedjars | 7m 18s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 53s | | hbase-common in the patch passed. |
| -1 :x: | unit | 246m 7s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/6/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 282m 46s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/6/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux e9a9123eca99 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / f029d1f7707d09b8e662a6061d76d8e1116ec1ff |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/6/testReport/ |
| Max. process+thread count | 5953 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/6/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable and lower default to 0.1 [hbase]
Posted by "jinhyukify (via GitHub)" <gi...@apache.org>.
jinhyukify commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2133765764
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/util/MemorySizeUtil.java:
##########
@@ -77,36 +77,73 @@ 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%%. "
+ + "Check the following configuration values:%n" + " - %s = %.2f%n" + " - %s = %s%n"
+ + " - %s = %s%n" + " - %s = %s%n" + " - %s = %s",
+ memStorePercent, blockCachePercent, minFreeHeapPercent, usedPercent, maxAllowedUsed,
+ MEMSTORE_SIZE_KEY, memStoreFraction, 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),
+ HConstants.HBASE_REGION_SERVER_FREE_HEAP_MEMORY_MIN_SIZE_KEY,
+ conf.get(HConstants.HBASE_REGION_SERVER_FREE_HEAP_MEMORY_MIN_SIZE_KEY),
+ HConstants.HBASE_REGION_SERVER_FREE_HEAP_MIN_SIZE_KEY,
+ conf.get(HConstants.HBASE_REGION_SERVER_FREE_HEAP_MIN_SIZE_KEY)));
Review Comment:
```
java.lang.RuntimeException: RegionServer heap memory allocation is invalid: memStore=40%, blockCache=40%, requiredFreeHeap=30%. Combined usage 80% exceeds allowed maximum 70%. Check the following configuration values:
- hbase.regionserver.global.memstore.size = 0.40
- hfile.block.cache.memory.size = null
- hfile.block.cache.size = 0.4
- hbase.regionserver.free.heap.memory.min.size = null
- hbase.regionserver.free.heap.min.size = 0.3
```
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2188969665
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/util/TestMemorySizeUtil.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.io.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.testclassification.MiscTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({ MiscTests.class, SmallTests.class })
+public class TestMemorySizeUtil {
+
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+ HBaseClassTestRule.forClass(TestMemorySizeUtil.class);
+
+ private Configuration conf;
+
+ @Before
+ public void setup() {
+ conf = new Configuration();
+ }
+
+ @Test
+ public void testValidateRegionServerHeapMemoryAllocation() {
+ // when memstore size + block cache size + default free heap min size == 1.0
+ conf.setFloat(MemorySizeUtil.MEMSTORE_SIZE_KEY, 0.4f);
+ conf.setFloat(HConstants.HFILE_BLOCK_CACHE_SIZE_KEY, 0.4f);
+ assertEquals(HConstants.HBASE_CLUSTER_MINIMUM_MEMORY_THRESHOLD, 0.2f, 0.0f);
+ MemorySizeUtil.validateRegionServerHeapMemoryAllocation(conf);
+
+ // when memstore size + block cache size + default free heap min size > 1.0
+ conf.setFloat(MemorySizeUtil.MEMSTORE_SIZE_KEY, 0.5f);
+ assertThrows(RuntimeException.class,
+ () -> MemorySizeUtil.validateRegionServerHeapMemoryAllocation(conf));
+ }
Review Comment:
It look like we can add a step that shows that setting `HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY` to zero can make the validation pass.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/util/TestMemorySizeUtil.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.io.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.testclassification.MiscTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({ MiscTests.class, SmallTests.class })
+public class TestMemorySizeUtil {
+
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+ HBaseClassTestRule.forClass(TestMemorySizeUtil.class);
+
+ private Configuration conf;
+
+ @Before
+ public void setup() {
+ conf = new Configuration();
+ }
+
+ @Test
+ public void testValidateRegionServerHeapMemoryAllocation() {
+ // when memstore size + block cache size + default free heap min size == 1.0
+ conf.setFloat(MemorySizeUtil.MEMSTORE_SIZE_KEY, 0.4f);
+ conf.setFloat(HConstants.HFILE_BLOCK_CACHE_SIZE_KEY, 0.4f);
+ assertEquals(HConstants.HBASE_CLUSTER_MINIMUM_MEMORY_THRESHOLD, 0.2f, 0.0f);
+ MemorySizeUtil.validateRegionServerHeapMemoryAllocation(conf);
+
+ // when memstore size + block cache size + default free heap min size > 1.0
+ conf.setFloat(MemorySizeUtil.MEMSTORE_SIZE_KEY, 0.5f);
+ assertThrows(RuntimeException.class,
+ () -> MemorySizeUtil.validateRegionServerHeapMemoryAllocation(conf));
+ }
Review Comment:
It looks like we can add a step that shows that setting `HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY` to zero can make the validation pass.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2145251529
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -241,6 +238,15 @@ public float getHeapOccupancyPercent() {
: this.heapOccupancyPercent;
}
+ private boolean isHeapMemoryUsageExceedingLimit(float memStoreFraction,
+ float blockCacheFraction) {
+ int memStorePercent = (int) (memStoreFraction * CONVERT_TO_PERCENTAGE);
+ int blockCachePercent = (int) (blockCacheFraction * CONVERT_TO_PERCENTAGE);
+ int minFreeHeapPercent = (int) (this.minFreeHeapFraction * CONVERT_TO_PERCENTAGE);
+
+ return memStorePercent + blockCachePercent + minFreeHeapPercent > CONVERT_TO_PERCENTAGE;
Review Comment:
How about we just compare the sum of the float values against 1?
```suggestion
return memStoreFraction + blockCacheFraction + minFreeHeapFraction > 1;
```
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable and lower default to 0.1 [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2133973186
##########
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;
Review Comment:
I'd recommend keeping the default unchanged unless there's a strong reason. Some servers may still have relatively limited memory (e.g. VM), so it's probably safer to keep the original value.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-3079714063
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 29s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 36s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 30s | | master passed |
| +1 :green_heart: | compile | 1m 16s | | master passed |
| +1 :green_heart: | javadoc | 0m 44s | | master passed |
| +1 :green_heart: | shadedjars | 6m 3s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 5s | | the patch passed |
| +1 :green_heart: | compile | 1m 20s | | the patch passed |
| +1 :green_heart: | javac | 1m 20s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 44s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 2s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 39s | | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 214m 1s | | hbase-server in the patch passed. |
| | | 245m 7s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/13/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux e623dc1084c0 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3ef55a993e5ce3a71425451e4ed0753519df28e7 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/13/testReport/ |
| Max. process+thread count | 5076 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/13/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2987409273
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 30s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 33s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 27s | | master passed |
| +1 :green_heart: | compile | 1m 17s | | master passed |
| +1 :green_heart: | javadoc | 0m 43s | | master passed |
| +1 :green_heart: | shadedjars | 6m 7s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 8s | | the patch passed |
| +1 :green_heart: | compile | 1m 17s | | the patch passed |
| +1 :green_heart: | javac | 1m 17s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 43s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 3s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 41s | | hbase-common in the patch passed. |
| -1 :x: | unit | 243m 46s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/11/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 275m 45s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/11/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 56a4dcd2c866 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b29e935f262414f48b9210763b50b0fd0f349357 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/11/testReport/ |
| Max. process+thread count | 5112 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/11/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2153637454
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -241,6 +224,31 @@ public float getHeapOccupancyPercent() {
: this.heapOccupancyPercent;
}
+ private boolean isHeapMemoryUsageExceedingLimit(float memStoreFraction,
+ float blockCacheFraction) {
+ // Use integer percentage to avoid subtle float precision issues and ensure consistent
+ // comparison. This also maintains backward compatibility with previous logic relying on int
+ // truncation.
+ int memStorePercent = (int) (memStoreFraction * CONVERT_TO_PERCENTAGE);
+ int blockCachePercent = (int) (blockCacheFraction * CONVERT_TO_PERCENTAGE);
+ int minFreeHeapPercent = (int) (this.minFreeHeapFraction * CONVERT_TO_PERCENTAGE);
+
+ return memStorePercent + blockCachePercent + minFreeHeapPercent > CONVERT_TO_PERCENTAGE;
+ }
+
+ private void checkHeapMemoryLimits(String memStoreConfKey, float memStoreFraction,
+ String blockCacheConfKey, float blockCacheFraction) {
+ if (!isHeapMemoryUsageExceedingLimit(memStoreFraction, blockCacheFraction)) {
Review Comment:
Good bot!
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2134900857
##########
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:
> Using an absolute size is not only more explicit for users, but also provides more stability.
Thanks for giving my suggestion some thought. Considering the use cases and requirements, I think it's enough to offer a single parameter for setting the minimum amount of heap that should be left unreserved. Anyone who wants to bypass the check can set it to 0, and if those who want to have some safety guarantee can set it to some realistic value. I believe it covers all scenarios, and having another option might distract/confuse the users.
But that's just one perspective, and I hope to hear what others think as well.
> If we go with absolute size only, we still need to think about the default value.
Yeah, it's important to provide sensible defaults, but we should be extra careful when changing the existing ones. Following "the principal of least surprise", I would just leave the value undefined, which means that 20% of the heap is used as the threshold as before.
> I'm also considering allowing this value to be disabled (by setting it to 0)
We should still allow `0` as a valid value for the option, for users who want to bypass the check entirely. An empty value of negative value can be used instead to indicate an undefined state.
> overridden to something like 128m in [conf/hbase-site.xml](https://github.com/apache/hbase/blob/master/conf/hbase-site.xml), especially for test environments.
While I think it's reasonable, I'm not confident that it won't impact existing user setups. In pseudo-distributed environments, users often run many processes locally, each with a relatively small heap (e.g., 512MB). In such cases, even 128m could cause their setup to stop working.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "jinhyukify (via GitHub)" <gi...@apache.org>.
jinhyukify commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2134479674
##########
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;
Review Comment:
You're right. That makes sense.
I'll revert the default value to 0.2.
Please check the commit https://github.com/apache/hbase/pull/7076/commits/741129d035605131c2f77d592d632d6039c95fa0 when you get a chance!
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable and lower default to 0.1 [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2952258515
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 28s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 10s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 19s | | master passed |
| +1 :green_heart: | compile | 3m 59s | | master passed |
| +1 :green_heart: | checkstyle | 0m 52s | | master passed |
| +1 :green_heart: | spotbugs | 2m 6s | | master passed |
| +0 :ok: | refguide | 2m 26s | | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotless | 0m 49s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 6s | | the patch passed |
| +1 :green_heart: | compile | 4m 0s | | the patch passed |
| +1 :green_heart: | javac | 4m 0s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 15s | [/buildtool-patch-checkstyle-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/1/artifact/yetus-general-check/output/buildtool-patch-checkstyle-hbase-server.txt) | The patch fails to run checkstyle in hbase-server |
| +1 :green_heart: | xmllint | 0m 0s | | No new issues. |
| -1 :x: | spotbugs | 1m 50s | [/new-spotbugs-hbase-server.html](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/1/artifact/yetus-general-check/output/new-spotbugs-hbase-server.html) | hbase-server generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +0 :ok: | refguide | 2m 8s | | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 12m 10s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 45s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 18s | | The patch does not generate ASF License warnings. |
| | | 47m 44s | | |
| Reason | Tests |
|-------:|:------|
| SpotBugs | module:hbase-server |
| | Format string should use %n rather than n in org.apache.hadoop.hbase.io.util.MemorySizeUtil.validateRegionServerHeapMemoryAllocation(Configuration) At MemorySizeUtil.java:rather than n in org.apache.hadoop.hbase.io.util.MemorySizeUtil.validateRegionServerHeapMemoryAllocation(Configuration) At MemorySizeUtil.java:[line 100] |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide |
| uname | Linux 2dc0f5a83725 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / f2c683757c265267a7998f2d9bb1f5a56a28fe02 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/1/yetus-general-check/output/branch-site/book.html |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/1/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/1/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "jinhyukify (via GitHub)" <gi...@apache.org>.
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2964973478
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 28s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 10s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 18s | | master passed |
| +1 :green_heart: | compile | 3m 59s | | master passed |
| +1 :green_heart: | checkstyle | 0m 51s | | master passed |
| +1 :green_heart: | spotbugs | 2m 6s | | master passed |
| +0 :ok: | refguide | 2m 22s | | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotless | 0m 50s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 8s | | the patch passed |
| +1 :green_heart: | compile | 4m 2s | | the patch passed |
| +1 :green_heart: | javac | 4m 2s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 49s | | the patch passed |
| +1 :green_heart: | xmllint | 0m 0s | | No new issues. |
| +1 :green_heart: | spotbugs | 2m 18s | | the patch passed |
| +0 :ok: | refguide | 2m 6s | | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 12m 6s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 44s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 19s | | The patch does not generate ASF License warnings. |
| | | 47m 40s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide |
| uname | Linux ee7ff1024ab1 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / f029d1f7707d09b8e662a6061d76d8e1116ec1ff |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/6/yetus-general-check/output/branch-site/book.html |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/6/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/6/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-3078971743
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 26s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 18s | | master passed |
| +1 :green_heart: | compile | 4m 8s | | master passed |
| +1 :green_heart: | checkstyle | 0m 51s | | master passed |
| +1 :green_heart: | spotbugs | 2m 9s | | master passed |
| +0 :ok: | refguide | 2m 25s | | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotless | 0m 53s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 8s | | the patch passed |
| +1 :green_heart: | compile | 4m 1s | | the patch passed |
| +1 :green_heart: | javac | 4m 1s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 50s | | the patch passed |
| +1 :green_heart: | xmllint | 0m 0s | | No new issues. |
| +1 :green_heart: | spotbugs | 2m 21s | | the patch passed |
| +0 :ok: | refguide | 2m 8s | | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 12m 22s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 46s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 19s | | The patch does not generate ASF License warnings. |
| | | 48m 33s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/13/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide |
| uname | Linux 86aa880a8653 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3ef55a993e5ce3a71425451e4ed0753519df28e7 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/13/yetus-general-check/output/branch-site/book.html |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/13/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/13/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
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 (possibly due to a mistake), 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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2982489199
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 28s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 20s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 57s | | master passed |
| +1 :green_heart: | compile | 3m 56s | | master passed |
| +1 :green_heart: | checkstyle | 0m 50s | | master passed |
| +1 :green_heart: | spotbugs | 2m 3s | | master passed |
| +0 :ok: | refguide | 2m 6s | | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotless | 0m 44s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 58s | | the patch passed |
| +1 :green_heart: | compile | 3m 57s | | the patch passed |
| +1 :green_heart: | javac | 3m 57s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 50s | | the patch passed |
| +1 :green_heart: | xmllint | 0m 0s | | No new issues. |
| +1 :green_heart: | spotbugs | 2m 15s | | the patch passed |
| +0 :ok: | refguide | 2m 0s | | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 11m 29s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 43s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 18s | | The patch does not generate ASF License warnings. |
| | | 45m 36s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/9/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide |
| uname | Linux 8c8d3f9430af 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b29e935f262414f48b9210763b50b0fd0f349357 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/9/yetus-general-check/output/branch-site/book.html |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/9/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/9/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2227042863
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -363,14 +371,15 @@ private void tune() {
+ blockCachePercentMaxRange + ". Resetting blockCacheSize to min size");
blockCacheSize = blockCachePercentMaxRange;
}
- int gml = (int) (memstoreSize * CONVERT_TO_PERCENTAGE);
- int bcul = (int) ((blockCacheSize) * CONVERT_TO_PERCENTAGE);
- if (CONVERT_TO_PERCENTAGE - (gml + bcul) < CLUSTER_MINIMUM_MEMORY_THRESHOLD) {
+
+ if (isHeapMemoryUsageExceedingLimit(memstoreSize, blockCacheSize)) {
LOG.info("Current heap configuration from HeapMemoryTuner exceeds "
- + "the threshold required for successful cluster operation. "
- + "The combined value cannot exceed 0.8. " + MemorySizeUtil.MEMSTORE_SIZE_KEY + " is "
- + memstoreSize + " and " + HFILE_BLOCK_CACHE_SIZE_KEY + " is " + blockCacheSize);
- // TODO can adjust the value so as not exceed 80%. Is that correct? may be.
+ + "the allowed heap usage. At least " + minFreeHeapFraction
+ + " of the heap must remain free to ensure stable RegionServer operation. "
+ + MemorySizeUtil.MEMSTORE_SIZE_KEY + " is " + memstoreSize + " and "
+ + HFILE_BLOCK_CACHE_SIZE_KEY + " is " + blockCacheSize);
+ // NOTE: In the future, we might adjust values to not exceed limits,
+ // but for now tuning is skipped if over threshold.
Review Comment:
Keeping the existing comment as-is, no need to change it in this scope.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2146750447
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -184,26 +184,23 @@ private boolean doInit(Configuration conf) {
tuningEnabled = false;
}
- int gml = (int) (globalMemStorePercentMaxRange * CONVERT_TO_PERCENTAGE);
- int bcul = (int) ((blockCachePercentMinRange) * CONVERT_TO_PERCENTAGE);
- if (CONVERT_TO_PERCENTAGE - (gml + bcul) < CLUSTER_MINIMUM_MEMORY_THRESHOLD) {
+ if (isHeapMemoryUsageExceedingLimit(globalMemStorePercentMaxRange, blockCachePercentMinRange)) {
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_MAX_RANGE_KEY + " and " + BLOCK_CACHE_SIZE_MIN_RANGE_KEY
- + " in your configuration. " + MEMSTORE_SIZE_MAX_RANGE_KEY + " is "
- + globalMemStorePercentMaxRange + " and " + BLOCK_CACHE_SIZE_MIN_RANGE_KEY + " is "
+ + "the allowed heap usage. At least " + minFreeHeapFraction
+ + " of the heap must remain free to ensure stable RegionServer operation. "
+ + "Please check the settings for " + MEMSTORE_SIZE_MAX_RANGE_KEY + " and "
+ + BLOCK_CACHE_SIZE_MIN_RANGE_KEY + " in your configuration. " + MEMSTORE_SIZE_MAX_RANGE_KEY
+ + " is " + globalMemStorePercentMaxRange + " and " + BLOCK_CACHE_SIZE_MIN_RANGE_KEY + " is "
+ blockCachePercentMinRange);
}
- gml = (int) (globalMemStorePercentMinRange * CONVERT_TO_PERCENTAGE);
- bcul = (int) ((blockCachePercentMaxRange) * CONVERT_TO_PERCENTAGE);
- if (CONVERT_TO_PERCENTAGE - (gml + bcul) < CLUSTER_MINIMUM_MEMORY_THRESHOLD) {
+
+ if (isHeapMemoryUsageExceedingLimit(globalMemStorePercentMinRange, blockCachePercentMaxRange)) {
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_MIN_RANGE_KEY + " and " + BLOCK_CACHE_SIZE_MAX_RANGE_KEY
- + " in your configuration. " + MEMSTORE_SIZE_MIN_RANGE_KEY + " is "
- + globalMemStorePercentMinRange + " and " + BLOCK_CACHE_SIZE_MAX_RANGE_KEY + " is "
+ + "the allowed heap usage. At least " + minFreeHeapFraction
+ + " of the heap must remain free to ensure stable RegionServer operation. "
+ + "Please check the settings for " + MEMSTORE_SIZE_MIN_RANGE_KEY + " and "
+ + BLOCK_CACHE_SIZE_MAX_RANGE_KEY + " in your configuration. " + MEMSTORE_SIZE_MIN_RANGE_KEY
+ + " is " + globalMemStorePercentMinRange + " and " + BLOCK_CACHE_SIZE_MAX_RANGE_KEY + " is "
Review Comment:
Maybe a chance to refactor this?
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2972147686
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 28s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 43s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 24s | | master passed |
| +1 :green_heart: | compile | 4m 4s | | master passed |
| +1 :green_heart: | checkstyle | 0m 52s | | master passed |
| +1 :green_heart: | spotbugs | 2m 6s | | master passed |
| +0 :ok: | refguide | 2m 28s | | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotless | 0m 46s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 2s | | the patch passed |
| +1 :green_heart: | compile | 3m 58s | | the patch passed |
| +1 :green_heart: | javac | 3m 58s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 51s | | the patch passed |
| +1 :green_heart: | xmllint | 0m 0s | | No new issues. |
| +1 :green_heart: | spotbugs | 2m 16s | | the patch passed |
| +0 :ok: | refguide | 2m 2s | | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 11m 30s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 43s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 17s | | The patch does not generate ASF License warnings. |
| | | 47m 23s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/7/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide |
| uname | Linux 182beeeea77b 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2ae1f991aaafbe176a7b6bd28f82f338f001149e |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/7/yetus-general-check/output/branch-site/book.html |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/7/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/7/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2139990296
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java:
##########
@@ -1040,10 +1040,12 @@ 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 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_MIN_MEMORY_SIZE_KEY =
Review Comment:
It appears the team wants to avoid making HConstants any bigger if possible.
https://github.com/apache/hbase/blob/5bc0f4c3e5aae08045a11fef12c6a9f50aa0ee85/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java#L38
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2139974904
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java:
##########
@@ -1040,10 +1040,12 @@ 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 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;
Review Comment:
Can we restore this constant? HConstants is `@InterfaceAudience.Public` so we should not remove the existing ones.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2980867601
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 28s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 32s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 17s | | master passed |
| +1 :green_heart: | compile | 4m 8s | | master passed |
| +1 :green_heart: | checkstyle | 0m 50s | | master passed |
| +1 :green_heart: | spotbugs | 2m 5s | | master passed |
| +0 :ok: | refguide | 2m 31s | | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotless | 0m 46s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 0s | | the patch passed |
| +1 :green_heart: | compile | 4m 0s | | the patch passed |
| +1 :green_heart: | javac | 4m 0s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 52s | | the patch passed |
| +1 :green_heart: | xmllint | 0m 0s | | No new issues. |
| +1 :green_heart: | spotbugs | 2m 17s | | the patch passed |
| +0 :ok: | refguide | 2m 12s | | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 11m 41s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 43s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 18s | | The patch does not generate ASF License warnings. |
| | | 47m 39s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/8/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide |
| uname | Linux 243b098685f2 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / e02fe517d1d6adf1548440eddcecd2cf208ec3b2 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/8/yetus-general-check/output/branch-site/book.html |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/8/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 87 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/8/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable and lower default to 0.1 [hbase]
Posted by "jinhyukify (via GitHub)" <gi...@apache.org>.
jinhyukify commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2134469078
##########
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:
I realize I didn't provide enough context earlier, sorry about that.
The naming of this configuration follows the convention used by the existing block cache size configurations. There are currently two settings that determine the size of the block cache: ([HBASE-28600](https://issues.apache.org/jira/browse/HBASE-28600))
- `hfile.block.cache.size`
- `hfile.block.cache.memory.size`
While users are generally familiar with the fraction-based approach, I thought it would be helpful to introduce an alternative that accepts human-readable memory sizes (e.g., 512m, 4g). So both options are now available.
At least for memory-related configurations, such as free heap, memstore, and block cache, I believe supporting both styles (fraction and absolute size) makes sense for the near future. Would love to hear your thoughts!
(For now, this dual option is only available for block cache, but I’m planning to extend it to memstore configuration soon as well.)
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "jinhyukify (via GitHub)" <gi...@apache.org>.
jinhyukify commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2134585925
##########
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:
Thanks for the detailed insights 😄
> 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?
You're right. There wasn't a clear convention initially, but following the existing blockCache configuration and using the `memory.size` suffix seems like a better approach.
> So could you explain how you are going to use these parameters on your clusters?
I was planning to use an absolute value for the heap size as well.
The reason I first added the fraction-based version was because the original config was already managed as a fraction.
But I agree with your point. Using an absolute size is not only more explicit for users, but also provides more stability.
Unlike memStore or blockCache, where we're allocating memory, this one is about preserving a minimum free heap, so an absolute value feels more appropriate.
If we go with absolute size only, we still need to think about the default value.
I typically use at least 31GB heap per RegionServer, but for low-spec VMs (e.g. 4GB heap), something like `512m` might make more sense? To avoid startup issues in smaller clusters, I’m leaning toward a conservative default for now.
I'm also considering allowing this value to be disabled (by setting it to 0) or overridden to something like 128m in [`conf/hbase-site.xml`](https://github.com/apache/hbase/blob/master/conf/hbase-site.xml), especially for test environments.
Do you have any thoughts on this?
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-3130439952
@jinhyukify Before we merge this to master, could you backport this to branch-2 and open another pull request?
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "jinhyukify (via GitHub)" <gi...@apache.org>.
jinhyukify commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2146347709
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -241,6 +238,15 @@ public float getHeapOccupancyPercent() {
: this.heapOccupancyPercent;
}
+ private boolean isHeapMemoryUsageExceedingLimit(float memStoreFraction,
+ float blockCacheFraction) {
+ int memStorePercent = (int) (memStoreFraction * CONVERT_TO_PERCENTAGE);
+ int blockCachePercent = (int) (blockCacheFraction * CONVERT_TO_PERCENTAGE);
+ int minFreeHeapPercent = (int) (this.minFreeHeapFraction * CONVERT_TO_PERCENTAGE);
+
+ return memStorePercent + blockCachePercent + minFreeHeapPercent > CONVERT_TO_PERCENTAGE;
Review Comment:
Thanks for pointing this out! I also gave this some thought.
At first, I assumed there must be a reason for converting to int before doing the comparison, since it's done that way in some methods in [MemoryUtil](https://github.com/apache/hbase/pull/7076/files#diff-6e3b744afc3d42d8708b76312e06419665aac8132f3a96e8307ba252b2f91e12L88-L94) as well.
Initially, I thought it might be to avoid floating-point precision issues.
However, on closer inspection, I'm not sure if casting to int provides any real advantage - especially since it introduces potential error due to truncation.
In effect, this approach ends up truncating values at the third decimal place, which could lead to subtle changes in behavior for users relying on specific configuration values.
For instance, with memstore=0.409, blockcache=0.4, and freeHeap=0.2, the previous logic wouldn’t have flagged this as exceeding memory limits. But with this change, it might.
What are your thoughts on this? 😄
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "jinhyukify (via GitHub)" <gi...@apache.org>.
jinhyukify commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2152500747
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -241,6 +238,15 @@ public float getHeapOccupancyPercent() {
: this.heapOccupancyPercent;
}
+ private boolean isHeapMemoryUsageExceedingLimit(float memStoreFraction,
+ float blockCacheFraction) {
+ int memStorePercent = (int) (memStoreFraction * CONVERT_TO_PERCENTAGE);
+ int blockCachePercent = (int) (blockCacheFraction * CONVERT_TO_PERCENTAGE);
+ int minFreeHeapPercent = (int) (this.minFreeHeapFraction * CONVERT_TO_PERCENTAGE);
+
+ return memStorePercent + blockCachePercent + minFreeHeapPercent > CONVERT_TO_PERCENTAGE;
Review Comment:
Thank you!
Added comment in https://github.com/apache/hbase/pull/7076/commits/e02fe517d1d6adf1548440eddcecd2cf208ec3b2
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2980881846
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 29s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 39s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 39s | | master passed |
| +1 :green_heart: | compile | 1m 18s | | master passed |
| +1 :green_heart: | javadoc | 0m 45s | | master passed |
| +1 :green_heart: | shadedjars | 6m 17s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 9s | | the patch passed |
| +1 :green_heart: | compile | 1m 18s | | the patch passed |
| +1 :green_heart: | javac | 1m 18s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 44s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 6s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 17s | | hbase-common in the patch passed. |
| -1 :x: | unit | 21m 34s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/8/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 50m 36s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/8/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux b34cddb3ae62 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / e02fe517d1d6adf1548440eddcecd2cf208ec3b2 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/8/testReport/ |
| Max. process+thread count | 2027 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/8/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2982861911
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 33s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 32s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 29s | | master passed |
| +1 :green_heart: | compile | 1m 17s | | master passed |
| +1 :green_heart: | javadoc | 0m 44s | | master passed |
| +1 :green_heart: | shadedjars | 6m 10s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 8s | | the patch passed |
| +1 :green_heart: | compile | 1m 18s | | the patch passed |
| +1 :green_heart: | javac | 1m 18s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 43s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 1s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 39s | | hbase-common in the patch passed. |
| -1 :x: | unit | 218m 3s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/9/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 249m 54s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/9/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux d611810348d8 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b29e935f262414f48b9210763b50b0fd0f349357 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/9/testReport/ |
| Max. process+thread count | 5090 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/9/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2227033245
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/util/MemorySizeUtil.java:
##########
@@ -77,34 +82,70 @@ 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: total memory usage exceeds 100%% "
+ + "(memStore + blockCache + requiredFreeHeap). "
+ + "Check the following configuration values:%n" + " - %s = %.2f%n" + " - %s = %s%n"
+ + " - %s = %s%n" + " - %s = %s",
+ MEMSTORE_SIZE_KEY, memStoreFraction, 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),
+ HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY,
+ conf.get(HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY)));
+ }
+ }
+
+ /**
+ * Retrieve an explicit minimum required free heap size in bytes in the configuration.
+ * @param conf used to read configs
+ * @return the minimum required free heap size in bytes, or a negative value if not configured.
+ * @throws IllegalArgumentException if HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY format is
+ * invalid
+ */
+ private static long getRegionServerMinFreeHeapInBytes(Configuration conf) {
+ final String key = HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY;
+ try {
+ return Long.parseLong(conf.get(key));
Review Comment:
If conf.get(key) returns null, Long.parseLong() will throw NumberFormatException. This should be handled by checking for null first or using a default value.
```suggestion
String value = conf.get(key);
if (value == null) {
return (long) conf.getStorageSize(key, -1, StorageUnit.BYTES);
}
try {
return Long.parseLong(value);
```
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/util/MemorySizeUtil.java:
##########
@@ -77,34 +82,70 @@ 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: total memory usage exceeds 100%% "
+ + "(memStore + blockCache + requiredFreeHeap). "
+ + "Check the following configuration values:%n" + " - %s = %.2f%n" + " - %s = %s%n"
+ + " - %s = %s%n" + " - %s = %s",
+ MEMSTORE_SIZE_KEY, memStoreFraction, 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),
+ HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY,
+ conf.get(HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY)));
+ }
+ }
+
+ /**
+ * Retrieve an explicit minimum required free heap size in bytes in the configuration.
+ * @param conf used to read configs
+ * @return the minimum required free heap size in bytes, or a negative value if not configured.
+ * @throws IllegalArgumentException if HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY format is
+ * invalid
+ */
+ private static long getRegionServerMinFreeHeapInBytes(Configuration conf) {
Review Comment:
The method could throw IllegalArgumentException but the exception handling only catches NumberFormatException. If conf.getStorageSize() throws IllegalArgumentException for invalid format, it won't be caught and the documented exception won't be thrown as expected.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -363,14 +371,15 @@ private void tune() {
+ blockCachePercentMaxRange + ". Resetting blockCacheSize to min size");
blockCacheSize = blockCachePercentMaxRange;
}
- int gml = (int) (memstoreSize * CONVERT_TO_PERCENTAGE);
- int bcul = (int) ((blockCacheSize) * CONVERT_TO_PERCENTAGE);
- if (CONVERT_TO_PERCENTAGE - (gml + bcul) < CLUSTER_MINIMUM_MEMORY_THRESHOLD) {
+
+ if (isHeapMemoryUsageExceedingLimit(memstoreSize, blockCacheSize)) {
LOG.info("Current heap configuration from HeapMemoryTuner exceeds "
- + "the threshold required for successful cluster operation. "
- + "The combined value cannot exceed 0.8. " + MemorySizeUtil.MEMSTORE_SIZE_KEY + " is "
- + memstoreSize + " and " + HFILE_BLOCK_CACHE_SIZE_KEY + " is " + blockCacheSize);
- // TODO can adjust the value so as not exceed 80%. Is that correct? may be.
+ + "the allowed heap usage. At least " + minFreeHeapFraction
+ + " of the heap must remain free to ensure stable RegionServer operation. "
+ + MemorySizeUtil.MEMSTORE_SIZE_KEY + " is " + memstoreSize + " and "
+ + HFILE_BLOCK_CACHE_SIZE_KEY + " is " + blockCacheSize);
+ // NOTE: In the future, we might adjust values to not exceed limits,
+ // but for now tuning is skipped if over threshold.
Review Comment:
[nitpick] This TODO-style comment should either be converted to a proper TODO with a tracking issue, or removed if no action is planned. Comments about future work without clear ownership can become stale.
```suggestion
// Tuning is skipped if the current configuration exceeds the allowed heap usage limits.
// This ensures stable RegionServer operation under the current constraints.
```
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2134900857
##########
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:
> Using an absolute size is not only more explicit for users, but also provides more stability.
Thanks for giving my suggestion some thought. Considering the use cases and requirements, I think it's enough to offer a single parameter for setting the minimum amount of heap that should be left unreserved. Anyone who wants to bypass the check can set it to 0, and those who want to have some safety guarantee can set it to some realistic value. I believe it covers all scenarios, and having another option might distract/confuse the users.
But that's just one perspective, and I hope to hear what others think as well.
> If we go with absolute size only, we still need to think about the default value.
Yeah, it's important to provide sensible defaults, but we should be extra careful when changing the existing ones. Following "the principal of least surprise", I would just leave the value undefined, which means that 20% of the heap is used as the threshold as before.
> I'm also considering allowing this value to be disabled (by setting it to 0)
We should still allow `0` as a valid value for the option, for users who want to bypass the check entirely. An empty value of negative value can be used instead to indicate an undefined state.
> overridden to something like 128m in [conf/hbase-site.xml](https://github.com/apache/hbase/blob/master/conf/hbase-site.xml), especially for test environments.
While I think it's reasonable, I'm not confident that it won't impact existing user setups. In pseudo-distributed environments, users often run many processes locally, each with a relatively small heap (e.g., 512MB). In such cases, even 128m could cause their setup to stop working.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable and lower default to 0.1 [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2952557464
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 32s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 16s | | master passed |
| +1 :green_heart: | compile | 4m 2s | | master passed |
| +1 :green_heart: | checkstyle | 0m 50s | | master passed |
| +1 :green_heart: | spotbugs | 2m 8s | | master passed |
| +0 :ok: | refguide | 2m 18s | | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotless | 0m 47s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 54s | | the patch passed |
| +1 :green_heart: | compile | 3m 57s | | the patch passed |
| +1 :green_heart: | javac | 3m 57s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 49s | | the patch passed |
| +1 :green_heart: | xmllint | 0m 0s | | No new issues. |
| +1 :green_heart: | spotbugs | 2m 15s | | the patch passed |
| +0 :ok: | refguide | 2m 2s | | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 11m 39s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 42s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 18s | | The patch does not generate ASF License warnings. |
| | | 46m 39s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide |
| uname | Linux 36da4e3eebcb 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 23c758c83a57081806c144af433b3792c86a42bb |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/2/yetus-general-check/output/branch-site/book.html |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/2/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/2/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2953949521
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 3m 4s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 14s | | master passed |
| +1 :green_heart: | compile | 1m 16s | | master passed |
| +1 :green_heart: | javadoc | 0m 43s | | master passed |
| +1 :green_heart: | shadedjars | 6m 3s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 5s | | the patch passed |
| +1 :green_heart: | compile | 1m 15s | | the patch passed |
| +1 :green_heart: | javac | 1m 15s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 42s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 2s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 38s | | hbase-common in the patch passed. |
| -1 :x: | unit | 213m 45s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/3/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 247m 5s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/3/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 2e2871f3b805 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 741129d035605131c2f77d592d632d6039c95fa0 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/3/testReport/ |
| Max. process+thread count | 4694 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/3/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2972163067
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 29s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 37s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 35s | | master passed |
| +1 :green_heart: | compile | 1m 16s | | master passed |
| +1 :green_heart: | javadoc | 0m 43s | | master passed |
| +1 :green_heart: | shadedjars | 5m 56s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 3s | | the patch passed |
| +1 :green_heart: | compile | 1m 17s | | the patch passed |
| +1 :green_heart: | javac | 1m 17s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 43s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 3s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 37s | | hbase-common in the patch passed. |
| -1 :x: | unit | 37m 47s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/7/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 66m 23s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/7/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux aa6dc0935778 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2ae1f991aaafbe176a7b6bd28f82f338f001149e |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/7/testReport/ |
| Max. process+thread count | 2080 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/7/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2959985980
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 29s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 10s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 10s | | master passed |
| +1 :green_heart: | compile | 4m 1s | | master passed |
| +1 :green_heart: | checkstyle | 0m 53s | | master passed |
| +1 :green_heart: | spotbugs | 2m 8s | | master passed |
| +0 :ok: | refguide | 2m 22s | | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotless | 0m 49s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 58s | | the patch passed |
| +1 :green_heart: | compile | 4m 5s | | the patch passed |
| +1 :green_heart: | javac | 4m 5s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 49s | | the patch passed |
| +1 :green_heart: | xmllint | 0m 0s | | No new issues. |
| +1 :green_heart: | spotbugs | 2m 18s | | the patch passed |
| +0 :ok: | refguide | 2m 2s | | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 11m 42s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 43s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 19s | | The patch does not generate ASF License warnings. |
| | | 46m 47s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide |
| uname | Linux e81f7c6bb355 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3d2b641ef55012997554be85acd2bb9140d7eb73 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/4/yetus-general-check/output/branch-site/book.html |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/4/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/4/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable and lower default to 0.1 [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2952498701
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 31s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 18s | | master passed |
| +1 :green_heart: | compile | 1m 20s | | master passed |
| +1 :green_heart: | javadoc | 0m 44s | | master passed |
| +1 :green_heart: | shadedjars | 6m 3s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 4s | | the patch passed |
| +1 :green_heart: | compile | 1m 16s | | the patch passed |
| +1 :green_heart: | javac | 1m 16s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 42s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 0s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 42s | | hbase-common in the patch passed. |
| -1 :x: | unit | 254m 3s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/1/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 285m 50s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/1/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux be5176212ac1 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / f2c683757c265267a7998f2d9bb1f5a56a28fe02 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/1/testReport/ |
| Max. process+thread count | 4989 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/1/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2134536849
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHeapMemoryManager.java:
##########
@@ -72,6 +72,7 @@ public void testAutoTunerShouldBeOffWhenMaxMinRangesForMemstoreIsNotGiven() thro
conf.setFloat(MemorySizeUtil.MEMSTORE_SIZE_KEY, 0.02f);
conf.setFloat(HeapMemoryManager.BLOCK_CACHE_SIZE_MAX_RANGE_KEY, 0.75f);
conf.setFloat(HeapMemoryManager.BLOCK_CACHE_SIZE_MIN_RANGE_KEY, 0.03f);
+ conf.setFloat(HConstants.HBASE_REGION_SERVER_FREE_HEAP_MIN_SIZE_KEY, 0.2f);
Review Comment:
So we have reverted the default. Do we still need this `setFloat`s?
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHeapMemoryManager.java:
##########
@@ -72,6 +72,7 @@ public void testAutoTunerShouldBeOffWhenMaxMinRangesForMemstoreIsNotGiven() thro
conf.setFloat(MemorySizeUtil.MEMSTORE_SIZE_KEY, 0.02f);
conf.setFloat(HeapMemoryManager.BLOCK_CACHE_SIZE_MAX_RANGE_KEY, 0.75f);
conf.setFloat(HeapMemoryManager.BLOCK_CACHE_SIZE_MIN_RANGE_KEY, 0.03f);
+ conf.setFloat(HConstants.HBASE_REGION_SERVER_FREE_HEAP_MIN_SIZE_KEY, 0.2f);
Review Comment:
So we have reverted the default. Do we still need these `setFloat`s?
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2134900857
##########
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:
> Using an absolute size is not only more explicit for users, but also provides more stability.
Thanks for giving my suggestion some thought. Considering the use cases and requirements, I think it's enough to offer a single parameter for setting the minimum amount of heap that should be left unreserved. Anyone who wants to bypass the check can set it to 0, and those who want to have some safety guarantee can set it to some realistic value. I believe it covers all scenarios, and having another option might distract/confuse the users.
But that's just one perspective, and I hope to hear what others think as well.
> If we go with absolute size only, we still need to think about the default value.
Yeah, it's important to provide sensible defaults, but we should be extra careful when changing the existing ones or introducing one that might impact the existing users. Following "the principal of least surprise", I would just leave the value undefined, which means that 20% of the heap is used as the threshold as before.
> I'm also considering allowing this value to be disabled (by setting it to 0)
We should still allow `0` as a valid value for the option, for users who want to bypass the check entirely. An empty value or a negative value can be used instead to indicate an undefined state.
> overridden to something like 128m in [conf/hbase-site.xml](https://github.com/apache/hbase/blob/master/conf/hbase-site.xml), especially for test environments.
While I think it's reasonable, I'm not confident that it won't impact existing user setups. In pseudo-distributed environments, users often run many processes locally, each with a relatively small heap (e.g., 512MB). In such cases, even 128m could cause their setup to stop working.
##########
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:
> Using an absolute size is not only more explicit for users, but also provides more stability.
Thanks for giving my suggestion some thought. Considering the use cases and requirements, I think it's enough to offer a single parameter for setting the minimum amount of heap that should be left unreserved. Anyone who wants to bypass the check can set it to 0, and those who want to have some safety guarantee can set it to some realistic value. I believe it covers all scenarios, and having another option might distract/confuse the users.
But that's just one perspective, and I hope to hear what others think as well.
> If we go with absolute size only, we still need to think about the default value.
Yeah, it's important to provide sensible defaults, but we should be extra careful when changing the existing ones or introducing one that might impact the existing use cases. Following "the principal of least surprise", I would just leave the value undefined, which means that 20% of the heap is used as the threshold as before.
> I'm also considering allowing this value to be disabled (by setting it to 0)
We should still allow `0` as a valid value for the option, for users who want to bypass the check entirely. An empty value or a negative value can be used instead to indicate an undefined state.
> overridden to something like 128m in [conf/hbase-site.xml](https://github.com/apache/hbase/blob/master/conf/hbase-site.xml), especially for test environments.
While I think it's reasonable, I'm not confident that it won't impact existing user setups. In pseudo-distributed environments, users often run many processes locally, each with a relatively small heap (e.g., 512MB). In such cases, even 128m could cause their setup to stop working.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2153462589
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -241,6 +224,31 @@ public float getHeapOccupancyPercent() {
: this.heapOccupancyPercent;
}
+ private boolean isHeapMemoryUsageExceedingLimit(float memStoreFraction,
+ float blockCacheFraction) {
+ // Use integer percentage to avoid subtle float precision issues and ensure consistent
+ // comparison. This also maintains backward compatibility with previous logic relying on int
+ // truncation.
+ int memStorePercent = (int) (memStoreFraction * CONVERT_TO_PERCENTAGE);
+ int blockCachePercent = (int) (blockCacheFraction * CONVERT_TO_PERCENTAGE);
+ int minFreeHeapPercent = (int) (this.minFreeHeapFraction * CONVERT_TO_PERCENTAGE);
+
+ return memStorePercent + blockCachePercent + minFreeHeapPercent > CONVERT_TO_PERCENTAGE;
+ }
+
+ private void checkHeapMemoryLimits(String memStoreConfKey, float memStoreFraction,
+ String blockCacheConfKey, float blockCacheFraction) {
+ if (!isHeapMemoryUsageExceedingLimit(memStoreFraction, blockCacheFraction)) {
Review Comment:
The negation is reversed: currently this throws when the heap usage is within limits. It should throw only when `isHeapMemoryUsageExceedingLimit(...)` returns true.
```suggestion
if (isHeapMemoryUsageExceedingLimit(memStoreFraction, blockCacheFraction)) {
```
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2960467191
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 31s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 10s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 16s | | master passed |
| +1 :green_heart: | compile | 1m 17s | | master passed |
| +1 :green_heart: | javadoc | 0m 44s | | master passed |
| +1 :green_heart: | shadedjars | 6m 4s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 14s | | the patch passed |
| +1 :green_heart: | compile | 1m 17s | | the patch passed |
| +1 :green_heart: | javac | 1m 17s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 43s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 0s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 14s | | hbase-common in the patch passed. |
| -1 :x: | unit | 215m 40s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/4/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 246m 10s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/4/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux a031d4a1c332 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3d2b641ef55012997554be85acd2bb9140d7eb73 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/4/testReport/ |
| Max. process+thread count | 5149 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/4/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2963580325
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 27s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 17s | | master passed |
| +1 :green_heart: | compile | 1m 16s | | master passed |
| +1 :green_heart: | javadoc | 0m 44s | | master passed |
| +1 :green_heart: | shadedjars | 6m 4s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 10s | | the patch passed |
| +1 :green_heart: | compile | 1m 15s | | the patch passed |
| +1 :green_heart: | javac | 1m 15s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 42s | | the patch passed |
| +1 :green_heart: | shadedjars | 5m 58s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 15s | | hbase-common in the patch passed. |
| -1 :x: | unit | 213m 36s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/5/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 243m 49s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/5/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux e094be16236d 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 772e71a2029a1c89c413c726cb3c88f0c8611b22 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/5/testReport/ |
| Max. process+thread count | 5785 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/5/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2994341097
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 30s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 33s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 24s | | master passed |
| +1 :green_heart: | compile | 3m 56s | | master passed |
| +1 :green_heart: | checkstyle | 0m 50s | | master passed |
| +1 :green_heart: | spotbugs | 2m 5s | | master passed |
| +0 :ok: | refguide | 2m 28s | | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotless | 0m 49s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 10s | | the patch passed |
| +1 :green_heart: | compile | 4m 53s | | the patch passed |
| +1 :green_heart: | javac | 4m 53s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 1m 2s | | the patch passed |
| +1 :green_heart: | xmllint | 0m 0s | | No new issues. |
| +1 :green_heart: | spotbugs | 3m 0s | | the patch passed |
| +0 :ok: | refguide | 2m 26s | | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 12m 14s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 48s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 17s | | The patch does not generate ASF License warnings. |
| | | 52m 32s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/12/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide |
| uname | Linux 4fd648b1915f 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 87bfbe7f967d50da45421012a763b042c5e16288 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/12/yetus-general-check/output/branch-site/book.html |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/12/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/12/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "jinhyukify (via GitHub)" <gi...@apache.org>.
jinhyukify commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2152499188
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -184,26 +184,23 @@ private boolean doInit(Configuration conf) {
tuningEnabled = false;
}
- int gml = (int) (globalMemStorePercentMaxRange * CONVERT_TO_PERCENTAGE);
- int bcul = (int) ((blockCachePercentMinRange) * CONVERT_TO_PERCENTAGE);
- if (CONVERT_TO_PERCENTAGE - (gml + bcul) < CLUSTER_MINIMUM_MEMORY_THRESHOLD) {
+ if (isHeapMemoryUsageExceedingLimit(globalMemStorePercentMaxRange, blockCachePercentMinRange)) {
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_MAX_RANGE_KEY + " and " + BLOCK_CACHE_SIZE_MIN_RANGE_KEY
- + " in your configuration. " + MEMSTORE_SIZE_MAX_RANGE_KEY + " is "
- + globalMemStorePercentMaxRange + " and " + BLOCK_CACHE_SIZE_MIN_RANGE_KEY + " is "
+ + "the allowed heap usage. At least " + minFreeHeapFraction
+ + " of the heap must remain free to ensure stable RegionServer operation. "
+ + "Please check the settings for " + MEMSTORE_SIZE_MAX_RANGE_KEY + " and "
+ + BLOCK_CACHE_SIZE_MIN_RANGE_KEY + " in your configuration. " + MEMSTORE_SIZE_MAX_RANGE_KEY
+ + " is " + globalMemStorePercentMaxRange + " and " + BLOCK_CACHE_SIZE_MIN_RANGE_KEY + " is "
+ blockCachePercentMinRange);
}
- gml = (int) (globalMemStorePercentMinRange * CONVERT_TO_PERCENTAGE);
- bcul = (int) ((blockCachePercentMaxRange) * CONVERT_TO_PERCENTAGE);
- if (CONVERT_TO_PERCENTAGE - (gml + bcul) < CLUSTER_MINIMUM_MEMORY_THRESHOLD) {
+
+ if (isHeapMemoryUsageExceedingLimit(globalMemStorePercentMinRange, blockCachePercentMaxRange)) {
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_MIN_RANGE_KEY + " and " + BLOCK_CACHE_SIZE_MAX_RANGE_KEY
- + " in your configuration. " + MEMSTORE_SIZE_MIN_RANGE_KEY + " is "
- + globalMemStorePercentMinRange + " and " + BLOCK_CACHE_SIZE_MAX_RANGE_KEY + " is "
+ + "the allowed heap usage. At least " + minFreeHeapFraction
+ + " of the heap must remain free to ensure stable RegionServer operation. "
+ + "Please check the settings for " + MEMSTORE_SIZE_MIN_RANGE_KEY + " and "
+ + BLOCK_CACHE_SIZE_MAX_RANGE_KEY + " in your configuration. " + MEMSTORE_SIZE_MIN_RANGE_KEY
+ + " is " + globalMemStorePercentMinRange + " and " + BLOCK_CACHE_SIZE_MAX_RANGE_KEY + " is "
Review Comment:
Thank you!
Applied in https://github.com/apache/hbase/pull/7076/commits/8fdb6ded0153dd938cef59edc30d557947f04685
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2983425245
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 40s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 23s | | master passed |
| +1 :green_heart: | compile | 4m 44s | | master passed |
| +1 :green_heart: | checkstyle | 1m 11s | | master passed |
| +1 :green_heart: | spotbugs | 2m 48s | | master passed |
| +0 :ok: | refguide | 3m 17s | | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotless | 1m 18s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 56s | | the patch passed |
| +1 :green_heart: | compile | 4m 49s | | the patch passed |
| +1 :green_heart: | javac | 4m 49s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 55s | | the patch passed |
| +1 :green_heart: | xmllint | 0m 1s | | No new issues. |
| +1 :green_heart: | spotbugs | 2m 41s | | the patch passed |
| +0 :ok: | refguide | 2m 26s | | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 13m 7s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 44s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 18s | | The patch does not generate ASF License warnings. |
| | | 57m 32s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/10/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide |
| uname | Linux 1dc2ebd7ed97 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b29e935f262414f48b9210763b50b0fd0f349357 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/10/yetus-general-check/output/branch-site/book.html |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/10/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 87 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/10/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2145216538
##########
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%%. "
+ + "Check the following configuration values:%n" + " - %s = %.2f%n" + " - %s = %s%n"
+ + " - %s = %s%n" + " - %s = %s",
+ memStorePercent, blockCachePercent, minFreeHeapPercent, usedPercent, maxAllowedUsed,
+ MEMSTORE_SIZE_KEY, memStoreFraction, 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),
+ MemorySizeUtil.HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY,
+ conf.get(MemorySizeUtil.HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY)));
Review Comment:
`MemorySizeUtil.` seems unnecessary.
```suggestion
HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY,
conf.get(HBASE_REGION_SERVER_FREE_HEAP_MIN_MEMORY_SIZE_KEY)));
```
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2962873930
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 29s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 32s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 20s | | master passed |
| +1 :green_heart: | compile | 3m 59s | | master passed |
| +1 :green_heart: | checkstyle | 0m 52s | | master passed |
| +1 :green_heart: | spotbugs | 2m 5s | | master passed |
| +0 :ok: | refguide | 2m 34s | | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotless | 0m 46s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 59s | | the patch passed |
| +1 :green_heart: | compile | 4m 0s | | the patch passed |
| +1 :green_heart: | javac | 4m 0s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 51s | | the patch passed |
| +1 :green_heart: | xmllint | 0m 0s | | No new issues. |
| +1 :green_heart: | spotbugs | 2m 17s | | the patch passed |
| +0 :ok: | refguide | 2m 5s | | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 11m 39s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 43s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 18s | | The patch does not generate ASF License warnings. |
| | | 47m 10s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide |
| uname | Linux dc28c6499563 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 772e71a2029a1c89c413c726cb3c88f0c8611b22 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/5/yetus-general-check/output/branch-site/book.html |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/5/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 83 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/5/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2145251529
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -241,6 +238,15 @@ public float getHeapOccupancyPercent() {
: this.heapOccupancyPercent;
}
+ private boolean isHeapMemoryUsageExceedingLimit(float memStoreFraction,
+ float blockCacheFraction) {
+ int memStorePercent = (int) (memStoreFraction * CONVERT_TO_PERCENTAGE);
+ int blockCachePercent = (int) (blockCacheFraction * CONVERT_TO_PERCENTAGE);
+ int minFreeHeapPercent = (int) (this.minFreeHeapFraction * CONVERT_TO_PERCENTAGE);
+
+ return memStorePercent + blockCachePercent + minFreeHeapPercent > CONVERT_TO_PERCENTAGE;
Review Comment:
How about we just compare the sum of the float values against 1?
```suggestion
return memStoreFraction + blockCacheFraction + minFreeHeapFraction > 1.0;
```
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "jinhyukify (via GitHub)" <gi...@apache.org>.
jinhyukify commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-3114036340
Thank you so much for your thorough review.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2953699984
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 28s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 30s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 14s | | master passed |
| +1 :green_heart: | compile | 3m 54s | | master passed |
| +1 :green_heart: | checkstyle | 0m 50s | | master passed |
| +1 :green_heart: | spotbugs | 2m 2s | | master passed |
| +0 :ok: | refguide | 2m 9s | | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotless | 0m 44s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 57s | | the patch passed |
| +1 :green_heart: | compile | 4m 2s | | the patch passed |
| +1 :green_heart: | javac | 4m 2s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 50s | | the patch passed |
| +1 :green_heart: | xmllint | 0m 1s | | No new issues. |
| +1 :green_heart: | spotbugs | 2m 16s | | the patch passed |
| +0 :ok: | refguide | 2m 8s | | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 11m 32s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 44s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 18s | | The patch does not generate ASF License warnings. |
| | | 46m 29s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide |
| uname | Linux 8653b3198866 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 741129d035605131c2f77d592d632d6039c95fa0 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/3/yetus-general-check/output/branch-site/book.html |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/3/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/3/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-3109825931
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 35s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 37s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 34s | | master passed |
| +1 :green_heart: | compile | 1m 17s | | master passed |
| +1 :green_heart: | javadoc | 0m 44s | | master passed |
| +1 :green_heart: | shadedjars | 6m 7s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 8s | | the patch passed |
| +1 :green_heart: | compile | 1m 17s | | the patch passed |
| +1 :green_heart: | javac | 1m 17s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 43s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 5s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 40s | | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 215m 12s | | hbase-server in the patch passed. |
| | | 246m 51s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/14/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 13b27f73861b 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / e583c2b9e414a76ad16bcee41c9c822d5f9eaada |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/14/testReport/ |
| Max. process+thread count | 4414 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/14/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "jinhyukify (via GitHub)" <gi...@apache.org>.
jinhyukify commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2210529671
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/util/TestMemorySizeUtil.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.io.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.testclassification.MiscTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({ MiscTests.class, SmallTests.class })
+public class TestMemorySizeUtil {
+
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+ HBaseClassTestRule.forClass(TestMemorySizeUtil.class);
+
+ private Configuration conf;
+
+ @Before
+ public void setup() {
+ conf = new Configuration();
+ }
+
+ @Test
+ public void testValidateRegionServerHeapMemoryAllocation() {
+ // when memstore size + block cache size + default free heap min size == 1.0
+ conf.setFloat(MemorySizeUtil.MEMSTORE_SIZE_KEY, 0.4f);
+ conf.setFloat(HConstants.HFILE_BLOCK_CACHE_SIZE_KEY, 0.4f);
+ assertEquals(HConstants.HBASE_CLUSTER_MINIMUM_MEMORY_THRESHOLD, 0.2f, 0.0f);
+ MemorySizeUtil.validateRegionServerHeapMemoryAllocation(conf);
+
+ // when memstore size + block cache size + default free heap min size > 1.0
+ conf.setFloat(MemorySizeUtil.MEMSTORE_SIZE_KEY, 0.5f);
+ assertThrows(RuntimeException.class,
+ () -> MemorySizeUtil.validateRegionServerHeapMemoryAllocation(conf));
+ }
Review Comment:
Thank you!
I’ve added a case to handle situations where the configuration is set to 0 or an invalid negative value.
Please take a look when you have a chance! 3ef55a993e5ce3a71425451e4ed0753519df28e7
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable and lower default to 0.1 [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2952803913
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 28s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 16s | | master passed |
| +1 :green_heart: | compile | 1m 17s | | master passed |
| +1 :green_heart: | javadoc | 0m 44s | | master passed |
| +1 :green_heart: | shadedjars | 6m 7s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 9s | | the patch passed |
| +1 :green_heart: | compile | 1m 16s | | the patch passed |
| +1 :green_heart: | javac | 1m 16s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 43s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 3s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 13s | | hbase-common in the patch passed. |
| -1 :x: | unit | 215m 55s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/2/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 246m 40s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 2cd253b6371b 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 23c758c83a57081806c144af433b3792c86a42bb |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/2/testReport/ |
| Max. process+thread count | 5743 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/2/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable and lower default to 0.1 [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2134000510
##########
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:
It's not immediately clear to me that `heap.min.size` expects a fraction, whereas `heap.memory.min.size` expects an actual size. But I don't really have a better idea, and I can imagine you had a hard time coming up with those names anyway, given that some existing `.size` parameters expect an actual size, and some other `.size` parameters expect a fraction.
That said, do you think we need both parameters? Wouldn't the actual size (which takes precedence over the fraction value) be enough in most cases?
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
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 is used instead and set 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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2134900857
##########
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:
> Using an absolute size is not only more explicit for users, but also provides more stability.
Thanks for giving my suggestion some thought. Considering the use cases and requirements, I think it's enough to offer a single parameter for setting the minimum amount of heap that should be left unreserved. Anyone who wants to bypass the check can set it to 0, and those who want to have some safety guarantee can set it to some realistic value. I believe it covers all scenarios, and having another option might distract/confuse the users.
But that's just one perspective, and I hope to hear what others think as well.
> If we go with absolute size only, we still need to think about the default value.
Yeah, it's important to provide sensible defaults, but we should be extra careful when changing the existing ones. Following "the principal of least surprise", I would just leave the value undefined, which means that 20% of the heap is used as the threshold as before.
> I'm also considering allowing this value to be disabled (by setting it to 0)
We should still allow `0` as a valid value for the option, for users who want to bypass the check entirely. An empty value or a negative value can be used instead to indicate an undefined state.
> overridden to something like 128m in [conf/hbase-site.xml](https://github.com/apache/hbase/blob/master/conf/hbase-site.xml), especially for test environments.
While I think it's reasonable, I'm not confident that it won't impact existing user setups. In pseudo-distributed environments, users often run many processes locally, each with a relatively small heap (e.g., 512MB). In such cases, even 128m could cause their setup to stop working.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "jinhyukify (via GitHub)" <gi...@apache.org>.
jinhyukify commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2139126038
##########
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:
I agreed with your suggestion and updated the code accordingly.
Now the configuration uses an absolute amount for setting the free heap instead of the previous fraction-based approach.
If no configuration is provided, it falls back to the default value of 0.2.
The commit is https://github.com/apache/hbase/pull/7076/commits/3d2b641ef55012997554be85acd2bb9140d7eb73
Not sure what others will think, but feel free to take a look and share your thoughts.
Thanks again for your helpful input!
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHeapMemoryManager.java:
##########
@@ -72,6 +72,7 @@ public void testAutoTunerShouldBeOffWhenMaxMinRangesForMemstoreIsNotGiven() thro
conf.setFloat(MemorySizeUtil.MEMSTORE_SIZE_KEY, 0.02f);
conf.setFloat(HeapMemoryManager.BLOCK_CACHE_SIZE_MAX_RANGE_KEY, 0.75f);
conf.setFloat(HeapMemoryManager.BLOCK_CACHE_SIZE_MIN_RANGE_KEY, 0.03f);
+ conf.setFloat(HConstants.HBASE_REGION_SERVER_FREE_HEAP_MIN_SIZE_KEY, 0.2f);
Review Comment:
I've updated this part as well. Thanks!
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "jinhyukify (via GitHub)" <gi...@apache.org>.
jinhyukify commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2146347709
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -241,6 +238,15 @@ public float getHeapOccupancyPercent() {
: this.heapOccupancyPercent;
}
+ private boolean isHeapMemoryUsageExceedingLimit(float memStoreFraction,
+ float blockCacheFraction) {
+ int memStorePercent = (int) (memStoreFraction * CONVERT_TO_PERCENTAGE);
+ int blockCachePercent = (int) (blockCacheFraction * CONVERT_TO_PERCENTAGE);
+ int minFreeHeapPercent = (int) (this.minFreeHeapFraction * CONVERT_TO_PERCENTAGE);
+
+ return memStorePercent + blockCachePercent + minFreeHeapPercent > CONVERT_TO_PERCENTAGE;
Review Comment:
I also gave this some thought.
At first, I assumed there must be a reason for converting to int before doing the comparison, since it's done that way in some methods in [MemoryUtil](https://github.com/apache/hbase/pull/7076/files#diff-6e3b744afc3d42d8708b76312e06419665aac8132f3a96e8307ba252b2f91e12L88-L94) as well.
Initially, I thought it might be to avoid floating-point precision issues.
However, on closer inspection, I'm not sure if casting to int provides any real advantage - especially since it introduces potential error due to truncation.
In effect, this approach ends up truncating values at the third decimal place, which could lead to subtle changes in behavior for users relying on specific configuration values.
For instance, with memstore=0.409, blockcache=0.4, and freeHeap=0.2, the previous logic wouldn’t have flagged this as exceeding memory limits. But with this change, it might.
What are your thoughts on this? 😄
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2984042121
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 27s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 10s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 15s | | master passed |
| +1 :green_heart: | compile | 1m 16s | | master passed |
| +1 :green_heart: | javadoc | 0m 43s | | master passed |
| +1 :green_heart: | shadedjars | 6m 3s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 13s | | the patch passed |
| +1 :green_heart: | compile | 1m 16s | | the patch passed |
| +1 :green_heart: | javac | 1m 16s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 43s | | the patch passed |
| +1 :green_heart: | shadedjars | 5m 59s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 14s | | hbase-common in the patch passed. |
| -1 :x: | unit | 214m 37s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/10/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 245m 13s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/10/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 2b8e504709dc 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b29e935f262414f48b9210763b50b0fd0f349357 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/10/testReport/ |
| Max. process+thread count | 5358 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/10/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #7076:
URL: https://github.com/apache/hbase/pull/7076#discussion_r2215307521
##########
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:
nit.
```suggestion
* Validates that heap allocations for MemStore and block cache do not exceed the allowed limit,
```
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn merged PR #7076:
URL: https://github.com/apache/hbase/pull/7076
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-3109222210
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 28s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 37s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 31s | | master passed |
| +1 :green_heart: | compile | 4m 10s | | master passed |
| +1 :green_heart: | checkstyle | 0m 53s | | master passed |
| +1 :green_heart: | spotbugs | 2m 11s | | master passed |
| +0 :ok: | refguide | 2m 36s | | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotless | 0m 48s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 7s | | the patch passed |
| +1 :green_heart: | compile | 4m 1s | | the patch passed |
| +1 :green_heart: | javac | 4m 1s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 51s | | the patch passed |
| +1 :green_heart: | xmllint | 0m 1s | | No new issues. |
| +1 :green_heart: | spotbugs | 2m 22s | | the patch passed |
| +0 :ok: | refguide | 2m 11s | | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 12m 20s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 46s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 18s | | The patch does not generate ASF License warnings. |
| | | 49m 14s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/14/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide |
| uname | Linux 14a8b6d2c807 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / e583c2b9e414a76ad16bcee41c9c822d5f9eaada |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/14/yetus-general-check/output/branch-site/book.html |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/14/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/14/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-15625 Make minimum free heap memory percentage configurable [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7076:
URL: https://github.com/apache/hbase/pull/7076#issuecomment-2986682762
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 31s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 31s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 23s | | master passed |
| +1 :green_heart: | compile | 4m 1s | | master passed |
| +1 :green_heart: | checkstyle | 0m 53s | | master passed |
| +1 :green_heart: | spotbugs | 2m 9s | | master passed |
| +0 :ok: | refguide | 2m 32s | | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotless | 0m 47s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 5s | | the patch passed |
| +1 :green_heart: | compile | 3m 58s | | the patch passed |
| +1 :green_heart: | javac | 3m 58s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 50s | | the patch passed |
| +1 :green_heart: | xmllint | 0m 0s | | No new issues. |
| +1 :green_heart: | spotbugs | 2m 17s | | the patch passed |
| +0 :ok: | refguide | 2m 4s | | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 12m 4s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 44s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 17s | | The patch does not generate ASF License warnings. |
| | | 48m 11s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/11/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7076 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide |
| uname | Linux f5016a80af3d 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b29e935f262414f48b9210763b50b0fd0f349357 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/11/yetus-general-check/output/branch-site/book.html |
| refguide | https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-7076/11/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7076/11/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org