nextgenhealthcare / connect

The swiss army knife of healthcare integration.
Other
905 stars 273 forks source link

[IDEA] Identify JVM and JVM parameters on start and DON'T start if some conflict exists. #5397

Open pacmano1 opened 2 years ago

pacmano1 commented 2 years ago

Is your feature request related to a problem? Please describe. Users install Mirth and expeect it to run without configuring things like java 9+ options.

Describe your use case Based on number of people that don't read the manual to see java 9+ instuctions and/or other related problem to the choice of JVM version and/or options, it seems that Mirth should just detect its JVM and parameters and not start if there is a conflict.

Describe the solution you'd like On start, identify conflicting JVM options. Log to mirth.log problems and don't start. Also, it's probably time to have java 9+ options installed by default.

Describe alternatives you've considered Putting a critical readme like java 9+ front and formost in the documentation, it is buried.

Additional context None really, just seems like an easy thing to do to reduce support burdens.

jonbartels commented 2 years ago

Examples: https://github.com/nextgenhealthcare/connect/discussions/5385 https://github.com/nextgenhealthcare/connect/discussions/5392 https://mirthconnect.slack.com/archives/C02SW0K4D/p1662054231865699?thread_ts=1662053792.097549&cid=C02SW0K4D

This manifests as exceptions like:

Unable to make field private static final long java.util.Collections$EmptySet.serialVersionUID accessible: module java.base does not "opens java.util" to unnamed module @1f2781cf

The ideal solution would:

I believe this solution can actually be implemented as a ServicePlugin whose start method can inspect the vmoptions and Java version. Implementing this as a core plugin should speed development because you don't need to alter core Mirth classes. Implementing this as a core plugin would also allow it to be optionally added to existing installations as a safeguard.

ChristopherSchultz commented 1 year ago

Can we just change this issue to "require Java 11/17/whatever+" and finally move-on from Java 8?

pacmano1 commented 1 year ago

Until some new thing comes along that requires yet another required modification, so "sorta".

pacmano1 commented 1 year ago

Perhaps mcservice and mcserver (the startup scripts) can just handle this automagically.

e.g.:


#!/bin/bash
​
# Get Java version
java_version=$(java -version 2>&1 | head -1 | cut -d'"' -f2 | sed '/^1\./s///' | cut -d'.' -f1)
echo $java_version
​
# Set JVM arguments based on Java version
if [[ "$java_version" -ge 11 ]]; then
  jvm_extra_args="--add-modules java.xml.bind ......"
fi
jonbartels commented 1 year ago

I would refine @pacmano1 idea to ONLY alter the modules for Java versions and NOT xmx or other vmopts. I would perhaps refine it as if java_ver == 11 then load mc.java.11.vmopts and prepend (maybe append? depends on load order) the specific options for that JVM version.

This has to be controllable by install4j. The mcserver script is non-trivial. Much of it is doing some sort of jvm existence/version testing. There is a specific create_db_entry function that checks the JVM version.

This appears to be the feature - https://www.ej-technologies.com/resources/install4j/help/doc/concepts/vmParameters.html

ChristopherSchultz commented 1 year ago

You would not ask such questions if you looked at the Install4j script that launches Mirth. It appears install4j supports version-specific JVM launch parameters. 😮

I'm a little curious why they use install4j at all. It's just not that hard to look for JAVA_HOME is some obvious places and bail if it's not found. Building a classpath from a few directories of JAR files is also not hard.