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

JAVA_DIAGNOSTICS for s2i java 11 image uses deprecated and removed JVM options #213

Closed
muff1nman opened this issue Jan 9, 2019 · 7 comments · Fixed by #219
Closed

JAVA_DIAGNOSTICS for s2i java 11 image uses deprecated and removed JVM options #213

muff1nman opened this issue Jan 9, 2019 · 7 comments · Fixed by #219

Comments

@muff1nman
Copy link
Contributor

Options such as PrintGCDateStamps are no longer valid.

@rhuss
Copy link
Contributor

rhuss commented Jan 14, 2019

@vorburger I guess we need to updatre run-java.sh for this, right ? I.e. we probably need a version detection in this startup script.

@vorburger
Copy link
Collaborator

Options such as PrintGCDateStamps are no longer valid.

@muff1nman but that's just a warning, it "works" anyway - or doesn't it for you? Curious.

Would you (@muff1nman) be willing to contribute a PR to https://github.com/fabric8io-images/run-java-sh (perhaps inspired by https://github.com/fabric8io-images/run-java-sh/pull/73/files) which does the trick?

we probably need a version detection in this startup script

yeah I guess, but in run-java-sh? From a (quick) look, it would seem that https://github.com/fabric8io-images//pull/73/files already added a if [ "${JAVA_MAJOR_VERSION:-0}" -ge "10" ]; in some places... perhaps someone could build on top of that? (I'm unlikely to get to that at least this week.)

@rhuss
Copy link
Contributor

rhuss commented Jan 14, 2019

BTW, I just submitted #214 which updates run-java.sh to not calculated defaults when running on Java 11.

@vorburger
Copy link
Collaborator

BTW, I just submitted #214 which updates run-java.sh to not calculated defaults when running on Java 11.

but #214 doesn't skip PrintGCDateStamps (this issue), does it?

@vorburger vorburger changed the title JAVA_DIAGNOSTIC for s2i java 11 image uses deprecated and removed JVM options JAVA_DIAGNOSTICS for s2i java 11 image uses deprecated and removed JVM options Jan 16, 2019
vorburger added a commit to vorburger/s2i that referenced this issue Jan 16, 2019
vorburger added a commit to vorburger/s2i that referenced this issue Jan 16, 2019
@vorburger
Copy link
Collaborator

but #214 doesn't skip PrintGCDateStamps (this issue), does it?

yup, I could (still, so not fixed by #214) reproduce this bug like this.

So we need a if(Java 11) similar to run-java-sh/pull/73/ also here in run-java.sh ...

Who would be willing to have a closer look at which of -XX:NativeMemoryTracking=summary -XX:+PrintGC -XX:+PrintGCDateStamps -XX:+PrintGCTimeStamps -XX:+UnlockDiagnosticVMOptions are NOK for Java 11?

Some of these options perhaps can / need to be replaced with something else now, or (which of them exactly?) should just be skipped, and raise a PR against run-java-sh for it? Then we'll need a new release of https://github.com/fabric8io-images/run-java-sh and a bump to it here (like in #214) and then a rebase #216 which should then pass.

@rhuss
Copy link
Contributor

rhuss commented Jan 16, 2019

Yeah, would be interesting which option combo to use for diagnostics for Java 11. Unfortunately, I'm overloaded, but if someone finds out a good combination and creates a PR for run-java.sh I can do a release and rebase quickly.

@muff1nman
Copy link
Contributor Author

muff1nman commented Jan 16, 2019

I have opened a potential PR, but my biggest concern is that I want to be able to override the -Xlog:gc option. I.e. the following are options that allow me to log gc to a pvc for later viewing:

-XX:+PrintFlagsFinal -Xlog:gc*:file=/logs/gc.log:time:filecount=5,filesize=25M  -XX:NativeMemoryTracking=summary -XX:+UnlockDiagnosticVMOptions -XX:+PrintNMTStatistics -XX:CompressedClassSpaceSize=150m -XX:MaxMetaspaceSize=150m -XX:MetaspaceSize=75m

The options for overriding -Xlog:gc as proposed in #217 are to:

  1. rely on the behavior specified here (https://bugs.openjdk.java.net/browse/JDK-8142952?focusedCommentId=13866870&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-13866870) and move JAVA_OPTIONS to be applied after JAVA_DIAGNOSTICS
  2. keep JAVA_DIAGNOSTICS off and instead specify all options via JAVA_OPTIONS and hope that JAVA_DIAGNOSTICS doesn't drive other valuable logic.
  3. Build logic into JAVA_DIAGNOSTICS so that it omits any duplicate options already specified in JAVA_OPTIONS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants