Bug 15103 - Import Borrowers Performance Improvement
Summary: Import Borrowers Performance Improvement
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: cnorthcott.work
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 7923
  Show dependency treegraph
 
Reported: 2015-11-01 23:39 UTC by cnorthcott.work
Modified: 2017-10-23 14:50 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15103: Import Borrowers Performance Improvement (8.88 KB, patch)
2015-11-05 01:11 UTC, cnorthcott.work
Details | Diff | Splinter Review
CSV file containing patron data for import (222.49 KB, text/plain)
2015-11-05 01:15 UTC, cnorthcott.work
Details
CSV file containing patron data for import (222.77 KB, text/plain)
2015-11-05 01:15 UTC, cnorthcott.work
Details
Bug 15103: Import Borrowers Performance Improvement (8.92 KB, patch)
2016-01-13 23:27 UTC, Rocio Dressler
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description cnorthcott.work 2015-11-01 23:39:42 UTC
Import Borrowers has a long run/load time to import batch of borrowers. I have attempted to improve the runtime of the import borrowers tool and my enhancement reduces the time to import by an average of 22%.

I discovered the extended runtime was mostly due to the intentionally expensive password hashing algorithm used by Koha which is largely unavoidable, but I was able to save some time by switching calls to DBIx::Class::ResultSet to reduce the import of 5000 patrons from 242.2 seconds to 187.8 seconds, saving almost a whole extra minute (54.4 seconds) of database calls.
Comment 1 cnorthcott.work 2015-11-02 00:55:13 UTC
Sorry, looks like I didn't proofread my description very well it should read:

I saved time by switching calls from DBIx::Class::ResultSet to basic SQL to $dbh which reduced the import of 5000 patrons from 242.2 seconds to 187.8 seconds, saving almost a whole extra minute (54.4 seconds) of database calls.
Comment 2 cnorthcott.work 2015-11-05 01:11:18 UTC Comment hidden (obsolete)
Comment 3 cnorthcott.work 2015-11-05 01:15:31 UTC
Created attachment 44444 [details]
CSV file containing patron data for import
Comment 4 cnorthcott.work 2015-11-05 01:15:48 UTC
Created attachment 44445 [details]
CSV file containing patron data for import
Comment 5 Rocio Dressler 2016-01-13 23:13:32 UTC
I tested this on a test site and got these results:

csv1 - 53.75 secs
csv2 - 48.27 secs

I erased all borrowers in the table and tested again:

csv1 - 52.92 sec
csv2 - 47.74 sec

I would recommend that someone else test this to confirm. I used the csv files as-is, with the exception of changing the branch and category codes to match my configuration.
Comment 6 Rocio Dressler 2016-01-13 23:27:33 UTC
Created attachment 46625 [details] [review]
Bug 15103: Import Borrowers Performance Improvement

Improves the performance of importing borrowers in batches from file
by by switching out resultset calls for more direct SQL queries and
using hash lookups instead of repeated DB lookups.

Test Plan:
1) Prepare NTYprof (http://wiki.koha-community.org/wiki/Profiling_with_Devel::NYTProf)
   or procure a stopwatch (stopwatch will be simpler but less accurate).
2) Download the PatronDataCSV1.csv and PatronDataCSV2.csv attachements from
   the bug page or organise your own csv's with data for 1000 or more
   patrons to import.
3) Navigate to Home > Tools > Import Patrons in the Koha Intranet
   (http://demo-intra.mykoha.co.nz/cgi-bin/koha/tools/import_borrowers.pl)
4) Click the "Browse..." button and select PatronDataCSV1.csv or your equilient.
5) If you are using a stopwatch, prepare your stopwatch so that you will
   start counting seconds from the point you click the button in the next
   step.
6) Scroll to the bottom of the page and click the "Import" button (simultaneously
   start your stopwatch if using one)
7) When the page appears showing completion of the import, stop your stopwatch
   and check the time or navigate to the folder you have set NYTProf to output.
8) Record the runtime. This is the pre-optimisation time.
9) Apply this patch.
9) Repeat steps 2-7 of this testplan with the patch applied and use the
   PatronDataCSV2.csv this time around. This will yield the post-optimisation
   time.
10) Compare the pre-optimisation time and post optimisation time. The second
    post-optimisation time should be faster.

Signed-off-by: Rocio Jordan <rocio@bywatersolutions.com>
Comment 7 Jonathan Druart 2016-01-14 11:35:15 UTC
Comment on attachment 46625 [details] [review]
Bug 15103: Import Borrowers Performance Improvement

Review of attachment 46625 [details] [review]:
-----------------------------------------------------------------

::: C4/Members.pm
@@ +736,5 @@
>          $data{'dateenrolled'} = output_pref( { dt => dt_from_string, dateonly => 1, dateformat => 'iso' } );
>      }
>  
> +    # Check the member category privacy level
> +    my $patron_privacy = GetCategoryPrivacy( $data{'categorycode'} );

You are moving backward, we are trying to use DBIx::Class everywhere.
The generation of SQL queries should not be done anymore.
Moreover bug 15407 is going to remove the patron categories code from C4::Members.