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

Info on compressed ordinary object pointers #15489

Merged
merged 1 commit into from
Dec 16, 2015
Merged

Info on compressed ordinary object pointers #15489

merged 1 commit into from
Dec 16, 2015

Conversation

jasontedor
Copy link
Member

This commit adds to JvmInfo the status of whether or not compressed
ordinary object pointers are enabled. Additionally, logging of the max
heap size and the status of the compressed ordinary object pointers flag
are provided on startup.

Relates #13187, relates elastic/elasticsearch-definitive-guide#455

@jasontedor
Copy link
Member Author

This provides log messages on startup that look like

[2015-12-16 13:53:33,417][INFO ][env                      ] [Illyana Rasputin] heap size [989.8mb], compressed ordinary object pointers [true]

and

[2015-12-16 13:54:25,051][INFO ][env                      ] [Everyman] heap size [31.9gb], compressed ordinary object pointers [false]

and provides a using_compressed_ordinary_object_pointers field in the jvm field of the nodes info API response.

@polyfractal
Copy link
Contributor

Woo! The log message LGTM, but I don't think I'm qualified to comment on the JvmInfo code itself :)

Should this be added to the /_cat/nodes output too as an optional column? Is it possible to backport this to the 2.x series, or something technical which disallows it?

VMOption usingCompressedOopsVmOption = hotSpotDiagnosticMXBean.getVMOption("UseCompressedOops");
info.usingCompressedOops = Boolean.parseBoolean(usingCompressedOopsVmOption.getValue());
} catch (Throwable t) {
info.usingCompressedOops = false;
Copy link
Member

Choose a reason for hiding this comment

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

false or "unknown"?

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 I considered that and I don't mind going either way.

Copy link
Member

Choose a reason for hiding this comment

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

I think I'd prefer if it were just a string/enum/whatever that made the logs and api spit out "true"/"false"/"unknown".

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 Done in c20d5e41b12105045ee19834bc3b18ea906dddd3.

@jasontedor
Copy link
Member Author

Should this be added to the /_cat/nodes output too as an optional column?

@polyfractal I considered that and I ultimately decided against; the reason that I decided against is because nearly all of the information that is in JvmInfo is not provided in the cat nodes API. That's just my thinking; I'm certainly open to other thoughts.

Is it possible to backport this to the 2.x series, or something technical which disallows it?

It's possible.

@polyfractal
Copy link
Contributor

I considered that and I ultimately decided against; the reason that I decided against is because nearly all of the information that is in JvmInfo is not provided in the cat nodes API.

Makes sense. Someone would only need to check this once, and you'd never need to monitor it dynamically... no need to clutter up the cat endpoint with it :)

@rmuir
Copy link
Contributor

rmuir commented Dec 16, 2015

Is reflection on this API really allowed in java 9? We should not add this if not.

@jasontedor
Copy link
Member Author

Is reflection on this API really allowed in java 9? We should not add this if not.

@rmuir Compiled with the 9-ea+96-2015-12-10-032020.javare.4030.nc compiler, and run on that JVM, it's still available and works.

                "using_compressed_ordinary_object_pointers": true,
                "version": "9-ea",
                "vm_name": "Java HotSpot(TM) 64-Bit Server VM",
                "vm_vendor": "Oracle Corporation",
                "vm_version": "9-ea+96-2015-12-10-032020.javare.4030.nc"

@nik9000
Copy link
Member

nik9000 commented Dec 16, 2015

LGTM if @rmuir is ok with it.

@rmuir
Copy link
Contributor

rmuir commented Dec 16, 2015

that is not a jigsaw build.

@jasontedor
Copy link
Member Author

that is not a jigsaw build.

@rmuir Oops, I had JAVA_HOME set to the wrong build; thanks for catching that. That said, it works against the latest Jigsaw build 9-ea+96-jigsaw-nightly-h4080-20151215:

                "using_compressed_ordinary_object_pointers": true,
                "version": "9-ea",
                "vm_name": "Java HotSpot(TM) 64-Bit Server VM",
                "vm_vendor": "Oracle Corporation",
                "vm_version": "9-ea+96-jigsaw-nightly-h4080-20151215"

@rmuir
Copy link
Contributor

rmuir commented Dec 16, 2015

OK, looks good, thanks for checking, like the other management apis we do this trick for, it is a public api (just not guaranteed in all jvms) and the reflection is correct.

This commit adds to JvmInfo the status of whether or not compressed
ordinary object pointers are enabled. Additionally, logging of the max
heap size and the status of the compressed ordinary object pointers flag
are provided on startup.

Relates #13187, relates elastic/elasticsearch-definitive-guide#455
jasontedor added a commit that referenced this pull request Dec 16, 2015
Info on compressed ordinary object pointers
@jasontedor jasontedor merged commit cfc46da into elastic:master Dec 16, 2015
@jasontedor jasontedor deleted the compressed-oops branch December 16, 2015 22:00
@jasontedor
Copy link
Member Author

Thanks for reviewing @nik9000, @rmuir, and @polyfractal.

jasontedor added a commit that referenced this pull request Dec 16, 2015
This commit backports commits cfc46da
and 12e241f from master to 2.x.

Relates #15489
ByteSizeValue maxHeapSize = JvmInfo.jvmInfo().getMem().getHeapMax();
Boolean usingCompressedOops = JvmInfo.jvmInfo().usingCompressedOops();
String usingCompressedOopsStatus = usingCompressedOops == null ? "unknown" : Boolean.toString(usingCompressedOops);
logger.info("heap size [{}], compressed ordinary object pointers [{}]", maxHeapSize, usingCompressedOopsStatus);
Copy link
Member

Choose a reason for hiding this comment

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

Sorry I am late, but I think this should go under debug logging, as by default, this will be logged each time a node starts, and I don't think it is important enough to log each time a node starts...

Copy link
Member

Choose a reason for hiding this comment

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

mmm, maybe heap size I guess? was that the aim here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry I am late, but I think this should go under debug logging, as by default, this will be logged each time a node starts, and I don't think it is important enough to log each time a node starts...

@kimchy We log mount points, disk space metrics, whether or not the disks are maybe spinning and the underlying file system types at the info level; I opted for consistency with that.

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.

5 participants