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

Access to JVM PID in universal package #168

Closed
kodemaniak opened this issue Feb 14, 2014 · 12 comments
Closed

Access to JVM PID in universal package #168

kodemaniak opened this issue Feb 14, 2014 · 12 comments

Comments

@kodemaniak
Copy link
Contributor

I am using the universal packager to build a zip for a backend service. Upon deployment I want to stop the old process before starting the new one. Unfortunately the script does not exec the java process, so the PID I can observe points to the script and does not terminate the JVM. Is there any workaround for that or can the script modified to exec the JVM?

@jsuereth
Copy link
Member

Huh, look at #99. Can you explain what we broke with that fix?

@kodemaniak
Copy link
Contributor Author

Not that I am a guru on this, but AFAIK the exec results in the original process (the script) being replaced with the called process (the JVM), meaning that the PID in $! actually is the PID of the JVM. I write that PID to a file and use it to kill the service later, when I want to start a new version. Currently I get the PID of the start script, and when I kill the script, the JVM continues to run.

Maybe there other ways to achieve the same. It could also be an option to modify the script generation for java_server packages such that start/stop can be provided to the script and the script handles the PID.

@muuki88
Copy link
Contributor

muuki88 commented Feb 15, 2014

@kardapoltsev proposal looked like this:

${{exec_cmd}} "$java_cmd" 

And in bash script replacements smth like this:

replacements ++= Seq("exec_cmd" -> (if(fork) "exec" else ""))

However we only removed the exec in #99 . I didn't experienced any issues with this yet.

@kodemaniak , can you post a minimal example for this?

@muuki88
Copy link
Contributor

muuki88 commented Feb 15, 2014

I made some tests with following results.

Execution Command Startscript Result
./bin/mukis-test "$@" PIDs equal, kill process works
./bin/mukis-test exec "$@" PIDs equal, kill process doesn't work
./bin/mukis-test & "$@" PIDs differ
./bin/mukis-test & exec "$@" PIDs equal, kill process works
  • play 2.2.0
  • current sbt-native-packager

@kardapoltsev
Copy link
Member

I've done the same tests but got different results. And from man page of exec I realize that it should be added back. But I need to test init.d script before.

If exec is specified with command, it shall replace the shell with command without creating a new process.

@muuki88 Did you use pid file created by Play application in your tests?

@kardapoltsev
Copy link
Member

I've tested systemV start scripts with exec $java_cmd and now it works fine.

@muuki88
Copy link
Contributor

muuki88 commented Feb 16, 2014

Yes, play always creates its own PID. So I checked if the PID play outputs and the one thats running are the same. I didnt test it with a bootsystem, just on commandline.

@kodemaniak
Copy link
Contributor Author

I assume, you don't need an example anymore? I guess what I am doing/experiencing is the same that @kardapoltsev tested.

@muuki88
Copy link
Contributor

muuki88 commented Feb 17, 2014

I will test it wil Upstart and SystemV again (just to make sure, that a commandline call is different than a call from the bootsystem).

Is an option, like @kardapoltsev proposed in #99, neccessary or just adding it should be fine?

@kodemaniak
Copy link
Contributor Author

I don't need the option, but if exec is problematic in some scenarios making it configurable is probably the best solution.

@kardapoltsev
Copy link
Member

I was totally wrong when asking for removing exec. Now I've tested system V start script, bash script and read man exec. I think we should put in back.

@muuki88
Copy link
Contributor

muuki88 commented Feb 18, 2014

I think we're all in some kind of learning process here :P
We'll read it and I think we should document this and provide an alternative way (IMHO overriding the start-script-template) to do otherwise.

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

No branches or pull requests

4 participants