fabric8io-images / s2i

OpenShift S2I images for Java and Karaf applications
Apache License 2.0
70 stars 84 forks source link

adapt run-java-sh to adopt the auto-generation options for Java 11 #191

Open vorburger opened 5 years ago

vorburger commented 5 years ago

@rhuss said in #160 that, quote:

major work will be to adapt https://github.com/fabric8io-images/run-java-sh to adopt the auto-generation options for the JDK. E.g. it would need a Java 8 / Java 11 dynamic detection (or taken from an env var) and use only the self-calculated cgroup based limits if not running on Java 11. Also, it would be awesome to update the fine-tuned options for Java 11 as well. A similar issue is open over there btw --> https://issues.jboss.org/browse/CLOUD-2839

But somewhat to my surprise, I seem to actually have been able to get a (basic) Java 11 working without touching run-java-sh ... I therefore propose that we work incrementally, and get #160 wrapped up and merged, and then iterate to improve things re. the point raised above under this issue.

gunnarmorling commented 5 years ago

I don't think it's too surprising that run-java.sh still works, but some of the things it's doing (like calculating default memory settings) shouldn't be needed any more with Java 11.

vorburger commented 5 years ago

https://github.com/fabric8io-images/run-java-sh/blob/master/fish-pepper/run-java-sh/fp-files/run-java.sh

https://github.com/fabric8io-images/s2i/blob/master/java/fish-pepper.yml#L17

derkoe commented 5 years ago

Already created a PR for this: https://github.com/fabric8io-images/run-java-sh/pull/73 which is still not merged.

rhuss commented 5 years ago

213 is a case where Java 11 breaks with the current script.

rhuss commented 5 years ago

214 is the PR which includes the update to run-java with the fix for Java 11 included.

vorburger commented 5 years ago

@rhuss not sure if there is anything left to do here? (Other than perhaps #213.)

rhuss commented 5 years ago

Not sure either ;) I think if we have #213 done, we can close this issue, too.

jerboaa commented 5 years ago

With JDK-8146115 all of the size detection should be done automatically by the JVM. That includes JDK 8 (u191) and JDK 11. The relevant options are -Xlog:os+container=trace for JDK 11 and -XX:+UnlockDiagnosticVMOptions -XX:+PrintContainerInfo on JDK 8.

I cannot comment on other options those scripts use.

rhuss commented 5 years ago

@jerboaa thanks for the feedback. Actually, we should update run-java.sh either to detect also whether a Java 8 JDK supports the cgroup awareness automatically (and switch off the homegrown calculation in this case), or add an extra flag/env var to indicate that this is not needed anymore. The latter is probably is simpler (and then could be reused here when we are using the proper base image), but we might strive for the former, too.

Regardless what, works needs to be done over there in ghttps://github.com/fabric8io-images/run-java-sh