Bug 17814

Summary: koha-plack --stop should make sure that Plack really stop
Product: Koha Reporter: Pongtawat <pongtawat>
Component: Architecture, internals, and plumbingAssignee: Pongtawat <pongtawat>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: barton, jonathan.druart, jsterben, julian.maurice, katrin.fischer, kyle, m.de.rooy, magnus, mtj, veron
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16885
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18998
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 31348    
Attachments: Unverified patch
Add retry to koha-plack stop
Bug 17814: koha-plack --stop should make sure that Plack really stop
Bug 17814: koha-plack --stop should make sure that Plack really stop

Description Pongtawat 2016-12-26 10:43:59 UTC
koha-plack --stop doesn't ensure that Plack was really stopped before returning. 

So in case that koha-plack --stop was quickly follows by koha-plack --start (e.g. logrotate), it could leave Plack in stop state. This is due to koha-plack --start think that Plack was already started, while it actually is being stopped.

For me I think this is the cause why I got random Plack stop when logrotate is run. 

It should be similar to the case of Zebra in Bug #16885, and the solution might be the same: adding "--retry=TERM/30/KILL/5;" to start-stop-daemon command.

I just try that on our server and will see if the problem goes away.
Comment 1 Pongtawat 2016-12-26 10:54:26 UTC
Created attachment 58427 [details] [review]
Unverified patch

A simple patch follows that in #16885. If it work there, it should work here too. But not yet verified.
Comment 2 Magnus Enger 2016-12-30 14:19:20 UTC
Should this be "Needs signoff"? Or is it too early yet?
Comment 3 Barton Chittenden 2017-01-09 18:35:13 UTC
I think that --restart also has this issue. Here's a paste from a recent restart:

sudo koha-plack --restart glenoaks
[....] Restarting Plack daemon for glenoaks:[....] Stopping Plack daemon for gle[ ok :.
[FAIL] Error: Plack already running for glenoaks: failed!
 failed!
Comment 4 Pongtawat 2017-01-10 03:42:55 UTC
Won't --restart work properly if stop work properly? 
From the code, it seems that --restart just call stop and then start.
Comment 5 Pongtawat 2017-01-10 03:58:03 UTC
TEST PLAN
---------
1) Login to staff client and do something that will hold connection for a long time, e.g. a batch import or a slow report.
2) sudo koha-plack --stop mykoha
3) ps aux | grep plack   <-- a Plack process will still running work in 1)
4) wait for 1) to finish and all Plack processes exit
5) sudo koha-plack --start mykoha
6) apply the patch
7) repeat step 1)-2)
8) ps aux | grep plack   <-- There should be no Plack process running now
9) Note that work in step 1) might get terminated midway. Since we force Plack to stop after some wait.

Alternatively, I think we can test with --restart too. Before patch, it should behave like in comment 3. After patch, it should restart properly.
Comment 6 Pongtawat 2017-02-22 17:05:50 UTC
With this patch, I'm running without problem for a month. So, just hope this get merged in.
Comment 7 Pongtawat 2017-04-04 16:13:12 UTC
Created attachment 61845 [details] [review]
Add retry to koha-plack stop
Comment 8 Marc Véron 2017-04-05 20:49:18 UTC
Created attachment 61927 [details] [review]
Bug 17814: koha-plack --stop should make sure that Plack really stop

Followed test plan in comment #5.
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 9 Jonathan Druart 2017-04-19 19:49:53 UTC
Created attachment 62421 [details] [review]
Bug 17814: koha-plack --stop should make sure that Plack really stop

koha-plack --stop doesn't ensure that Plack was really stopped before
returning

So in case that koha-plack --stop was quickly follows by koha-plack
--start (e.g. logrotate), it could leave Plack in stop state. This is
due to koha-plack --start think that Plack was already started, while it
actually is being stopped.

For me I think this is the cause why I got random Plack stop when
logrotate is run.

It should be similar to the case of Zebra in Bug #16885, and the
solution might be the same: adding "--retry=TERM/30/KILL/5;" to
start-stop-daemon command.

TEST PLAN
---------
1) Login to staff client and do something that will hold connection for
a long time, e.g. a batch import or a slow report.
2) sudo koha-plack --stop mykoha
3) ps aux | grep plack   <-- a Plack process will still running work in
1)
4) wait for 1) to finish and all Plack processes exit
5) sudo koha-plack --start mykoha
6) apply the patch
7) repeat step 1)-2)
8) ps aux | grep plack   <-- There should be no Plack process running
now
9) Note that work in step 1) might get terminated midway. Since we force
Plack to stop after some wait.

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Kyle M Hall 2017-04-21 11:38:30 UTC
Pushed to master for 17.05, thanks Pongtawat!
Comment 11 Katrin Fischer 2017-04-21 22:23:57 UTC
Thx Pongtawat!
Comment 12 Katrin Fischer 2017-04-21 22:24:17 UTC
This patch has been pushed to 16.11.x and will be in 16.11.07.
Comment 13 Mason James 2017-05-03 04:33:48 UTC
Pushed to 16.05.x, for 16.05.12 release
Comment 14 Julian Maurice 2017-05-10 09:29:15 UTC
Pushed to 3.22.x for 3.22.21