[Bug 39769] New: es_indexer_daemon.pl uses stale L1 cache
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Bug ID: 39769 Summary: es_indexer_daemon.pl uses stale L1 cache Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Searching - Elasticsearch Assignee: koha-bugs@lists.koha-community.org Reporter: dcook@prosentient.com.au QA Contact: testopia@bugs.koha-community.org As bug 36549 notes, the es_indexer_daemon.pl doesn't fork child processes to do its work. This leads to memory leaks, but it also means that it's using a stale L1 cache. If you update a system preference like IncludeSeeFromInSearches, you have to restart your es_indexer_daemon.pl worker before you can re-index your database. Note that the solution to this problem does not have to be updating the daemon to fork child process workers. Rather, we need to clear the L1 cache after we fetch a job. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36549 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Robin Sheat <robin@kallisti.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |robin@kallisti.net.nz --- Comment #1 from Robin Sheat <robin@kallisti.net.nz> --- A model that I've used in other things with good success to solve this sort of issue is: 1. Have the script die after, say, 20 minutes (you'll need to add a timeout to `receive_frame` so that it doesn't block, but that's find) 2. Have a cron job that launches the script every 15 minutes, but does so using a mechanism that prevents multiple instances running at once (I think systemd has a function for this, otherwise it's possible to use flock) This way, you are sure that any unused memory is cleared regularly, any updates to the code are picked up quickly, if it crashes it'll be running again soon, and so on. We called them "cron daemons" (distinct from the cron daemon that actually does cron.) It will need some migration (from daemon start to a cronjob), so it might be good to have a "timeout" command line option and then it can be migrated in the packages for example, but normal "run forever" behaviour will remain for everyone else. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 --- Comment #2 from Robin Sheat <robin@kallisti.net.nz> --- To clarify, the flock or whatever you use to prevent multiple executions should *block and wait* until the running process terminates before it starts, it shouldn't abort immediately. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |42669 Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Patch complexity|--- |Trivial patch Status|NEW |Needs Signoff Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42669 [Bug 42669] es_indexer_daemon.pl silently marks jobs finished on indexing failure and doesn't recover from NoNodes -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 --- Comment #3 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 199403 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199403&action=edit Bug 39769: Flush L1 cache in es_indexer_daemon.pl before each batch The daemon runs as a long-lived process and never clears its L1 cache. System preference changes (e.g. IncludeSeeFromInSearches) are not picked up until the daemon is manually restarted. Flush L1 caches at the start of each commit() call so that preference changes take effect on the next indexing batch without requiring a daemon restart. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |The Elasticsearch indexer release notes| |daemon is a long-lived | |process that never cleared | |its L1 (in-process) cache, | |meaning changes to system | |preferences like | |IncludeSeeFromInSearches | |were not picked up until | |the daemon was manually | |restarted. The L1 cache is | |now flushed before each | |indexing batch, so | |preference changes take | |effect on the next indexing | |run without requiring a | |daemon restart. CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 --- Comment #4 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- This implementation looks simple and good enough :-D It adds a cache flush on each commit, meaning we will query sysprefs more often i.e. fetch them from the DB engine cache 99% of the time. I believe it is good enough to save bigger problems :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #5 from Jonathan Druart <jonathan.druart@gmail.com> --- We have C4::Context->disable_syspref_cache(); that is used in misc/batchRebuildItemsTables.pl:C4::Context->disable_syspref_cache() if ( defined( C4::Context->disable_syspref_cache() ) ); But I would not rely on it, flushing L1 feels better here. However I feel like we should also do it in Koha::BackgroundJob->process, but that could be done on a separate bug.
Rather, we need to clear the L1 cache after we fetch a job.
This is what I would do however, it is safer to flush right after we receive the frame or fetch the jobs. Before this 161 my $job = Koha::BackgroundJobs->find( $args->{job_id} ); and before this: 200 @jobs = Koha::BackgroundJobs->search( 201 { status => 'new', queue => 'elastic_index' }, 202 { rows => $batch_size } 203 )->as_list; Don't you think? Also I would remove the ref to the bug number in the comment, what we usually don't do. We have git log for that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 --- Comment #6 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #5)
We have C4::Context->disable_syspref_cache(); that is used in misc/batchRebuildItemsTables.pl:C4::Context->disable_syspref_cache() if ( defined( C4::Context->disable_syspref_cache() ) );
But I would not rely on it, flushing L1 feels better here. However I feel like we should also do it in Koha::BackgroundJob->process, but that could be done on a separate bug.
Rather, we need to clear the L1 cache after we fetch a job.
This is what I would do however, it is safer to flush right after we receive the frame or fetch the jobs.
Before this 161 my $job = Koha::BackgroundJobs->find( $args->{job_id} );
and before this: 200 @jobs = Koha::BackgroundJobs->search( 201 { status => 'new', queue => 'elastic_index' }, 202 { rows => $batch_size } 203 )->as_list;
Don't you think?
I agree
Also I would remove the ref to the bug number in the comment, what we usually don't do. We have git log for that.
Yeah, fair -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199403|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 --- Comment #7 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201488 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201488&action=edit Bug 39769: Flush L1 cache in es_indexer_daemon.pl before each batch The daemon runs as a long-lived process and never clears its L1 cache. System preference changes (e.g. IncludeSeeFromInSearches) are not picked up until the daemon is manually restarted. Flush L1 caches at the start of each commit() call so that preference changes take effect on the next indexing batch without requiring a daemon restart. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201488|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 --- Comment #8 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 202506 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202506&action=edit Bug 39769: Flush L1 cache in es_indexer_daemon.pl before each batch The daemon runs as a long-lived process and never clears its L1 cache. System preference changes (e.g. IncludeSeeFromInSearches) are not picked up until the daemon is manually restarted. Flush L1 caches at the start of each commit() call so that preference changes take effect on the next indexing batch without requiring a daemon restart. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |nick@bywatersolutions.com |y.org | CC| |lisette@bywatersolutions.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #202506|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 --- Comment #9 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 203889 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203889&action=edit Bug 39769: Flush L1 cache in es_indexer_daemon.pl before each batch The daemon runs as a long-lived process and never clears its L1 cache. System preference changes (e.g. IncludeSeeFromInSearches) are not picked up until the daemon is manually restarted. Flush L1 caches at the start of each commit() call so that preference changes take effect on the next indexing batch without requiring a daemon restart. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 --- Comment #10 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Thanks everyone! Pushed to main for 26.11! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|26.11.00 |26.11.00,26.05.05 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39769 --- Comment #11 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to 26.05.x for the upcoming 26.05.05 release. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org