From 3eec846d7f49c39cb4b4079818eeb42268aca769 Mon Sep 17 00:00:00 2001 From: Andrew Nugged Date: Mon, 6 Apr 2020 19:11:35 +0300 Subject: [PATCH] Bug 24027: Call ModZebra once after all items added/deleted in a batch Issue description: - call to ModZebra was unconditional inside 'store' method for Koha::Item, so it was after each item added, or deleted. - ModZebra called with param biblionumber, so it is the same parameter across calls for each items with same biblionumber, especially when we adding/removing in a batch. - with ElasticSearch enabled this makes even more significant load and it is also progressively grows when more items already in DB Solution: - to add extra parameter 'skip_modzebra_update' and propagate it down to 'store' method call to prevent call of ModZebra, - but to call ModZebra once after the whole batch loop in the upper layer Test plan / how to replicate: - make sure that you have in the admin settings "SearchEngine" set to "Elasticsearch" and your ES is configured and working ( /cgi-bin/koha/admin/preferences.pl?op=search&searchfield=SearchEngine ) - select one of biblioitems without items ( /cgi-bin/koha/cataloguing/additem.pl?biblionumber=XXX ) - press button "add multiple copies of this item", - enter 200 items, start measuring time and submit the page/form... On my test machine when adding 200 items 3 times in a row (so 600 in total, but to show that time grows with every next batch gradually): WHEN ElasticSearch DISABLED (only Zebra queue): - 9s, 12s, 13s WHEN ElasticSearch ENABLED: - 1.3m, 3.2m, 4.8m WITH PATCH WHEN ElasticSearch ENABLED: - 10s, 13s, 15s Same slowness (because also same call to ModZebra) happens when you try to delete all items ("op=delallitems"). And same fix. Signed-off-by: Tomas Cohen Arazi Signed-off-by: Jonathan Druart Signed-off-by: Martin Renvoize --- C4/Items.pm | 17 ++++++++++++++--- Koha/Item.pm | 20 +++++++++++++------- cataloguing/additem.pl | 9 +++++++-- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/C4/Items.pm b/C4/Items.pm index 94808084ac..c69e9d0575 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -137,15 +137,26 @@ sub CartToShelf { =head2 AddItemFromMarc my ($biblionumber, $biblioitemnumber, $itemnumber) - = AddItemFromMarc($source_item_marc, $biblionumber); + = AddItemFromMarc($source_item_marc, $biblionumber[, $params]); Given a MARC::Record object containing an embedded item record and a biblionumber, create a new item record. +The final optional parameter, C<$params>, expected to contain +'skip_modzebra_update' key, which relayed down to Koha::Item/store, +there it prevents calling of ModZebra (and Elasticsearch update), +which takes most of the time in batch adds/deletes: ModZebra better +to be called later in C after the whole loop. + +$params: + skip_modzebra_update => true / false =cut sub AddItemFromMarc { - my ( $source_item_marc, $biblionumber ) = @_; + my $source_item_marc = shift; + my $biblionumber = shift; + my $params = @_ ? shift : {}; + my $dbh = C4::Context->dbh; # parse item hash from MARC @@ -161,7 +172,7 @@ sub AddItemFromMarc { $item_values->{biblionumber} = $biblionumber; $item_values->{cn_source} = delete $item_values->{'items.cn_source'}; # Because of C4::Biblio::_disambiguate $item_values->{cn_sort} = delete $item_values->{'items.cn_sort'}; # Because of C4::Biblio::_disambiguate - my $item = Koha::Item->new( $item_values )->store; + my $item = Koha::Item->new( $item_values )->store({ skip_modzebra_update => $params->{skip_modzebra_update} }); return ( $item->biblionumber, $item->biblioitemnumber, $item->itemnumber ); } diff --git a/Koha/Item.pm b/Koha/Item.pm index 70819cc96b..a6ad7c6ac3 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -61,7 +61,8 @@ Koha::Item - Koha Item object class =cut sub store { - my ($self, $params) = @_; + my $self = shift; + my $params = @_ ? shift : {}; my $log_action = $params->{log_action} // 1; @@ -99,7 +100,8 @@ sub store { $self->cn_sort($cn_sort); } - C4::Biblio::ModZebra( $self->biblionumber, "specialUpdate", "biblioserver" ); + C4::Biblio::ModZebra( $self->biblionumber, "specialUpdate", "biblioserver" ) + unless $params->{skip_modzebra_update}; logaction( "CATALOGUING", "ADD", $self->itemnumber, "item" ) if $log_action && C4::Context->preference("CataloguingLog"); @@ -156,7 +158,8 @@ sub store { $self->permanent_location( $self->location ); } - C4::Biblio::ModZebra( $self->biblionumber, "specialUpdate", "biblioserver" ); + C4::Biblio::ModZebra( $self->biblionumber, "specialUpdate", "biblioserver" ) + unless $params->{skip_modzebra_update}; $self->_after_item_action_hooks({ action => 'modify' }); @@ -176,12 +179,14 @@ sub store { =cut sub delete { - my ( $self ) = @_; + my $self = shift; + my $params = @_ ? shift : {}; # FIXME check the item has no current issues # i.e. raise the appropriate exception - C4::Biblio::ModZebra( $self->biblionumber, "specialUpdate", "biblioserver" ); + C4::Biblio::ModZebra( $self->biblionumber, "specialUpdate", "biblioserver" ) + unless $params->{skip_modzebra_update}; $self->_after_item_action_hooks({ action => 'delete' }); @@ -196,14 +201,15 @@ sub delete { =cut sub safe_delete { - my ($self) = @_; + my $self = shift; + my $params = @_ ? shift : {}; my $safe_to_delete = $self->safe_to_delete; return $safe_to_delete unless $safe_to_delete eq '1'; $self->move_to_deleted; - return $self->delete; + return $self->delete($params); } =head3 safe_to_delete diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl index ec27c1a426..1a7e9385eb 100755 --- a/cataloguing/additem.pl +++ b/cataloguing/additem.pl @@ -596,7 +596,8 @@ if ($op eq "additem") { # Adding the item if (!$exist_itemnumber) { - my ($oldbiblionumber,$oldbibnum,$oldbibitemnum) = AddItemFromMarc($record,$biblionumber); + my ( $oldbiblionumber, $oldbibnum, $oldbibitemnum ) = + AddItemFromMarc( $record, $biblionumber, { skip_modzebra_update => 1 } ); set_item_default_location($oldbibitemnum); # We count the item only if it was really added @@ -610,6 +611,9 @@ if ($op eq "additem") { # Preparing the next iteration $oldbarcode = $barcodevalue; } + + C4::Biblio::ModZebra( $biblionumber, "specialUpdate", "biblioserver" ); + undef($itemrecord); } } @@ -682,10 +686,11 @@ if ($op eq "additem") { #------------------------------------------------------------------------------- my $items = Koha::Items->search({ biblionumber => $biblionumber }); while ( my $item = $items->next ) { - $error = $item->safe_delete; + $error = $item->safe_delete({ skip_modzebra_update => 1 }); next if $error eq '1'; # Means ok push @errors,$error; } + C4::Biblio::ModZebra( $biblionumber, "specialUpdate", "biblioserver" ); if ( @errors ) { $nextop="additem"; } else { -- 2.39.5