[Bug 13135] New: cancel_expired_holds.pl/CancelExpiredReserves() logging improvements to catch evasive bugs in production.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13135 Bug ID: 13135 Summary: cancel_expired_holds.pl/CancelExpiredReserves() logging improvements to catch evasive bugs in production. Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: olli-antti.kivilahti@jns.fi QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com We have issues with the Calendar giving bad expiration dates for waiting holds. So far unable to isolate the bug, which appears to appear only in prodcution. This logging improvement gives a lot more information about the cronjob internals in varying levels. (L1) Logs expired holds which have expirationdate overdue. (L1) Logs holds waiting too long, those that did get cancelled and (L2) those that didn't. (L3) Logs the used Calendar-objects content ** * * * ** * TEST PLAN * ** * * * ** 0a. Set the ReservesMaxPickUpDelay-syspref to 6 days. 0b. Create reservations, with expirydate in the past, and in the future, and now. Catch some of those to a borrower. 0c. Create reservations without an expirationdate and catch them to borrowers. In the DB, set the waitingdate 2 days to the past for some of them, 4 days to the past for a portion, 6 days to past for some and 8 days past for the rest. 1. Run the cancel_expired_holds.pl-script with --verbose 3 Observe log entries of all the handled reservations and of all the used Calendars (not all Calendars, simply those that were needed). -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13135 --- Comment #1 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 32619 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32619&action=edit Bug 13135 - cancel_expired_holds.pl/CancelExpiredReserves() logging improvements to catch evasive bugs in production. We have issues with the Calendar giving bad expiration dates for waiting holds. So far unable to isolate the bug, which appears to appear only in prodcution. This logging improvement gives a lot more information about the cronjob internals in varying levels. (L1) Logs expired holds which have expirationdate overdue. (L1) Logs holds waiting too long, those that did get cancelled and (L2) those that didn't. (L3) Logs the used Calendar-objects content ** * * * ** * TEST PLAN * ** * * * ** 0a. Set the ReservesMaxPickUpDelay-syspref to 6 days. 0b. Create reservations, with expirydate in the past, and in the future, and now. Catch some of those to a borrower. 0c. Create reservations without an expirationdate and catch them to borrowers. In the DB, set the waitingdate 2 days to the past for some of them, 4 days to the past for a portion, 6 days to past for some and 8 days past for the rest. 1. Run the cancel_expired_holds.pl-script with --verbose 3 Observe log entries of all the handled reservations and of all the used Calendars (not all Calendars, simply those that were needed). -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13135 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|koha-bugs@lists.koha-commun |olli-antti.kivilahti@jns.fi |ity.org | Severity|enhancement |trivial -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13135 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #2 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Implemented for 3.16.4, not applying to master. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13135 --- Comment #3 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 32619 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32619 Bug 13135 - cancel_expired_holds.pl/CancelExpiredReserves() logging improvements to catch evasive bugs in production. Review of attachment 32619: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=13135&attachment=32619) ----------------------------------------------------------------- ::: C4/Reserves.pm @@ +949,4 @@
=cut
sub CancelExpiredReserves { + my $verbose = shift;
|| 0 would be a good thing to add, so $verbose is always defined. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13135 --- Comment #4 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 32619 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32619 Bug 13135 - cancel_expired_holds.pl/CancelExpiredReserves() logging improvements to catch evasive bugs in production. Review of attachment 32619: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=13135&attachment=32619) ----------------------------------------------------------------- ::: Koha/Calendar.pm @@ +365,5 @@
+sub printMe { + my $self = shift; + my @sb; #String buffer to collect self output. +
Do you have your git configured properly? White space like this is bad. git config --global core.whitespace trailing-space,space-before-tab git config --global apply.whitespace fix ::: misc/cronjobs/holds/cancel_expired_holds.pl @@ +17,4 @@
# with Koha; if not, write to the Free Software Foundation, Inc., # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+use Modern::Perl;
Don't just change to Modern::Perl when both strict and warnings are commented out. Are you sure this doesn't break anything else? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13135 --- Comment #5 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 32619 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32619 Bug 13135 - cancel_expired_holds.pl/CancelExpiredReserves() logging improvements to catch evasive bugs in production. Review of attachment 32619: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=13135&attachment=32619) ----------------------------------------------------------------- Given that 3.16.x is much higher than 3.16.4, is there merit reworking this? Or should it be closed RESOLVED WON'T FIX? ::: C4/Reserves.pm @@ +981,4 @@
my $today = DateTime->now( time_zone => C4::Context->tz() );
+ push @sb, "##Removing holds waiting too long\n##using today=$today, ReservesMaxPickUpDelay=$max_pickup_delay, ExpireReservesMaxPickUpDelayCharge=$charge\n".
The whole today logic was ripped out and replaced with SQL logic in 3.16.x -- not required. @@ +992,5 @@
} CancelReserve({ reserve_id => $res->{'reserve_id'} }); + push @sb, printReserve($res,'tab',['reserve_id','borrowernumber','branchcode','waitingdate']).sprintf("% 14s",substr($expiration,0,10))."| past lastpickupdate.\n" if $verbose; + } + elsif($verbose > 1) {
Because of SQL logic, there is no if statement to elsif off of. @@ +998,2 @@
} + $usedCalendars{ $res->{branchcode} } = Koha::Calendar->new( branchcode => $res->{branchcode} ) if ( $verbose > 2 && not(exists($usedCalendars{$res->{branchcode}})) );
Is this required? -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org