Bug 31348 - Plack stop should be graceful
Summary: Plack stop should be graceful
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Packaging (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: David Cook
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 17814
Blocks:
  Show dependency treegraph
 
Reported: 2022-08-12 02:22 UTC by David Cook
Modified: 2022-10-23 09:57 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00,22.05.06,21.11.12


Attachments
Bug 31348: Make koha-plack stop gracefully (1.33 KB, patch)
2022-08-12 02:29 UTC, David Cook
Details | Diff | Splinter Review
Bug 31348: Make koha-plack stop gracefully (1.37 KB, patch)
2022-08-19 23:48 UTC, David Nind
Details | Diff | Splinter Review
Bug 31348: Make koha-plack stop gracefully (1.47 KB, patch)
2022-08-22 07:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2022-08-12 02:22:12 UTC
Instead of sending a TERM to Starman, we should send a QUIT, as that will complete any current requests before shutting down. 

It should make for a better user experience.
Comment 1 David Cook 2022-08-12 02:29:01 UTC
Created attachment 139036 [details] [review]
Bug 31348: Make koha-plack stop gracefully

0. Apply patch
1. cp /usr/sbin/koha-plack /usr/sbin/koha-plack.bak
2. cp debian/scripts/koha-plack /usr/sbin/koha-plack
3. Now at roughly the same time do the following two actions:
    3a. Go to http://localhost:8081/cgi-bin/koha/about.pl
    3b. koha-plack --stop kohadev
4. Note that the HTTP request completes
5. Click on "Home"
6. Note that the error is "Service Unavailable" rather than "Proxy Error",
which would've occurred with an interrupted HTTP request/response
Comment 2 Jonathan Druart 2022-08-17 09:50:04 UTC
Same for debian/scripts/koha-z3950-responder?
Comment 3 David Cook 2022-08-18 01:27:38 UTC
(In reply to Jonathan Druart from comment #2)
> Same for debian/scripts/koha-z3950-responder?

I don't think so. The QUIT signal is specific to Starman. The Z3950 responder might have signal handling somewhere but I didn't notice anything at a glance.
Comment 4 David Nind 2022-08-19 23:48:00 UTC
Created attachment 139525 [details] [review]
Bug 31348: Make koha-plack stop gracefully

0. Apply patch
1. cp /usr/sbin/koha-plack /usr/sbin/koha-plack.bak
2. cp debian/scripts/koha-plack /usr/sbin/koha-plack
3. Now at roughly the same time do the following two actions:
    3a. Go to http://localhost:8081/cgi-bin/koha/about.pl
    3b. koha-plack --stop kohadev
4. Note that the HTTP request completes
5. Click on "Home"
6. Note that the error is "Service Unavailable" rather than "Proxy Error",
which would've occurred with an interrupted HTTP request/response

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Marcel de Rooy 2022-08-22 07:06:34 UTC
Created attachment 139566 [details] [review]
Bug 31348: Make koha-plack stop gracefully

0. Apply patch
1. cp /usr/sbin/koha-plack /usr/sbin/koha-plack.bak
2. cp debian/scripts/koha-plack /usr/sbin/koha-plack
3. Now at roughly the same time do the following two actions:
    3a. Go to http://localhost:8081/cgi-bin/koha/about.pl
    3b. koha-plack --stop kohadev
4. Note that the HTTP request completes
5. Click on "Home"
6. Note that the error is "Service Unavailable" rather than "Proxy Error",
which would've occurred with an interrupted HTTP request/response

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2022-08-22 07:09:13 UTC
Why did you wait so long with this patch, David ? :)
I would be tempted to classify it as a bug btw. And backport?

Tomas: What do you think?
Comment 7 Tomás Cohen Arazi 2022-08-22 16:43:17 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 8 Marcel de Rooy 2022-08-23 12:53:00 UTC
(In reply to Marcel de Rooy from comment #6)
> Why did you wait so long with this patch, David ? :)
> I would be tempted to classify it as a bug btw. And backport?
> 
> Tomas: What do you think?

Lets do it.
Comment 9 Lucas Gass 2022-10-14 22:03:58 UTC
Backported to 22.05.x for upcoming 22.05.06 release.
Comment 10 Victor Grousset/tuxayo 2022-10-21 22:29:45 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document it seems, marking resolved.
Comment 11 Arthur Suzuki 2022-10-22 22:10:17 UTC
Thanks!

Pushed to 21.11 for 21.11.12