Closed BClark09 closed 7 years ago
@theoweiss @ThomDietrich, could you comment on the efficiency of the proposed script if you have time?
Thanks for the comments @theoweiss and @ThomDietrich! I updated to use ps aux --sort=start_time | grep openhab.*java | grep -v grep | awk '{print $2}' | tail -1
so that any successful start should be update the pid file with the latest openhab owned java pid.
I've tested the line on my system, looks good. I'm still unsure about the effect --sort=start_time
and tail -1
will have. Can there be multiple processes, what should be done in this case, how can we be certain that the latest of these is the one we are looking for?
Also, how about only defining the command once?
Thanks once again @ThomDietrich!
Can there be multiple processes, what should be done in this case, how can we be certain that the latest of these is the one we are looking for?
The only time I was able to get multiple processes with this script was if you follow a stop command with a start command. In this case, the combination of --sort=start_time
and tail -1
will pick the starting process, as the first process is invalid and will stop soon.
Let's assume that there is some way that the openHAB user can open a second java process for itself:
service openhab2 start
will function as expected, by preventing you from opening yet another instance of openHAB. The error message wouldn't technically be wrong either, any PID would still point to something that needs to be stopped before you should try and start another instance of openHAB.
service openhab2 stop
doesn't care about the PID number so it'll just call the stop script which should handle the correct stop procedure.
Also, how about only defining the command once?
Good idea, this is best done as a function call right?
Okay then, I see you you thought of everything.
Function call or you could define the command as a string variable and call it via eval
but maybe it's not worth it. I'd be emotionally okay if you just merged the script as it is now :smiley_cat:
Thanks, always good to test the script's (and my own) logic! @theoweiss if you are happy with this implementation and it's style/formatting then I would consider this safe to merge. I've tested on Ubuntu 14.04 and Debian 8 (just in case), which use the two different startup processes we should support.
Uups, I added some comments on monday but forgot to finish and submit the review. Now the comments are lost. I'll try it again this evening :-((
LGTM so far. I'll merge this PR.
Added init.d to the gradle build for Linux init systems based on sysVinit.
Also prevented more than one instance from running "start" multiple times and implemented a simple mechanism to keep /var/run/openhab.pid up to date.
This should not affect Linux systems based on systemd.
Resolves https://github.com/openhab/openhab-distro/issues/328
Signed-off-by: Ben Clark ben@benjyc.uk