[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