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

Show message if JVM args are present but new JVM is spawned based on active modules #6476

Closed
jnehlmeier opened this issue Jun 28, 2021 · 7 comments · Fixed by #6597 or #6598
Closed

Comments

@jnehlmeier
Copy link

Target Jetty version(s)

10.0+

Enhancement Description

We recently upgraded to Jetty 10 and consequently used some more out-of-the-box modules that ship with jetty. One of the modules used an [exec] block which causes the creating of a new JVM.

It would be nice if some message will be printed (info or warn) saying that a new JVM will be created. With our old configuration (Jetty 9.x) we used to start Jetty with java $JAVA_OPTIONS -jar $JETTY_HOME/start.jar with all kinds of java parameters in $JAVA_OPTIONS. This stopped working with our new configuration in Jetty 10, but it wasn't immediately obvious why it stopped working.

Ideally if you detect that Jetty has been launched with JVM args, e.g. java --add-opens <module> -Xmx 2GB -jar ./start.jar and with exec mode being active, then log a warning that JVM parameters will not have any effect for the newly spawned JVM. Alternatively you might be able to transparently use the provided JVM args for the newly spawned JVM as well. To get the JVM args you could use ManagementFactory.getRuntimeMXBean().getInputArguments().

@sbordet
Copy link
Contributor

sbordet commented Aug 5, 2021

Thanks for the suggestion, it is indeed useful.

Transparently copying the first JVM parameters to the spawned JVM might work, but the main concern I have is that for large heaps sizes, a large heap is allocated for the first JVM that does nothing.

So perhaps a WARN message is better.
@joakime opinions?

@gregw
Copy link
Contributor

gregw commented Aug 7, 2021

The other option is to do what we do with the docker setup for jetty: use start.jar with --dry-run to generate the start line that you then run directly without keeping the first JVM running. Hmmmm perhaps we should document how to do that...

Something like java $(java -jar start.jar --dry-run)

@jnehlmeier
Copy link
Author

@gregw The whole issue is that when using jetty provided modules it is unclear wether or not they use [exec] from one release to another. Basically you have to know how modules are implemented in order to launch jetty correctly.

With Jetty 10 we now have a custom my-jvm.mod file in our docker container which contains

[exec]
# static properties
systemProperty1=${ENV_VARIABLE_1}
....
# optional additions
${ENV_JAVA_OPTIONS}

and then launch jetty directly using --dry-run while making sure parameter expansion works so we can use env variables in docker.

I think shipping a shell script for windows / linux with jetty could be valuable. That way the script could always use the --dry-run mechanism to account for possible active [exec] modules and the script can do the required steps to apply JVM / Jetty parameters correctly.

For example ./jetty.sh --jvm-params="${JAVA_OPTIONS} -Dprop=value" <start.jar params>

@joakime
Copy link
Contributor

joakime commented Aug 8, 2021

@jnehlmeier the --list-config output should show if exec will happen or not.

@gregw
Copy link
Contributor

gregw commented Aug 8, 2021

@jnehlmeier by "launch directly using --dry-run do you mean something like: java $(java -jar start.jar --dry-run)?
What are the steps you need to take to "making sure parameter expansion works"? An example would be interesting.

If you are doing the --dry-run style startup, then there is no need to know if a module has exec or not.

Also, you say you are using docker, so why not use our docker-jetty images that manage doing the --dry-run when building the image rather than at start time ? or at least have a look at how they work and copy that approach. Doing as much resolution as possible at image build time is good for images that need to start quickly.

We actually do have a shell script in jetty-home... but it doesn't do the --dry-run step (probably should and we should fix for this issue), but then writing a portable shell script is actually more difficult than start.jar!

@jnehlmeier
Copy link
Author

@jnehlmeier by "launch directly using --dry-run do you mean something like: java $(java -jar start.jar --dry-run)?
What are the steps you need to take to "making sure parameter expansion works"? An example would be interesting.

Our jetty 9 image did not use --dry-run since we never needed it. But now with Jetty 10 we are using it because of exec. However given we have environment variables placed in a custom jvm.mod (see above) these need to be expanded. Something like $(java -jar start.jar --dry-run) does not work, as it does not expand the environment variables in the command line that --dry-run produces.

So we had to do:

# entrypoint.sh
#
# store the generated command which still contains ${ENV_VARIABLE} parameters 
CMD=`java -jar start.jar --dry-run`
# apply parameter expansion so that env variables are now replaced with their content
CMD=$(eval "echo $CMD")
# start jetty
exec $CMD

If you are doing the --dry-run style startup, then there is no need to know if a module has exec or not.

Yes, but figuring out that we now need --dry-run because we applied some jetty provided modules did cost some time. Thats why I opened this issue. Now that we have a docker image that uses --dry-run we probably never have to change it again.

Also, you say you are using docker, so why not use our docker-jetty images that manage doing the --dry-run when building the image rather than at start time ? or at least have a look at how they work and copy that approach. Doing as much resolution as possible at image build time is good for images that need to start quickly.

Interesting. Yes, currently we do the --dry-run thing in our docker entrypoint script since we just wanted to get it fixed. Will take a look how to best do it at build time.

but then writing a portable shell script is actually more difficult than start.jar!

Thats true.

@joakime
Copy link
Contributor

joakime commented Aug 9, 2021

Here's the docker images: https://hub.docker.com/_/jetty
And the source repository for those images: https://github.com/eclipse/jetty.docker

sbordet added a commit that referenced this issue Aug 10, 2021
…wned

* Added WARN message.
* Updated documentation.
* Added test case.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet linked a pull request Aug 10, 2021 that will close this issue
@gregw gregw linked a pull request Aug 11, 2021 that will close this issue
sbordet added a commit that referenced this issue Aug 11, 2021
…wned

* Updated --list-config to report whether a JVM is forked.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue Aug 11, 2021
…wned

Updates after review: reduced the WARN message lines.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue Aug 13, 2021
Fixes #6476 - Show message if JVM args are present but new JVM is spawned

* Improved documentation by correctly redacting out `jetty-halt.xml`,
an XML file that is only necessary for rendering the documentation.
* Added WARN message when new JVM is spawned.
* Updated documentation.
* Updated --list-config to report whether a JVM is forked.
* Added test case.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue Aug 13, 2021
* Fix #6597 Use dry run in jetty.sh

Use a --dry-run in jetty.sh to pre-expand the java arguments and thus avoid having two JVMs running in the case of exec.

Also made a small change to allow script to check the current directory for JETTY_BASE, as that allows testing and runs in the same style as direct calls to start.jar

Signed-off-by: Greg Wilkins <gregw@webtide.com>
Co-authored-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet added this to To do in Jetty 10.0.7/11.0.7 FROZEN via automation Aug 13, 2021
@sbordet sbordet moved this from To do to Done in Jetty 10.0.7/11.0.7 FROZEN Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
4 participants