I noticed that checkins were being processed very slowly and ran some tests. I found that before the patch for Bug 11244 checking in an item which is checked out to a restricted patron took around 2.5 seconds (as measured by Firebug's Net panel). After Bug 11244 the same type of checkin took over 50 seconds. Checking in an item which is checked out to a patron in good standing doesn't seem to be affected.
Very interesting! I assume that your patrons get "check in" emails? We don't do those kinds of emails so I never even thought about that happening... Amazing that it would go from 2.5 seconds to 50 seconds though :S. That's much much higher than the worst time we recorded for generating overdues. Olli-Antti Kivilahti processed 12 messages in 31 seconds. I processed 35 messages in 31 seconds. Astounding that it would be 1 message in 50 seconds. I suppose that regex would be running over every single replaceable field. Does your check in message have a lot of fields to replace with database data? I'm not sure how to fix this one. The regex idea seemed like the best way of dealing with the dateformat issue in notices :S. I thought about using DBIC to check what type of field we were using, but you can't do that because many table names don't translate to DBIC resultset names. DBIC is pretty slow too though, so I don't know. I'm fresh out of ideas on this one :(
(In reply to David Cook from comment #1) > I suppose that regex would be running over every single replaceable field. > Does your check in message have a lot of fields to replace with database > data? Yes, could you give a copy of the template letter used? I have the default one (installer/data/mysql/en/mandatory/sample_notices.sql), I manually added a restriction to a patron, checkin, and it took the same time as before (I didn't measured precisely but not 50sec).
(In reply to Jonathan Druart from comment #2) > Yes, could you give a copy of the template letter used? I have the default one.
Created attachment 33426 [details] time checkin Owen, could you execute this script like: time perl time_checkin.pl and give me the result please? Before, fill the $barcode_to_checkin and $branchcode variable with correct values. I would like to be sure the 50sec are spent in this subroutine. Here it spent 1sec. I check an item out to a patron, enable the CHECKIN notice for this patron and add a restriction on it.
> time perl time_checkin.pl > and give me the result please? real 0m51.725s user 0m50.347s sys 0m0.468s
Created attachment 33485 [details] [review] DO NOT PUSH: profiling for _parseletter
You can also try to put some warns (see this patch).
Created attachment 33494 [details] [review] Bug 13226: 9999-12-31 should not considered as a valid date DateTime::Format::DateParse (called in Koha::DateUtils::dt_from_string) does not manage to parse 9999-12-31 if a time zone is given. my $date = DateTime->new(year => 9999, month => 12, day => 31); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31' ); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31', 'America/Los_Angeles' ); => KO (~20sec on my laptop) It should not be considered as a valid date when the letter is parsed.
Created attachment 33495 [details] [review] Bug 13226: 9999-12-31 should not considered as a valid date DateTime::Format::DateParse (called in Koha::DateUtils::dt_from_string) does not manage to parse 9999-12-31 if a time zone is given. my $date = DateTime->new(year => 9999, month => 12, day => 31); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31' ); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31', 'America/Los_Angeles' ); => KO (~20sec on my laptop) It should not be considered as a valid date when the letter is parsed.
It seems that this patch does not completely fix the problem. I still have a 9999-12-31 somewhere else. But not introduced by bug 11244. Owen could you test it please?
(In reply to Jonathan Druart from comment #10) > It seems that this patch does not completely fix the problem. With this patch I see a significant improvement: Check-ins from the account of a patron who is restricted indefinitely have gone from taking around 50 seconds to between 7 and 10 seconds. It's still slower than the 2-3 seconds for other checkins, but much better.
Created attachment 33496 [details] [review] [SIGNED-OFF] Bug 13226: 9999-12-31 should not considered as a valid date DateTime::Format::DateParse (called in Koha::DateUtils::dt_from_string) does not manage to parse 9999-12-31 if a time zone is given. my $date = DateTime->new(year => 9999, month => 12, day => 31); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31' ); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31', 'America/Los_Angeles' ); => KO (~20sec on my laptop) It should not be considered as a valid date when the letter is parsed. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Note that to reproduce the problem you much be checking in items from an account which has been restricted indefinitely (either manually or by the overdues process). With this patch such checkins go from taking around 50 seconds (in my test system) to around 7 to 10 seconds.
(In reply to Owen Leonard from comment #12) > Note that to reproduce the problem you much be checking in items from an > account which has been restricted indefinitely (either manually or by > the overdues process). With this patch such checkins go from taking > around 50 seconds (in my test system) to around 7 to 10 seconds. Does that mean that check-ins for non-restricted patrons are processed quickly (e.g. 2-3 seconds)?
(In reply to David Cook from comment #13) > (In reply to Owen Leonard from comment #12) > > Note that to reproduce the problem you much be checking in items from an > > account which has been restricted indefinitely (either manually or by > > the overdues process). With this patch such checkins go from taking > > around 50 seconds (in my test system) to around 7 to 10 seconds. > > Does that mean that check-ins for non-restricted patrons are processed > quickly (e.g. 2-3 seconds)? Yes.
Owen, please have a look at bug 13242 to get rid of some other seconds.
Created attachment 33539 [details] [review] [PASSED QA] Bug 13226: 9999-12-31 should not considered as a valid date DateTime::Format::DateParse (called in Koha::DateUtils::dt_from_string) does not manage to parse 9999-12-31 if a time zone is given. my $date = DateTime->new(year => 9999, month => 12, day => 31); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31' ); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31', 'America/Los_Angeles' ); => KO (~20sec on my laptop) It should not be considered as a valid date when the letter is parsed. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Note that to reproduce the problem you much be checking in items from an account which has been restricted indefinitely (either manually or by the overdues process). With this patch such checkins go from taking around 50 seconds (in my test system) to around 7 to 10 seconds. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Good catch! Works as described, no problems found.
Fix pushed to master. Thanks Jonathan!