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 <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/C4/Items.pm b/C4/Items.pm
index 9480808..c69e9d0 100644 (file)
--- 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<additem.pl> 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 70819cc..a6ad7c6 100644 (file)
--- 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 ec27c1a..1a7e938 100755 (executable)
--- 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 {