Enable Plack hot-reload via ENV
This patch adds a new ENV var KOHA_PLACK_HOT_RELOAD. If it is set,
run.sh
will stop the koha-plack
that's started via koha-common
and
instead call plackup
with a bunch of options that turn on hot reload,
watching for changes in /kohadevbox/koha
(i.e. the whole koha source)
As KTD is only using the pre-built images, I also mounted my localy
changed run.sh
into the container via the following snippet (which I
did not commit, because it makes no sense as soon this commit is merged)
diff --git a/docker-compose-light.yml b/docker-compose-light.yml
index 02354d9..c457882 100644
--- a/docker-compose-light.yml
+++ b/docker-compose-light.yml
@@ -19,6 +19,7 @@ services:
- ALL
volumes:
- ${SYNC_REPO}:/kohadevbox/koha
+ - ./files/run.sh:/kohadevbox/run.sh
tmpfs:
- /cover_db
- /tmp
The params to plackup
were extracted from koha-plack
based on the
values set inside KTD. I don't think it makes sense / is neccesary to
dynamically calculate most of those values, as KTD is a very controlled
environment.
Merge request reports
Activity
added 1 commit
- d7a30b87 - document KOHA_PLACK_HOT_RELOAD=1 and remove the description of the old workaround
I mean...
I know this would be too much to ask. But shouldn't we either:
- Tweak koha-plack to handle this through a switch (
DEV_INSTALL
already makes it deal with the path to/kohadevbox/koha
usingadjust_paths_dev_install
. - Provide an alias/script. Do a
git grep start_plack_debug
and you will get it.
Any of those options seem cleaner. I'd go with (2) for simplicity.
Side note: as mentioned above, koha-plack already has some handling for development mode, including remote debugger configurations read from ENV. So that's still a solid place to wire this in.
- Tweak koha-plack to handle this through a switch (
Just testing the current POC: an interesting difference with main is that multiple workers seems to be forked from a compiled 1st worker. Or some similar approach that in the end allows to start ktd with 100 workers and have a code reload with these 100 workers without having 100 compilations. The reload is quite heavy, but nowhere near what it would be with 100 compilations.
(cc @minusdavid I don't know if there is hope in managing to get just that startup mechanism in production to help with some drawbacks with https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31074#c29 And helping in general with instance restart. Maybe just the same startup way without -R)
RAM usage at a first glace seems similar than with one worker. At least it wasn't the usage of 100 workers. Then over the requests, all workers will respond in a round robin fashion with a normal response time. And the when each will have responded to one request, the RAM usage was much larger.
Let's test final RAM usage with 20 workers started the classic way vs the with the hot reload way (the different way that gives us hot reload and what seems to be post compile forking). In each case, requests where made to ensure each worker got to process one. htop was used to confirm that each worker got a request (by seing a change from process base ram usage). The classic way to start starman doesn't seem to have the requests distributed in a round robin fashion, I had to do between 5 and 10 additional requests in a rapid burst to ensure all workers got one. When making them slowly, it seems it allocates them to the same worker. Maybe to get some sort of scheduling or process affinity benefit or to not have all workers go a the same time through their life cycle and accumulate eventual memory leaks all at the same time. So maybe there is a (small?) optimisation is lost there that the classic way has.
Restarting ktd and doing the requests was repeated 3 times per approach. Results (used RAM + swap(got other things on the background but I didn't touch them))
- classic startup
- 12787
- 13233
- 13164
- hot reload startup
- 9534
- 9911
- 9093
What that tells us is that the copy on write bonus that post compile forking (←IIUC) provides is still there after processing a request, there is still a lot of untouched RAM per worker. So the RAM usage bonus isn't going away after the 1st request. That's a nice difference! :D
Edited by Victor Grousset/tuxayo- classic startup
That's an interesting observation. Looking at Plack::Runner (the module used by plackup) and Plack::Loader::Restarter, it does look like the whole app is preloaded when using plackup.
Starman also has a --preload-app option which defaults to false/off, and comes with a warning: "Enabling this option can cause bad things happen when resources like sockets or database connections are opened at load time by the master process and shared by multiple children"
So I think we'd want to be careful before we started using that in production. However... if we started doing it in dev first maybe we would catch any problems before they become problems.
I think DBIx::Class might be able to auto-detect forking and refresh database connections. And then the only other socket connections would be to Zebra I suspect, and we don't create that socket on startup it seems.
Certainly something to explore!
--
On a related note, I have done some work on "safe" preloading with Koha: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36721
@tuxayo you might find that interesting. There's a lot more that could be done with it. And really pre-loading the whole app would be ideal. I do that with other apps, and it's wonderful. It makes for very fast low impact restarts.
With the hot reload startup I see the main/master process every 3 seconds having a peak of 100% CPU thread activity during like 1 second. It might be the scanning for changes, that's very heavy. It's not seen with the classic way of starting starman. Maybe tuning the paths to watch would bring that to a reasonable level? At least avoiding the main .git directory, the l10n .git directory and the node_modules one would make a big cut in the number of files. Hopefully it is possible to allowlist only the relevant files and directories to get something much more narrow than avoiding the big spots.