| Summary: | background job worker is still running with all the modules in RAM | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
| Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
| Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
| Severity: | major | ||
| Priority: | P5 - low | CC: | arthur.suzuki, dcook, lucas, m.de.rooy, martin.renvoize, tomascohen |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32558 | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
23.05.00,22.11.03,22.05.10
|
|
| Circulation function: | |||
| Bug Depends on: | 28413, 30410 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 32561: Prevent worker to run with unecessary modules in memory
Bug 32561: Add test Bug 32561: Add test Bug 32561: Prevent worker to run with unecessary modules in memory Bug 32561: Add test Bug 32561: Test loaded modules for background_jobs_worker.pl Bug 32561: Prevent worker to run with unecessary modules in memory |
||
|
Description
Jonathan Druart
2023-01-04 08:34:46 UTC
Created attachment 144973 [details] [review] Bug 32561: Prevent worker to run with unecessary modules in memory Regression of bug 28413, bug 30410 added a "use Koha::Plugins" statement at the top of Koha::BackgroundJob, and so all Koha module are loaded by the worker on startup. See bug 28413 for more info Created attachment 144974 [details] [review] Bug 32561: Add test This test does not seem reliable enough, how could we improve it? I guess it will fail randomly, especially if ran in parallel with other processes/tests. Created attachment 144975 [details] [review] Bug 32561: Add test This test does not seem reliable enough, how could we improve it? I guess it will fail randomly, especially if ran in parallel with other processes/tests. Created attachment 144984 [details] [review] Bug 32561: Prevent worker to run with unecessary modules in memory Regression of bug 28413, bug 30410 added a "use Koha::Plugins" statement at the top of Koha::BackgroundJob, and so all Koha module are loaded by the worker on startup. See bug 28413 for more info Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 144985 [details] [review] Bug 32561: Add test This test does not seem reliable enough, how could we improve it? I guess it will fail randomly, especially if ran in parallel with other processes/tests. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 144986 [details] [review] Bug 32561: Test loaded modules for background_jobs_worker.pl Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> xt/memory_check.t bash: xt/memory_check.t: Permission denied root@master:/usr/share/koha# chmod 755 xt/memory_check.t Argument "/usr/share/koha/misc/background_jobs_worker.pl" isn't numeric in numeric gt (>) at xt/memory_check.t line 31. ok 1 - background_jobs_worker.pl is consuming /usr/share/koha/misc/background_jobs_worker.pl in memory Warning needs a fix? The test description of A consumes A is weird. I would personally only go for pushing the first patch. The two others seem not really needed. But thats just my first impression.. (In reply to Marcel de Rooy from comment #7) > I would personally only go for pushing the first patch. The two others seem > not really needed. But thats just my first impression.. Yes, same feeling for me. But this problem will come up again if there is no test :-/ (In reply to Jonathan Druart from comment #8) > (In reply to Marcel de Rooy from comment #7) > > I would personally only go for pushing the first patch. The two others seem > > not really needed. But thats just my first impression.. > > Yes, same feeling for me. But this problem will come up again if there is no > test :-/ That's possible although I think plugin support was one of the last pieces we needed to add? Perhaps it would be an idea to add some text to the "use <module>" area like "DO NOT ADD KOHA MODULES HERE OR YOU WILL FACE JONATHAN'S WRATH! See BZ 32561!" Like I asked in https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30410#c18, do we have any example plugins that we can use to verify this change? I don't see any obvious ones listed at https://wiki.koha-community.org/wiki/Koha_plugins (In reply to Marcel de Rooy from comment #7) > xt/memory_check.t > bash: xt/memory_check.t: Permission denied > root@master:/usr/share/koha# chmod 755 xt/memory_check.t Please fix > Argument "/usr/share/koha/misc/background_jobs_worker.pl" isn't numeric in > numeric gt (>) at xt/memory_check.t line 31. Please check. > ok 1 - background_jobs_worker.pl is consuming > /usr/share/koha/misc/background_jobs_worker.pl in memory > Warning needs a fix? The test description of A consumes A is weird. Please adjust. (In reply to Marcel de Rooy from comment #11) > (In reply to Marcel de Rooy from comment #7) > > xt/memory_check.t > > bash: xt/memory_check.t: Permission denied > > root@master:/usr/share/koha# chmod 755 xt/memory_check.t > > Please fix > > > Argument "/usr/share/koha/misc/background_jobs_worker.pl" isn't numeric in > > numeric gt (>) at xt/memory_check.t line 31. > > Please check. > > > ok 1 - background_jobs_worker.pl is consuming > > /usr/share/koha/misc/background_jobs_worker.pl in memory > > Warning needs a fix? The test description of A consumes A is weird. > > Please adjust. We removed the tests here. Created attachment 145617 [details] [review] Bug 32561: Prevent worker to run with unecessary modules in memory Regression of bug 28413, bug 30410 added a "use Koha::Plugins" statement at the top of Koha::BackgroundJob, and so all Koha module are loaded by the worker on startup. See bug 28413 for more info Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> (In reply to Jonathan Druart from comment #12) > We removed the tests here. Good for me. Pushed to master for 23.05. Nice work everyone, thanks! Nice work everyone! Pushed to stable for 22.11.x Backported to 22.05.x for upcoming 22.05.10 applied to 21.11.x for 21.11.16 oh, actually this one depends on 30410 which was not applied to 21.11. Can't backport. |