[GitHub] [incubator-druid] himanshug commented on a change in pull request #7606: Set direct memory if unable to detect JVM config
himanshug commented on a change in pull request #7606: Set direct memory if
unable to detect JVM config
URL: https://github.com/apache/incubator-druid/pull/7606#discussion_r282205588
##
File path:
processing/src/main/java/org/apache/druid/query/DruidProcessingConfig.java
##
@@ -52,7 +52,22 @@ public int intermediateComputeSizeBytes()
return computedBufferSizeBytes.get();
}
-long directSizeBytes =
JvmUtils.getRuntimeInfo().getDirectMemorySizeBytes();
+long directSizeBytes;
+try {
+ directSizeBytes = JvmUtils.getRuntimeInfo().getDirectMemorySizeBytes();
Review comment:
so this always fails for jdk 9 onwards ?
if yes, that means if user wanted buffers of size
DEFAULT_PROCESSING_BUFFER_SIZE_BYTES , they would configure that number but
instead get buffers of size 25%-of-max-heap/totalNumBuffers ?
in that case we need to change line 49 to have a different way of detecting
whether user explicitly provided sizeBytes config .
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] himanshug commented on a change in pull request #7606: Set direct memory if unable to detect JVM config
himanshug commented on a change in pull request #7606: Set direct memory if
unable to detect JVM config
URL: https://github.com/apache/incubator-druid/pull/7606#discussion_r282209983
##
File path:
server/src/main/java/org/apache/druid/guice/DruidProcessingModule.java
##
@@ -157,9 +157,16 @@ private void verifyDirectMemory(DruidProcessingConfig
config)
}
}
catch (UnsupportedOperationException e) {
+ log.debug("Checking for direct memory size is not support on this
platform: %s", e);
log.info(
- "Could not verify that you have enough direct memory, so I hope you
do! Error message was: %s",
- e.getMessage()
+ "Unable to determine max direct memory size. If
-XX:MaxDirectMemorySize is set, make sure "
Review comment:
if above change is made then this should be something like...
"Unable to determine max direct memory size. If
druid.processing.buffer.sizeBytes is explicitly set then
-XX:MaxDirectMemorySize to at least 'druid.processing.buffer.sizeBytes *
(druid.processing.numMergeBuffers[%,d] + druid.processing.numThreads[%,d] + 1)'
or else to at least 25% of maximum heap size."
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] himanshug commented on a change in pull request #7606: Set direct memory if unable to detect JVM config
himanshug commented on a change in pull request #7606: Set direct memory if
unable to detect JVM config
URL: https://github.com/apache/incubator-druid/pull/7606#discussion_r282610012
##
File path:
processing/src/main/java/org/apache/druid/query/DruidProcessingConfig.java
##
@@ -52,7 +52,22 @@ public int intermediateComputeSizeBytes()
return computedBufferSizeBytes.get();
}
-long directSizeBytes =
JvmUtils.getRuntimeInfo().getDirectMemorySizeBytes();
+long directSizeBytes;
+try {
+ directSizeBytes = JvmUtils.getRuntimeInfo().getDirectMemorySizeBytes();
Review comment:
duh, I thought DEFAULT_PROCESSING_BUFFER_SIZE_BYTES was 1G but it got
changed at some point I think. I should've checked.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] himanshug commented on a change in pull request #7606: Set direct memory if unable to detect JVM config
himanshug commented on a change in pull request #7606: Set direct memory if
unable to detect JVM config
URL: https://github.com/apache/incubator-druid/pull/7606#discussion_r282209983
##
File path:
server/src/main/java/org/apache/druid/guice/DruidProcessingModule.java
##
@@ -157,9 +157,16 @@ private void verifyDirectMemory(DruidProcessingConfig
config)
}
}
catch (UnsupportedOperationException e) {
+ log.debug("Checking for direct memory size is not support on this
platform: %s", e);
log.info(
- "Could not verify that you have enough direct memory, so I hope you
do! Error message was: %s",
- e.getMessage()
+ "Unable to determine max direct memory size. If
-XX:MaxDirectMemorySize is set, make sure "
Review comment:
if above change is made then this should be something like...
"Unable to determine max direct memory size. If
druid.processing.buffer.sizeBytes is explicitly set then
-XX:MaxDirectMemorySize to at least 'druid.processing.buffer.sizeBytes *
(druid.processing.numMergeBuffers[%,d] + druid.processing.numThreads[%,d] + 1)'
or else to at least 25% of maximum heap size."
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] himanshug commented on a change in pull request #7606: Set direct memory if unable to detect JVM config
himanshug commented on a change in pull request #7606: Set direct memory if
unable to detect JVM config
URL: https://github.com/apache/incubator-druid/pull/7606#discussion_r282611829
##
File path:
server/src/main/java/org/apache/druid/guice/DruidProcessingModule.java
##
@@ -157,9 +157,16 @@ private void verifyDirectMemory(DruidProcessingConfig
config)
}
}
catch (UnsupportedOperationException e) {
+ log.debug("Checking for direct memory size is not support on this
platform: %s", e);
log.info(
- "Could not verify that you have enough direct memory, so I hope you
do! Error message was: %s",
- e.getMessage()
+ "Unable to determine max direct memory size. If
-XX:MaxDirectMemorySize is set, make sure "
+ + "a) -XX:MaxDirectMemorySize is set to at least [%,d] bytes (25%%
of maximum heap size), or "
+ + "b) set druid.processing.buffer.sizeBytes, such that "
+ + "druid.processing.buffer.sizeBytes *
(druid.processing.numMergeBuffers[%,d] + druid.processing.numThreads[%,d] + 1) "
+ + "does not exceed the value of -XX:MaxDirectMemorySize",
Review comment:
optional nit: this is still confusing for me, maybe it could be reworded as
```suggestion
Unable to determine max direct memory size. If
druid.processing.buffer.sizeBytes is explicitly set then make sure to set
-XX:MaxDirectMemorySize to at least "druid.processing.buffer.sizeBytes *
(druid.processing.numMergeBuffers[%,d] + druid.processing.numThreads[%,d] + 1)"
or else set it to at least 25% of maximum jvm heap size.
```
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
