[Bug 31969] New: Options for cleanup_database.pl to remove finished jobs from the background job queue
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Bug ID: 31969 Summary: Options for cleanup_database.pl to remove finished jobs from the background job queue Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: System Administration Assignee: koha-bugs@lists.koha-community.org Reporter: bjorn.nylen@ub.lu.se QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com The background jobs queue will need purging of finished jobs. Bug 31629 focuses on the elastic indexing jobs but the table will fill up with other types of jobs -- 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=31969 Björn Nylén <bjorn.nylen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|koha-bugs@lists.koha-commun |bjorn.nylen@ub.lu.se |ity.org | --- Comment #1 from Björn Nylén <bjorn.nylen@ub.lu.se> --- Created attachment 142603 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142603&action=edit Bug 31969 - Add options for cleanup_database.pl to remove finished jobs from the background job queue -- 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=31969 --- Comment #2 from Björn Nylén <bjorn.nylen@ub.lu.se> --- Bug 31969: Add background queue options to cleanup_database This patch adds background queue options to cleanup_database.pl to allow for purging completed background jobs. --bg-jobs DAYS Purge all finished background jobs this many days old. Defaults to 1 if no DAYS provided. --bg-type TYPE What type of background job to purge. Defaults to "update_elastic_index" if omitted Specifying "all" will purge all types. Repeatable. To test: 1 - Enable elastic search in Koha 2 - perl misc/maintenance/touch_all_items.pl 3 - Generate an number of diffrent types of bg-jobs (eg batch_hold_cancel, batch_biblio_record_deletion, batch_item_record_deletion) 4 - Check db and note there are a bunch of diffrent jobs 5 - Update to make them old UPDATE background_jobs SET enqued_on = '2022-10-01 00:00:00', status='finished' 6 - perl misc/cronjobs/cleanup_database.pl 7 - Note bg-jobs entry shows in help 8 - perl misc/cronjobs/cleanup_database.pl --bg-jobs 1 -v 9 - Note that elasticqueue would have been cleared 10 - perl misc/cronjobs/cleanup_database.pl --bg-jobs 1 -v --confirm 11 - Note that number of entries deleted is reported 12 - Attempt to clear other job types, including "all" eg perl misc/cronjobs/cleanup_database.pl --bg-jobs 1 --bg-type batch_item_record_deletion -v --confirm perl misc/cronjobs/cleanup_database.pl --bg-jobs 1 --bg-type all -v --confirm 13 - Confirm in staff interface that jobs are gone: http://localhost:8081/cgi-bin/koha/admin/background_jobs.pl (Uncheck 'Current jobs only') -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Björn Nylén <bjorn.nylen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=31629 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 31970 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |estherm@sodaspringsid.com --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 31978 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wizzyrea@gmail.com --- Comment #5 from Liz Rea <wizzyrea@gmail.com> --- YES thank you -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 --- Comment #6 from Frédéric Demians <frederic@tamil.fr> --- Thanks Björn for your patch. Have you considered putting the code of your PurgeBackgroundJobs function into Koha::BackgroundJobs as a class method? This could be used from elsewhere, the WUI for example, and from cleanup_database.pl like that: Koha::BackgroundJobs::purge(10, 'all', 1); -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Nick Clemens <nick@bywatersolutions.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=31969 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142603|0 |1 is obsolete| | --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 142668 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142668&action=edit Bug 31969: Add background queue options to cleanup_database This patch adds background queue options to cleanup_database.pl to allow for purging completed background jobs. --bg-jobs DAYS Purge all finished background jobs this many days old. Defaults to 1 if no DAYS provided. --bg-type TYPE What type of background job to purge. Defaults to "update_elastic_index" if omitted Specifying "all" will purge all types. Repeatable. To test: 1 - Enable elastic search in Koha 2 - perl misc/maintenance/touch_all_items.pl 3 - Generate an number of diffrent types of bg-jobs (eg batch_hold_cancel, batch_biblio_record_deletion, batch_item_record_deletion) 4 - Check db and note there are a bunch of diffrent jobs 5 - Update to make them old UPDATE background_jobs SET ended_on = '2022-10-01 00:00:00', status='finished' 6 - perl misc/cronjobs/cleanup_database.pl 7 - Note bg-jobs entry shows in help 8 - perl misc/cronjobs/cleanup_database.pl --bg-jobs 1 -v 9 - Note that elasticqueue would have been cleared 10 - perl misc/cronjobs/cleanup_database.pl --bg-jobs 1 -v --confirm 11 - Note that number of entries deleted is reported 12 - Attempt to clear other job types, including "all" eg perl misc/cronjobs/cleanup_database.pl --bg-jobs 1 --bg-type batch_item_record_deletion -v --confirm perl misc/cronjobs/cleanup_database.pl --bg-jobs 1 --bg-type all -v --confirm 13 - Confirm in staff interface that jobs are gone: http://localhost:8081/cgi-bin/koha/admin/background_jobs.pl (Uncheck 'Current jobs only') 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=31969 --- Comment #8 from Björn Nylén <bjorn.nylen@ub.lu.se> --- (In reply to Frédéric Demians from comment #6)
Thanks Björn for your patch. Have you considered putting the code of your PurgeBackgroundJobs function into Koha::BackgroundJobs as a class method? This could be used from elsewhere, the WUI for example, and from cleanup_database.pl like that:
Koha::BackgroundJobs::purge(10, 'all', 1);
Might be an idea. Easy to do but writing the needed tests is a bit of a pain ( for me at least :) ). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 --- Comment #9 from Björn Nylén <bjorn.nylen@ub.lu.se> --- Created attachment 142699 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142699&action=edit [Follow-up] Move deletion code to new method Koha::BackgroundJobs->purge Follow-up: Move deletion to new method Koha::BackgroundJobs->purge for reuse in other places. Functionality unchanged. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 --- Comment #10 from Björn Nylén <bjorn.nylen@ub.lu.se> --- Created attachment 142700 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142700&action=edit [Follow-up] Test for Koha::BackgroundJobs->purge [Follow-up] Test for Koha::BackgroundJobs->purge prove -v t/db_dependent/Koha/BackgroundJobs.t should pass. Feel free to accept this here or if it should be in a seperate bug? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA 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=31969 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142700|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=31969 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142699|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=31969 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142668|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=31969 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Thx Björn! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 142727 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142727&action=edit Bug 31969: Add background queue options to cleanup_database This patch adds background queue options to cleanup_database.pl to allow for purging completed background jobs. --bg-jobs DAYS Purge all finished background jobs this many days old. Defaults to 1 if no DAYS provided. --bg-type TYPE What type of background job to purge. Defaults to "update_elastic_index" if omitted Specifying "all" will purge all types. Repeatable. To test: 1 - Enable elastic search in Koha 2 - perl misc/maintenance/touch_all_items.pl 3 - Generate an number of diffrent types of bg-jobs (eg batch_hold_cancel, batch_biblio_record_deletion, batch_item_record_deletion) 4 - Check db and note there are a bunch of diffrent jobs 5 - Update to make them old UPDATE background_jobs SET ended_on = '2022-10-01 00:00:00', status='finished' 6 - perl misc/cronjobs/cleanup_database.pl 7 - Note bg-jobs entry shows in help 8 - perl misc/cronjobs/cleanup_database.pl --bg-jobs 1 -v 9 - Note that elasticqueue would have been cleared 10 - perl misc/cronjobs/cleanup_database.pl --bg-jobs 1 -v --confirm 11 - Note that number of entries deleted is reported 12 - Attempt to clear other job types, including "all" eg perl misc/cronjobs/cleanup_database.pl --bg-jobs 1 --bg-type batch_item_record_deletion -v --confirm perl misc/cronjobs/cleanup_database.pl --bg-jobs 1 --bg-type all -v --confirm 13 - Confirm in staff interface that jobs are gone: http://localhost:8081/cgi-bin/koha/admin/background_jobs.pl (Uncheck 'Current jobs only') Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 142728 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142728&action=edit Bug 31969: (follow-up) Move deletion code to Koha::BackgroundJobs->purge for reuse. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 142729 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142729&action=edit Bug 31969: (follow-up) Unit tests for Koha::BackgroundJobs->purge Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |22.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 22.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 --- Comment #16 from Nick Clemens <nick@bywatersolutions.com> --- *** Bug 31629 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32093 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32093 [Bug 32093] cleanup_database.pl bg-jobs parameter should be bg-days -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32304 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32304 [Bug 32304] Subtest purge in BackgroundJobs.t fails when having older jobs -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #17 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Any good reasons to not have used the filter_by_last_update methods for filtering, which is the pattern used all around cleanup_database? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 --- Comment #18 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Since when are we sending a confirm flag to a method? Why isn't this simply reusing the same pattern that exist for other flags? 381 if ($pMessages) { 382 print "Purging messages older than $pMessages days.\n" if $verbose; 383 my $messages = Koha::Patron::Messages->filter_by_last_update( 384 { timestamp_column_name => 'message_date', days => $pMessages } ); 385 my $count = $messages->count; 386 $messages->delete if $confirm; 387 if ( $verbose ) { 388 say $confirm 389 ? sprintf( "Done with purging %d messages", $count ) 390 : sprintf( "%d messages would have been removed", $count ); 391 } 392 } That's all what you needed. No additional methods, unit tests, etc. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 --- Comment #19 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Also, before this is released in 22.11, should not we replace "background" and "bg" with "job"? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 --- Comment #20 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 144189 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144189&action=edit Bug 31969: Use filter_by_last_update This script has a pattern to delete rows depending on a given date/number of days, we should use the filter_by_last_update Koha::Objects method. No need for another method and tests, everything is already tested there. This patch also suggests to rename the reference to "background" and "bg" with "jobs", which seems more appropriate and not an abbreviation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144189|0 |1 is obsolete| | --- Comment #21 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 144190 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144190&action=edit Bug 31969: Use filter_by_last_update This script has a pattern to delete rows depending on a given date/number of days, we should use the filter_by_last_update Koha::Objects method. No need for another method and tests, everything is already tested there. This patch also suggests to rename the reference to "background" and "bg" with "jobs", which seems more appropriate and not an abbreviation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|System Administration |Command-line Utilities CC| |robin@catalyst.net.nz -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 --- Comment #22 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Jonathan Druart from comment #21)
Created attachment 144190 [details] [review] Bug 31969: Use filter_by_last_update
This script has a pattern to delete rows depending on a given date/number of days, we should use the filter_by_last_update Koha::Objects method. No need for another method and tests, everything is already tested there.
This patch also suggests to rename the reference to "background" and "bg" with "jobs", which seems more appropriate and not an abbreviation
Can we move this to a different bug? This has already been released so I think we need to tackle it from a new bug now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 --- Comment #23 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Katrin Fischer from comment #22)
(In reply to Jonathan Druart from comment #21)
Created attachment 144190 [details] [review] [review] Bug 31969: Use filter_by_last_update
This script has a pattern to delete rows depending on a given date/number of days, we should use the filter_by_last_update Koha::Objects method. No need for another method and tests, everything is already tested there.
This patch also suggests to rename the reference to "background" and "bg" with "jobs", which seems more appropriate and not an abbreviation
Can we move this to a different bug? This has already been released so I think we need to tackle it from a new bug now.
The patch has been pushed to master. commit 404c88fe87b27e4b4578e0bb6df2252a0bad9ed1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |minor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31969 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35948 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35948 [Bug 35948] cleanup_database.pl should remove not only finished background jobs -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org