-
Notifications
You must be signed in to change notification settings - Fork 25k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't drop user's MaxDirectMemorySize flag on jdk8/windows #48657
Don't drop user's MaxDirectMemorySize flag on jdk8/windows #48657
Conversation
We had been testing whether a user had passed a value for MaxDirectMemorySize by parsing the output of "java -XX:PrintFlagsFinal -version". If MaxDirectMemorySize equals zero, we set it to half of max heap. The problem is that on Windows with JDK 8, a JDK bug incorrectly truncates values over 4g and returns multiples of 4g as zero. In order to always respect the user-defined settings, we need to check our input to see if an "-XX:MaxDirectMemorySize" value has been passed.
Pinging @elastic/es-core-infra (:Core/Infra/Core) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I understand the problem in #44174, this PR still won't solve it. If a heap size of e.g. 8GB is specified and the user sets -XX:MaxDirectMemorySize=4GB
(or Elasticsearch ergonomically chooses 4GB), Elasticsearch would still behave as specified in #44174, wouldn't it? The only difference is that we will now correctly pass whatever the user has specified in -XX:MaxDirectMemorySize
. However, as they have specified a value, they won't see the new warning message. Maybe we should show the warning message on Windows / JDK 8 always when maxDirectMemorySize
is zero?
@danielmitterdorfer Thanks for the review. On the current 7.x branch, with the following in
We get a failure:
But with the changes on this PR, we get a startup with no JDK issue warning. JDK 8 is able to read a setting like The unit test that I modified covers this behavior in a way, but it's not very clear. So I'll update that for clarity. Finally, I agree with you that we should print some kind of warning even if the user is passing in a setting, because it would be good for users who are able to upgrade their JDK anyway. So I will adjust this a little bit. I don't want to instruct users to set the MaxDirectMemorySize when they've already done so; I think that would be confusing. |
Even if a user has set MaxDirectMemorySize, they aren't future-proof for this JDK bug. With this change, we issue a general warning for the windows/JDK8 issue, and a specific warning if MaxDirectMemorySize is unset.
Now, when a Windows/JDK8 user fails to set MaxDirectMemorySize, they will see:
When they do set MaxDirectMemorySize, they'll see, e.g.:
…and so forth, on the way to a successful startup. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for iterating! LGTM
…8657) * Always pass user-specified MaxDirectMemorySize We had been testing whether a user had passed a value for MaxDirectMemorySize by parsing the output of "java -XX:PrintFlagsFinal -version". If MaxDirectMemorySize equals zero, we set it to half of max heap. The problem is that on Windows with JDK 8, a JDK bug incorrectly truncates values over 4g and returns multiples of 4g as zero. In order to always respect the user-defined settings, we need to check our input to see if an "-XX:MaxDirectMemorySize" value has been passed. * Always warn for Windows/jdk8 ergo issue Even if a user has set MaxDirectMemorySize, they aren't future-proof for this JDK bug. With this change, we issue a general warning for the windows/JDK8 issue, and a specific warning if MaxDirectMemorySize is unset.
…8657) * Always pass user-specified MaxDirectMemorySize We had been testing whether a user had passed a value for MaxDirectMemorySize by parsing the output of "java -XX:PrintFlagsFinal -version". If MaxDirectMemorySize equals zero, we set it to half of max heap. The problem is that on Windows with JDK 8, a JDK bug incorrectly truncates values over 4g and returns multiples of 4g as zero. In order to always respect the user-defined settings, we need to check our input to see if an "-XX:MaxDirectMemorySize" value has been passed. * Always warn for Windows/jdk8 ergo issue Even if a user has set MaxDirectMemorySize, they aren't future-proof for this JDK bug. With this change, we issue a general warning for the windows/JDK8 issue, and a specific warning if MaxDirectMemorySize is unset.
* Warn when MaxDirectMemorySize may be incorrect (Windows/JDK8 only issue) (#48365) Our JVM ergonomics extract max heap size from JDK PrintFlagsFinal output. On JDK 8, there is a system-dependent bug where memory sizes are cast to 32-bit integers. On affected systems (namely, Windows), when 1/4 of physical memory is more than the maximum integer value, the output of PrintFlagsFinal will be inaccurate. In the pathological case, where the max heap size would be a multiple of 4g, the test will fail. The practical effect of this bug, beyond test failures, is that we may set MaxDirectMemorySize to an incorrect value on Windows. This commit adds a warning about this situation during startup. * Don't drop user's MaxDirectMemorySize flag on jdk8/windows (#48657) * Always pass user-specified MaxDirectMemorySize We had been testing whether a user had passed a value for MaxDirectMemorySize by parsing the output of "java -XX:PrintFlagsFinal -version". If MaxDirectMemorySize equals zero, we set it to half of max heap. The problem is that on Windows with JDK 8, a JDK bug incorrectly truncates values over 4g and returns multiples of 4g as zero. In order to always respect the user-defined settings, we need to check our input to see if an "-XX:MaxDirectMemorySize" value has been passed. * Always warn for Windows/jdk8 ergo issue Even if a user has set MaxDirectMemorySize, they aren't future-proof for this JDK bug. With this change, we issue a general warning for the windows/JDK8 issue, and a specific warning if MaxDirectMemorySize is unset.
…8/Windows (#49043) * Warn when MaxDirectMemorySize may be incorrect (Windows/JDK8 only issue) (#48365) Our JVM ergonomics extract max heap size from JDK PrintFlagsFinal output. On JDK 8, there is a system-dependent bug where memory sizes are cast to 32-bit integers. On affected systems (namely, Windows), when 1/4 of physical memory is more than the maximum integer value, the output of PrintFlagsFinal will be inaccurate. In the pathological case, where the max heap size would be a multiple of 4g, the test will fail. The practical effect of this bug, beyond test failures, is that we may set MaxDirectMemorySize to an incorrect value on Windows. This commit adds a warning about this situation during startup. On 7.4, we also warn for io.netty.allocator.type. * Don't drop user's MaxDirectMemorySize flag on jdk8/windows (#48657) * Always pass user-specified MaxDirectMemorySize We had been testing whether a user had passed a value for MaxDirectMemorySize by parsing the output of "java -XX:PrintFlagsFinal -version". If MaxDirectMemorySize equals zero, we set it to half of max heap. The problem is that on Windows with JDK 8, a JDK bug incorrectly truncates values over 4g and returns multiples of 4g as zero. In order to always respect the user-defined settings, we need to check our input to see if an "-XX:MaxDirectMemorySize" value has been passed. * Always warn for Windows/jdk8 ergo issue Even if a user has set MaxDirectMemorySize, they aren't future-proof for this JDK bug. With this change, we issue a general warning for the windows/JDK8 issue, and a specific warning if MaxDirectMemorySize is unset. * Adjust warning for io.netty.allocator.type
We had been testing whether a user had passed a value for MaxDirectMemorySize by parsing the output of "java -XX:PrintFlagsFinal -version". If MaxDirectMemorySize equals zero, we set it to half of max heap. The problem is that on Windows with JDK 8, a JDK bug incorrectly truncates values over 4g and returns multiples of 4g as zero. In order to always respect the user-defined settings, we need to check our input to see if an "-XX:MaxDirectMemorySize" value has been passed.
It may seem that the logic here could be slightly tighter: we could check for the MaxDirectMemorySize flag only on Windows/JDK8. But in practice, I believe that on all other systems,
maxDirectMemorySize == 0
will only be true when a user sets MaxDirectMemorySize to 0 (or to a value greater than 8 exbibytes ((2^63)), which I believe not a presently realistic value).Fixes #44174
Relates #48365