ndimiduk commented on code in PR #7726:
URL: https://github.com/apache/hbase/pull/7726#discussion_r2874102262
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/util/MemorySizeUtil.java:
##########
@@ -151,10 +154,29 @@ public static float
getRegionServerMinFreeHeapFraction(final Configuration conf)
/**
* Retrieve global memstore configured size as percentage of total heap.
*/
- public static float getGlobalMemStoreHeapPercent(final Configuration c,
+ public static float getGlobalMemStoreHeapPercent(final Configuration conf,
final boolean logInvalid) {
+ // Check if an explicit memstore size is configured.
+ long memStoreSizeInBytes = getMemstoreSizeInBytes(conf);
Review Comment:
What happens if MEMSTORE_MEMORY_SIZE_KEY is not set. Won't this result in
spurious warnings?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -137,16 +139,20 @@ private boolean doInit(Configuration conf) {
globalMemStorePercentMaxRange =
conf.getFloat(MEMSTORE_SIZE_MAX_RANGE_KEY, globalMemStorePercent);
if (globalMemStorePercent < globalMemStorePercentMinRange) {
- LOG.warn("Setting " + MEMSTORE_SIZE_MIN_RANGE_KEY + " to " +
globalMemStorePercent
- + ", same value as " + MemorySizeUtil.MEMSTORE_SIZE_KEY
- + " because supplied value greater than initial memstore size value.");
+ LOG.warn(
+ "Setting {} to {} (lookup order: {} -> {}), same value as "
Review Comment:
Are you missing '{}' after "same value as " ?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -137,16 +139,20 @@ private boolean doInit(Configuration conf) {
globalMemStorePercentMaxRange =
conf.getFloat(MEMSTORE_SIZE_MAX_RANGE_KEY, globalMemStorePercent);
if (globalMemStorePercent < globalMemStorePercentMinRange) {
- LOG.warn("Setting " + MEMSTORE_SIZE_MIN_RANGE_KEY + " to " +
globalMemStorePercent
- + ", same value as " + MemorySizeUtil.MEMSTORE_SIZE_KEY
- + " because supplied value greater than initial memstore size value.");
+ LOG.warn(
+ "Setting {} to {} (lookup order: {} -> {}), same value as "
+ + " because supplied value less than initial memstore size value.",
Review Comment:
I think you swapped the "greater than / less than" log lines vs. the if
statements.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
