Skip to content
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

Add heap size bootstrap check #17728

Merged
merged 1 commit into from
Apr 13, 2016
Merged

Add heap size bootstrap check #17728

merged 1 commit into from
Apr 13, 2016

Conversation

jasontedor
Copy link
Member

This commit adds a bootstrap check to ensure that the initial heap size
and max heap size are set equal to each other.

Closes #17490

try {
Object maxHeapSizeVmOption = vmOptionMethod.invoke(hotSpotDiagnosticMXBean, "MaxHeapSize");
info.configuredMaxHeapSize = Long.parseLong((String) valueMethod.invoke(maxHeapSizeVmOption));
} catch (Exception ignored) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think maybe we should WARN or something if this happens?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nik9000 By the time we get that far in that block, nothing should happen there and those try/catch blocks are defensive more than anything. The expected way to see an exception out of that entire block is if the end user is not on the HotSpot VM or com.sun.management.HotSpotDiagnosticMXBean is otherwise unavailable.

I do not think that we should log in that case as there is nothing the user can actively do other than switch to HotSpot. But we don't support non-HotSpot. 😄

What I do think that we should do though is write a test that tests that these methods do work on HotSpot because otherwise they can silently be broken and we will never no. I will take that on.

@nik9000
Copy link
Member

nik9000 commented Apr 13, 2016

Seems fine with me. Left non-critical comments.

This commit adds a bootstrap check to ensure that the initial heap size
and max heap size are set equal to each other.
@jasontedor jasontedor merged commit 11648cb into elastic:master Apr 13, 2016
@jasontedor jasontedor deleted the heap-size branch April 13, 2016 19:11
@clintongormley clintongormley added the :Core/Infra/Core Core issues without another label label May 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants