Bugzilla – Attachment 44443 Details for
Bug 15103
Import Borrowers Performance Improvement
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 15103: Import Borrowers Performance Improvement
Bug-15103-Import-Borrowers-Performance-Improvement.patch (text/plain), 8.88 KB, created by
cnorthcott.work
on 2015-11-05 01:11:18 UTC
(
hide
)
Description:
Bug 15103: Import Borrowers Performance Improvement
Filename:
MIME Type:
Creator:
cnorthcott.work
Created:
2015-11-05 01:11:18 UTC
Size:
8.88 KB
patch
obsolete
>From b6e7369bdc1159cd75dd79eead6b6a97b4a4ab5b Mon Sep 17 00:00:00 2001 >From: Catherine <cnorthcott.work@gmail.com> >Date: Tue, 3 Nov 2015 01:38:21 +0000 >Subject: [PATCH] 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. >--- > C4/Members.pm | 89 +++++++++++++++++++++++++++++++-------------- > tools/import_borrowers.pl | 10 +++-- > 2 files changed, 68 insertions(+), 31 deletions(-) > >diff --git a/C4/Members.pm b/C4/Members.pm >index 5fb1764..8ad9063 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -458,14 +458,12 @@ sub GetMember { > $debug && warn $select, " ",values %information; > my $sth = $dbh->prepare("$select"); > $sth->execute(map{$information{$_}} keys %information); >- my $data = $sth->fetchall_arrayref({}); >- #FIXME interface to this routine now allows generation of a result set >- #so whole array should be returned but bowhere in the current code expects this >- if (@{$data} ) { >- return $data->[0]; >- } >- >- return; >+ my $data = $sth->fetchrow_hashref; >+ return $data if $data >+ # FIXME this routine can now support generation of a resultset >+ # but until that is needed only what is expected by current >+ # routines will be generated and returned for the sake of >+ # perfomance. > } > > =head2 GetMemberRelatives >@@ -747,13 +745,14 @@ sub AddMember { > unless ( $data{'dateenrolled'} ) { > $data{'dateenrolled'} = C4::Dates->new()->output("iso"); > } >- >- my $patron_category = $schema->resultset('Category')->find( $data{'categorycode'} ); >+ >+ # Check the member category privacy level >+ my $patron_privacy = GetCategoryPrivacy( $data{'categorycode'} ); > $data{'privacy'} = >- $patron_category->default_privacy() eq 'default' ? 1 >- : $patron_category->default_privacy() eq 'never' ? 2 >- : $patron_category->default_privacy() eq 'forever' ? 0 >- : undef; >+ $patron_privacy eq 'default' ? 1 >+ : $patron_privacy eq 'never' ? 2 >+ : $patron_privacy eq 'forever' ? 0 >+ : undef; > # Make a copy of the plain text password for later use > my $plain_text_password = $data{'password'}; > >@@ -811,17 +810,18 @@ sub Check_Userid { > > return 0 unless ($uid); # userid is a unique column, we should assume NULL is not unique > >- return 0 if ( $uid eq C4::Context->config('user') ); >- >- my $rs = Koha::Database->new()->schema()->resultset('Borrower'); >- >- my $params; >- $params->{userid} = $uid; >- $params->{borrowernumber} = { '!=' => $borrowernumber } if ($borrowernumber); >- >- my $count = $rs->count( $params ); >- >- return $count ? 0 : 1; >+ my $sql = 'SELECT borrowernumber FROM borrowers WHERE userid=?'; >+ $sql .= ' AND borrowernumber != ?' if $borrowernumber; >+ my $dbh = C4::Context->dbh; >+ my $sth = $dbh->prepare($sql); >+ if ($borrowernumber) { >+ $sth->execute( $uid, $borrowernumber ); >+ } >+ else { >+ $sth->execute($uid); >+ } >+ my $data = $sth->fetchrow; >+ return $data ? 0 : 1; > } > > =head2 Generate_Userid >@@ -1318,7 +1318,7 @@ sub checkcardnumber { > return 0 unless defined $cardnumber; > > my $dbh = C4::Context->dbh; >- my $query = "SELECT * FROM borrowers WHERE cardnumber=?"; >+ my $query = "SELECT borrowernumber FROM borrowers WHERE cardnumber=?"; > $query .= " AND borrowernumber <> ?" if ($borrowernumber); > my $sth = $dbh->prepare($query); > $sth->execute( >@@ -1326,7 +1326,7 @@ sub checkcardnumber { > ( $borrowernumber ? $borrowernumber : () ) > ); > >- return 1 if $sth->fetchrow_hashref; >+ return 1 if $sth->fetchrow; > > my ( $min_length, $max_length ) = get_cardnumber_length(); > return 2 >@@ -2535,6 +2535,41 @@ WHERE categorycode = ? AND DATEDIFF( NOW(), dateenrolled ) > ? |; > return $cnt; > } > >+=head2 GetBorrowecategoryHash >+ >+ $arrayref_hashref = &GetBorrowercateforyHash; >+ >+ This function returns a hash of borrower categories where the keys are category >+ codes and values are category descriptions. >+ >+=cut >+ >+sub GetBorrowercategoryHash { >+ my $categories_list = GetBorrowercategoryList(); >+ my %categories_hash = map { $_->{categorycode} => $_->{description} } @{$categories_list}; >+ return %categories_hash; >+} >+ >+=head2 GetCategoryPrivacyHash >+ >+ $hashref = &GetCategoryPrivacyHash >+ Returns the default privacy description of a patron's category given the category code. >+ >+=cut >+ >+sub GetCategoryPrivacy { >+ my ($categorycode) = @_; >+ my $dbh = C4::Context->dbh; >+ my $sth = $dbh->prepare( >+ qq{ >+ SELECT default_privacy >+ FROM categories >+ WHERE categorycode = ? >+ }); >+ $sth->execute($categorycode); >+ return $sth->fetchrow; >+} >+ > =head2 DeleteUnverifiedOpacRegistrations > > Delete all unverified self registrations in borrower_modifications, >diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl >index ad29a66..98e68f8 100755 >--- a/tools/import_borrowers.pl >+++ b/tools/import_borrowers.pl >@@ -40,7 +40,7 @@ use warnings; > use C4::Auth; > use C4::Output; > use C4::Context; >-use C4::Branch qw/GetBranchesLoop GetBranchName/; >+use C4::Branch qw/GetBranchesLoop GetBranches/; > use C4::Members; > use C4::Members::Attributes qw(:all); > use C4::Members::AttributeTypes; >@@ -144,6 +144,8 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { > my $today_iso = output_pref( { dt => dt_from_string, dateonly => 1, dateformat => 'iso' }); > my @criticals = qw(surname branchcode categorycode); # there probably should be others > my @bad_dates; # I've had a few. >+ my $all_branches = GetBranches(); >+ my %all_categories = C4::Members::GetBorrowercategoryHash(); > LINE: while ( my $borrowerline = <$handle> ) { > my %borrower; > my @missing_criticals; >@@ -178,13 +180,13 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { > #warn join(':',%borrower); > if ($borrower{categorycode}) { > push @missing_criticals, {key=>'categorycode', line=>$. , lineraw=>$borrowerline, value=>$borrower{categorycode}, category_map=>1} >- unless GetBorrowercategory($borrower{categorycode}); >+ unless $all_categories{ $borrower{categorycode} } > } else { > push @missing_criticals, {key=>'categorycode', line=>$. , lineraw=>$borrowerline}; > } > if ($borrower{branchcode}) { > push @missing_criticals, {key=>'branchcode', line=>$. , lineraw=>$borrowerline, value=>$borrower{branchcode}, branch_map=>1} >- unless GetBranchName($borrower{branchcode}); >+ unless $all_branches->{ $borrower{branchcode} }->{branchname}; > } else { > push @missing_criticals, {key=>'branchcode', line=>$. , lineraw=>$borrowerline}; > } >@@ -314,7 +316,7 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { > $template->param('lastoverwritten'=>$borrower{'surname'}.' / '.$borrowernumber); > } else { > # FIXME: fixup_cardnumber says to lock table, but the web interface doesn't so this doesn't either. >- # At least this is closer to AddMember than in members/memberentry.pl >+ # At least this is closer to AddMembes $all_categories{ $borrower{categorycode} } than in members/memberentry.pl > if (!$borrower{'cardnumber'}) { > $borrower{'cardnumber'} = fixup_cardnumber(undef); > } >-- >1.7.10.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 15103
:
44443
|
44444
|
44445
|
46625