Bug 32561 - background job worker is still running with all the modules in RAM
Summary: background job worker is still running with all the modules in RAM
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 28413 30410
Blocks:
  Show dependency treegraph
 
Reported: 2023-01-04 08:34 UTC by Jonathan Druart
Modified: 2023-01-31 14:16 UTC (History)
4 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:
23.05.00,22.11.03


Attachments
Bug 32561: Prevent worker to run with unecessary modules in memory (1.19 KB, patch)
2023-01-04 09:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32561: Add test (2.03 KB, patch)
2023-01-04 09:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32561: Add test (1.95 KB, patch)
2023-01-04 09:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32561: Prevent worker to run with unecessary modules in memory (1.24 KB, patch)
2023-01-04 11:53 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 32561: Add test (2.00 KB, patch)
2023-01-04 11:53 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 32561: Test loaded modules for background_jobs_worker.pl (2.27 KB, patch)
2023-01-04 11:53 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 32561: Prevent worker to run with unecessary modules in memory (1.34 KB, patch)
2023-01-24 13:56 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 Jonathan Druart 2023-01-04 08:34:46 UTC
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.
Comment 1 Jonathan Druart 2023-01-04 09:09:19 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
Comment 2 Jonathan Druart 2023-01-04 09:15:42 UTC
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.
Comment 3 Jonathan Druart 2023-01-04 09:16:43 UTC
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.
Comment 4 Kyle M Hall 2023-01-04 11:53:34 UTC
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>
Comment 5 Kyle M Hall 2023-01-04 11:53:45 UTC
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>
Comment 6 Kyle M Hall 2023-01-04 11:53:49 UTC
Created attachment 144986 [details] [review]
Bug 32561: Test loaded modules for background_jobs_worker.pl

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Marcel de Rooy 2023-01-13 10:28:26 UTC
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..
Comment 8 Jonathan Druart 2023-01-13 10:39:16 UTC
(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 :-/
Comment 9 David Cook 2023-01-16 04:12:24 UTC
(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!"
Comment 10 David Cook 2023-01-16 04:18:36 UTC
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
Comment 11 Marcel de Rooy 2023-01-24 12:06:43 UTC
(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.
Comment 12 Jonathan Druart 2023-01-24 12:58:48 UTC
(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.
Comment 13 Marcel de Rooy 2023-01-24 13:56:51 UTC
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>
Comment 14 Marcel de Rooy 2023-01-24 13:57:28 UTC
(In reply to Jonathan Druart from comment #12)
> We removed the tests here.
Good for me.
Comment 15 Tomás Cohen Arazi 2023-01-27 18:22:01 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 16 Matt Blenkinsop 2023-01-31 14:16:18 UTC
Nice work everyone!

Pushed to stable for 22.11.x