[jitsi-dev] [jitsi-videobridge] Use exec in wrapper scripts for launching java (#100)


#1

When launching Java from a wrapper script, it's usually better to use
exec, so that the extra bash process isn't hanging around. It also helps
shutdown and process monitoring (like with upstart, systemd, docker,
etc.) since they would be monitoring (and killing) the Java process
itself, instead of the wrapper script.
You can view, comment on, or merge this pull request online at:

  https://github.com/jitsi/jitsi-videobridge/pull/100

-- Commit Summary --

  * Use exec in wrapper scripts for launching java

-- File Changes --

    M resources/install/linux-64/jvb.sh (2)
    M resources/install/linux/jvb.sh (2)
    M resources/install/macosx/jvb.sh (2)

-- Patch Links --

https://github.com/jitsi/jitsi-videobridge/pull/100.patch
https://github.com/jitsi/jitsi-videobridge/pull/100.diff

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/100


#2

I think that's a nice fix, but we need to make sure that it doesn't brake anything. [This script](https://github.com/jitsi/jitsi-videobridge/blob/master/resources/collect-dump-logs.sh) will need an update. @damencho WDYT?

@leedm777 Before we merge any of your contributions we need you to sign our contributor agreement ([individual](https://jitsi.org/icla) or [corporate](https://jitsi.org/ccla)). Apologies if you've already done this.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/100#issuecomment-159182331


#3

@bgrozev I'm not sure what needs to change in `collect-dump-logs.sh`. Can you be more specific?

I think my company (Digium) already has a corporate CLA on file. Can you check to see if that would cover my contribution?

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/100#issuecomment-159324073


#4

You do need to sign the CLA again (as the currently signed one doesn't include your name).

For the script, we need to remove PROC_PID and just use PID instead. @damencho just pointed out that we will need to do the same in the [init script](https://github.com/leedm777/jitsi-videobridge/blob/master/resources/install/debian/init.d)

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/100#issuecomment-159368577


#5

@bgrozev I've fixed the init and collect dump logs scripts.

I'm a little unsure about the changes to the init script; especially the status option. Do you have a setup you can test that on?

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/100#issuecomment-159406806


#6

@leedm777 Thank you! We'll get this tested when we get a chance.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/100#issuecomment-160721186


#7

BTW, I've submitted the signed CLA.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/100#issuecomment-161088993


#8

bump.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/100#issuecomment-169203673


#9

Will test and merge this or next week. Thanks!

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/100#issuecomment-169416710


#10

Merged #100.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/100#event-509432560


#11

Merged. Also applied the same to jicofo: https://github.com/jitsi/jicofo/commit/57563e815b60cc5a5e7ca6091d7ffac332c82e77

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/100#issuecomment-170012296


#12

@bgrozev Very cool. Thanks!

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/100#issuecomment-170615898