[Bug 15240] New: Performance issue running overdue_notices.pl
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Bug ID: 15240 Summary: Performance issue running overdue_notices.pl Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: Tools Assignee: gmcharlt@gmail.com Reporter: sophie.meynieux@biblibre.com QA Contact: testopia@bugs.koha-community.org Since introduction of http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12529#add_comment "Overdue notices respect holidays" feature, there is a big problem of performance running overdue_notices.pl For one of our client, where it used to take less than one hour, it now takes more than 7 hours. And this should be rune every day... This is due to date criteria that were removed from SQL queries, so all current issues are parsed 3 times (once for each level of overdue) rather than just overdue issues concerning one given level. Dealing with holidays should be done before quering issues, so that date criteria could be used. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Sophie MEYNIEUX <sophie.meynieux@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=12529 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Sophie MEYNIEUX <sophie.meynieux@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |12529 Referenced Bugs: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12529 [Bug 12529] Overdue notices do not respect holidays -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org, | |martin.renvoize@ptfs-europe | |.com --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Could you give information about the holidays? How many entries do you have in the special_holidays and repeatable_holidays tables? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- We have a system that's really struggling with the same issue. 262 rows in special_holidays 9 rows in repeatable_holidays We also have a system where the preferences are set to ignore the holidays and this runs much more quickly (though still isn't super quick). It has 921 special_holidays and 307 repeatable_holidays. Hope that helps a little. I'm happy to run a profile at the next run of the scripts unless someone else beats me to it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 45524 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45524&action=edit Bug 15240: Do not process issues with a date due later than today There is no need to do this job in Perl, MySQL could do it instead. The idea is to only retrieve the issues info which could be overdued. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |In Discussion --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Sophie, Martin, If you are not using OverdueNoticeCalendar, the number of holidays should not change anything for you actually. It would be great if you could have a look at this patch and compare the execution time, with and without. Do not do it in production, I have not tested this patch enough to be 100% confident :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- That's what I meant by 'system where the preferences are set to ignore the holidays' in comment 2, I just couldn't remember what the pref was called ;) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |15342 Referenced Bugs: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15342 [Bug 15342] Performance 3.22 - Omnibus -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- So it could be related to the number of issues in the DB and this patch could do the job :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 45554 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45554&action=edit Bug 15344: Remove some other calls of GetMemberDetails from pl scripts Same as previously. For these files it's a bit less obvious. To make sure these changes won't introduce any regression, check that the variable returned by GetMember is never used to get something else than a borrower fields. The 'flags' should not be get neither. For opac-user.tt it's different, other keys are got but there are defined in the pl script. On the way: - 'showname' is removed (never used) - fix scope var issue in opac-user.tt (BORROWER_INF.OPACPatronDetails vs OPACPatronDetails) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45554|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Heather Braum <hbraum@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hbraum@nekls.org -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 --- Comment #8 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 45524 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45524 Bug 15240: Do not process issues with a date due later than today Review of attachment 45524: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=15240&attachment=45524) ----------------------------------------------------------------- This eyeballs well. ::: misc/cronjobs/overdue_notices.pl @@ +454,4 @@
AND b.branchcode = items.homebranch AND biblio.biblionumber = biblioitems.biblionumber AND issues.borrowernumber = ? + AND TO_DAYS($date)-TO_DAYS(issues.date_due) >= 0
This makes sure all the filtering based on date is done by MySQL. @@ -500,5 @@
# <date> <itemcount> <firstname> <lastname> <address1> <address2> <address3> <city> <postcode> <country>
- my $borrower_sql = <<'END_SQL'; -SELECT issues.borrowernumber, firstname, surname, address, address2, city, zipcode, country, email, emailpro, B_email, smsalertnumber, phone, cardnumber, -TO_DAYS(?)-TO_DAYS(date_due) as difference, date_due
'difference' field removed because it is moved to part of the WHERE condition. @@ +505,4 @@
FROM issues,borrowers,categories WHERE issues.borrowernumber=borrowers.borrowernumber AND borrowers.categorycode=categories.categorycode +AND TO_DAYS($date)-TO_DAYS(issues.date_due) >= 0
This was the difference field, which strangely was never really used, and by limiting the SQL query here (make SQL do the work), this speeds the script up. Also, the ? in the TO_DAYS was changed to a hard coded $date. Gut feeling: meh, not a big deal. @@ -508,3 @@
END_SQL my @borrower_parameters; - push @borrower_parameters, $date_to_run->datetime();
Removed, because the ? was changed to a hard coded $date. @@ -526,5 @@
my $borrowernumber; while ( my $data = $sth->fetchrow_hashref ) {
- next unless ( DateTime->compare( $date_to_run, dt_from_string($data->{date_due})) ) == 1; -
Perl filtering on a larger data set is slower. Hence the removal of this, when replaced by the SQL grunt work. @@ -616,5 @@
my $j = 0; my $exceededPrintNoticesMaxLines = 0; while ( my $item_info = $sth2->fetchrow_hashref() ) { - next unless ( DateTime->compare( $date_to_run, dt_from_string($item_info->{date_due})) ) == 1; -
Perl filtering on a larger data set is slower. Hence the removal of this, when replaced by the SQL grunt work. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- It's nearing midnight, and there is no express test plan to follow or steps to take to generate the data necessary to sign off. :( -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Test plan: Launch the overdue_notices.pl on a large DB and compare the execution times with and without patch applied. No behavior difference should be noticed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Janet McGowan <janet.mcgowan@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |janet.mcgowan@ptfs-europe.c | |om -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 --- Comment #11 from Heather Braum <hbraum@nekls.org> --- Jonathan, I'm checking with ByWater to see if our rather large test system (running 3.22.1) database could somehow be used to help test the patch as you suggested you needed a large db to do it. I'll let you know what I find out. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45524|0 |1 is obsolete| | --- Comment #12 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 46406 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46406&action=edit Bug 15240: Do not process issues with a date due later than today There is no need to do this job in Perl, MySQL could do it instead. The idea is to only retrieve the issues info which could be overdued. To test: 1/ Run the script 2/ Apply the patch 3/ Run the script again, notice the exact same results but execution time is faster Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Hi Chris, Thanks for testing! Did you compare the execution times before and after the patches? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Pre patch: time misc/cronjobs/overdue_notices.pl -t -itemscontent title,author,itemcallnumber,barcode,replacementprice real 2m9.252s user 2m5.492s sys 0m0.108s Post Patch: time misc/cronjobs/overdue_notices.pl -t -itemscontent title,author,itemcallnumber,barcode,replacementprice real 1m19.965s user 1m12.437s sys 0m0.192s mysql> select COUNT(*) from issues where date_due > now(); +----------+ | COUNT(*) | +----------+ | 480 | +----------+ 1 row in set (0.01 sec) mysql> select COUNT(*) from issues where date_due <= now(); +----------+ | COUNT(*) | +----------+ | 65218 | +----------+ 1 row in set (0.01 sec) I found it quite striking how much of a time difference there was even though the number of checkouts not due yet is quite low so I ran the tests again with very similar results! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Jesse Weaver <jweaver@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jweaver@bywatersolutions.co | |m --- Comment #15 from Jesse Weaver <jweaver@bywatersolutions.com> --- Kyle, Heather, does this changed script yield the same results? It passes my QA, as it just moves the date comparison into MySQL and preserves the --date functionality. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jesse Weaver from comment #15)
Kyle, Heather, does this changed script yield the same results? It passes my QA, as it just moves the date comparison into MySQL and preserves the --date functionality.
I did not think to check that, I was focused on performance testing. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- For us it's terribly important that the functionality remains unchanged and it's not quickly testable in Koha. Please don't rush this. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #17)
For us it's terribly important that the functionality remains unchanged and it's not quickly testable in Koha. Please don't rush this.
Katrin, you are more than welcome to take point on QA for this bug. I wasn't QA'ing it, I was only doing performance testing. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I hope to get around to it soon - but the more testing the better. Overdues are a bit of a complicated matter. We only actively use it with the overdue and notice triggers - if someone else could verify it works the same as before without -t that would be helpful. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | CC| |m.de.rooy@rijksmuseum.nl --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking at this one.. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15240 --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #20)
QA: Looking at this one..
Will continue later (if no one else does). -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org