Starman daemon as an option to ask for "reload" meaning the renew of all workers, like Apache graceful restart. Starman restart will kill at once the master process and workers. Starman reload will not impact the master process, it will kill and recreate idle workers, it will wait for busy workers to end before renew them. It is a nice way to refresh the plack middleware without impacting any running page, mostly useful for production servers.
To trigger this reload, juste run : /bin/kill -HUP $MASTER_PID MASTER_PID beeing the pid of the starman master process, can be fetched in pid file. Source : https://metacpan.org/pod/distribution/Starman/script/starman#SIGNALS
In koha-suomi fork we use start-stop-daemon to send the HUP signal, that's probably the preferable way since koha-plack uses start-stop-daemon already to send the process kill signal.
This would be great
(In reply to Joonas Kylmälä from comment #2) > In koha-suomi fork we use start-stop-daemon to send the HUP signal, that's > probably the preferable way since koha-plack uses start-stop-daemon already > to send the process kill signal. I'm only seeing "start-stop-daemon" for koha-plack --stop for some reason...
(In reply to Fridolin Somers from comment #1) > To trigger this reload, juste run : > > /bin/kill -HUP $MASTER_PID > > MASTER_PID beeing the pid of the starman master process, can be fetched in > pid file. > > Source : > https://metacpan.org/pod/distribution/Starman/script/starman#SIGNALS I'd forgotten about this comment, but I just used the following today in prod: kill -SIGHUP $(cat /var/run/koha/<instance name>/plack.pid) I wanted to enable plugins and then reload after installing the plugin, and it was so elegant.
I don't know if we could use it with Koha package installs, as I think the koha-common service is stopped and started, although I'm not 100% sure of that mechanism at this point...
Question: - Do we want "koha-plack --reload" or do we want "koha-plack --graceful" or both? I think Apache treats reload/graceful/force-reload as the same.
Would it be too risky to send a HUP-signal to the plack process automatically when a plugin is reuploaded? I think that this would be very nice, as it would make plugins upgradable without command line access.
(In reply to Andreas Jonsson from comment #8) > Would it be too risky to send a HUP-signal to the plack process > automatically when a plugin is reuploaded? > > I think that this would be very nice, as it would make plugins upgradable > without command line access. You know... I've been thinking about that idea for years but I never actually tried it. I just tried it now and it worked well! With the Debian packages, each instance is run using a specific Linux user, and it looks like that Linux user can HUP processes it owns, so yeah... it works. -- In terms of risk, I suppose allowing a web user to affect background Linux processes is not ideal. I have noticed that the REST API can be slow to startup and can consume a lot of CPU, so I suppose the risk would be that an accident or an attack could lead to a denial of service. But I think that's a fairly low risk. After all, you can already do a DDOS by attacking the OPAC search function. So I think the risk is worth the reward.
(In reply to Andreas Jonsson from comment #8) > Would it be too risky to send a HUP-signal to the plack process > automatically when a plugin is reuploaded? > > I think that this would be very nice, as it would make plugins upgradable > without command line access. I've opened bug 30897 for this one but it's easier said than done...
This bug report is assigned, can we expect a patch soon?
(In reply to Joonas Kylmälä from comment #2) > In koha-suomi fork we use start-stop-daemon to send the HUP signal, that's > probably the preferable way since koha-plack uses start-stop-daemon already > to send the process kill signal. Would that look like the following? start-stop-daemon --pidfile ${PIDFILE} --user="${instancename}-koha" --stop --signal HUP;
(In reply to Jonathan Druart from comment #11) > This bug report is assigned, can we expect a patch soon? It should be a simple patch. I feel like I'd probably be more useful in test/QA for this one, but I could write it if no one else is going to...
Created attachment 139011 [details] [review] Bug 21366: Add koha-plack --reload to gracefully restart Plack/Starman This patch adds a "--reload" option to koha-plack, which allows a sysadmin to gracefully restart Koha. This is very useful when installing Koha plugins or deploying a hot fix where you need to change code but don't want to interrupt anyone's usage of Koha. 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. ps -efww | grep "starman" 4. Now at roughly the same time do the following two actions: 4a. Go to http://localhost:8081/cgi-bin/koha/about.pl 4b. koha-plack --reload kohadev 5. Note that the web request completes successfully 6. ps -efww | grep "starman" 7. Note that the "starman master" process stays alive, but the "starman worker" processes have been restarted
Ok patch prepared. Looking at Fridolin and Joonas for sign off and QA!
Created attachment 139015 [details] [review] Bug 21366: Add koha-plack --reload to gracefully restart Plack/Starman This patch adds a "--reload" option to koha-plack, which allows a sysadmin to gracefully restart Koha. This is very useful when installing Koha plugins or deploying a hot fix where you need to change code but don't want to interrupt anyone's usage of Koha. 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. ps -efww | grep "starman" 4. Now at roughly the same time do the following two actions: 4a. Go to http://localhost:8081/cgi-bin/koha/about.pl 4b. koha-plack --reload kohadev 5. Note that the web request completes successfully 6. ps -efww | grep "starman" 7. Note that the "starman master" process stays alive, but the "starman worker" processes have been restarted Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Note that the help is missing the new option.
(In reply to Jonathan Druart from comment #17) > Note that the help is missing the new option. Believe it or not, that thought crossed my mind as I was falling asleep last night haha. I'll look at that.
Created attachment 139035 [details] [review] Bug 21366: Add --reload option to koha-plack help/manual
(Side note, I've opened bug 31348 to handle graceful stopping of Starman/Plack as well.)
Created attachment 139565 [details] [review] Bug 21366: Add --reload option to koha-plack help/manual
Last patch amended to explain what reloading means. Reload the Plack daemon for the desired Koha instances. ↓↓↓ Reload the Plack daemon for the desired Koha instances, letting the busy workers finish processing their requests before restarting them. Tell me if the phrasing can be improved.
Reading this on CPAN: Sending HUP signal to the master process will restart all the workers gracefully (meaning the currently running requests will shut down once the request is complete), and by default, the workers will pick up the code changes you make by reloading the application. If you enable --preload-app option, however, the code will be only loaded in the startup process and will not pick up the code changes you made. If you want to preload the app and do graceful restarts by reloading the code changes, you're recommended to use Server::Starter, configured to send QUIT signal when superdaemon received HUP ... [End of quote] While testing with about and inserting a small code change, I notice that while I started the about web request before the plack reload, the code change already is visible in the web request. I think that this is not what we want here ?
(In reply to Marcel de Rooy from comment #23) > If you enable --preload-app option, however, the code will be only loaded in the startup process and will not pick up the code changes you made. If you want to preload the app and do graceful restarts by reloading the code changes, you're recommended to use Server::Starter, configured to send QUIT signal when superdaemon received HUP ... > [End of quote] > While testing with about and inserting a small code change, I notice that > while I started the about web request before the plack reload, the code > change already is visible in the web request. > I think that this is not what we want here ? What do you mean? The patch doesn't use --preload-app. I tested changing something in about.pl: once over the ~10 tries the change was reflected only by reloading the page. Otherwise I had to run `pkill starman --signal SIGHUP` to get the change. Your plack_max_requests isn't 1 is it? That would be an explanation of what you observed.
(In reply to Marcel de Rooy from comment #23) > If you enable --preload-app option, however, the code will be only loaded in > the startup process and will not pick up the code changes you made. If you > want to preload the app and do graceful restarts by reloading the code > changes, you're recommended to use Server::Starter, configured to send QUIT > signal when superdaemon received HUP ... > [End of quote] We don't use --preload-app, so we don't need to worry about it. Even if we did use --preload-app, I think some of the text is misleading. While code changes to code that is preloaded wouldn't come through, changes to code that is lazy loaded after forking the children would still come through. I use mod_perl for a different Perl app where I preload the core Catalyst application into the httpd master process so that slow setup like the ORM just takes place 1 time, and then all the lazy loads for function specific things happen within the child processes. > While testing with about and inserting a small code change, I notice that > while I started the about web request before the plack reload, the code > change already is visible in the web request. > I think that this is not what we want here ? If you want to test a small code change, you'd need to first make sure that all Starman workers have already cached the about.pl page in their CGI to PSGI cache. Once they're cached, then you can make your code change, confirm that the change isn't visible, do the reload, and then note that the change is visible. I chose the about.pl test somewhat arbitrarily because koha-testing-docker is such a small database. In production, where you really see this being useful is in search.pl and opac-search.pl where a search might take 5+ seconds. Using koha-plack --reload will wait for that long search request to complete, and then it will restart that worker. -- Does that answer your questions? I will be online a bit tonight (ie European morning) so might be able to answer more on IRC :)
(In reply to Victor Grousset/tuxayo from comment #24) > (In reply to Marcel de Rooy from comment #23) > What do you mean? The patch doesn't use --preload-app. I did not say that. Please read the comment. It is a quote. > I tested changing something in about.pl: once over the ~10 tries the change > was reflected only by reloading the page. Otherwise I had to run `pkill > starman --signal SIGHUP` to get the change. I had the change reflected all tries. The comment of David explains it, I guess. > Your plack_max_requests isn't 1 is it? That would be an explanation of what > you observed. No it is not :)
(In reply to David Cook from comment #25) > We don't use --preload-app, so we don't need to worry about it. I know that. Quoting etc. > Even if we did use --preload-app, I think some of the text is misleading. > While code changes to code that is preloaded wouldn't come through, changes > to code that is lazy loaded after forking the children would still come > through. This explains my testing. Lazy brain :) All workers need to cache the old code first obviously. I just got the wrong worker a few times. Inspired by Victor, I will set the number of workers as low as possible and test again.. > I chose the about.pl test somewhat arbitrarily because koha-testing-docker > is such a small database. In production, where you really see this being > useful is in search.pl and opac-search.pl where a search might take 5+ > seconds. Using koha-plack --reload will wait for that long search request to > complete, and then it will restart that worker. I do not use k-t-d, but your point is clear. Resuming QA scheduled for this afternoon.
(In reply to Marcel de Rooy from comment #27) > > I chose the about.pl test somewhat arbitrarily because koha-testing-docker > > is such a small database. In production, where you really see this being > > useful is in search.pl and opac-search.pl where a search might take 5+ > > seconds. Using koha-plack --reload will wait for that long search request to > > complete, and then it will restart that worker. > I do not use k-t-d, but your point is clear. > > Resuming QA scheduled for this afternoon. I'll be around on and off for the next 5-7 hours, so shoot me any questions you might have. For what it's worth, I have a "reload_koha.sh" script I use in production all the time, especially when updating plugins, and it's wonderful hehe.
(In reply to Marcel de Rooy from comment #27) > > seconds. Using koha-plack --reload will wait for that long search request to > > complete, and then it will restart that worker. koha-plack does not wait for the request to complete but it does the job as expected: The current request gave me back the old contents, but Starman closed the old worker afterwards since a new request gives me the new contents. Excellent.
Created attachment 139668 [details] [review] Bug 21366: Add koha-plack --reload to gracefully restart Plack/Starman This patch adds a "--reload" option to koha-plack, which allows a sysadmin to gracefully restart Koha. This is very useful when installing Koha plugins or deploying a hot fix where you need to change code but don't want to interrupt anyone's usage of Koha. 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. ps -efww | grep "starman" 4. Now at roughly the same time do the following two actions: 4a. Go to http://localhost:8081/cgi-bin/koha/about.pl 4b. koha-plack --reload kohadev 5. Note that the web request completes successfully 6. ps -efww | grep "starman" 7. Note that the "starman master" process stays alive, but the "starman worker" processes have been restarted Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 139669 [details] [review] Bug 21366: Add --reload option to koha-plack help/manual Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 22.11. Nice work everyone, thanks!
(In reply to Marcel de Rooy from comment #26) > (In reply to Victor Grousset/tuxayo from comment #24) > > (In reply to Marcel de Rooy from comment #23) > > What do you mean? The patch doesn't use --preload-app. > I did not say that. Please read the comment. It is a quote. That confusion was after reading it several time before writing the message because I wasn't sure what you meant by this quote. And the best I could come up with is that we were using --preload-app So still confused but glad it got sorted out in this end ^o^
Enhancement will not be backported to 22.05.x series