Bug 19893: Add index status

Add persistent per index "index status" state to provide useful
user feedback when update of Elasticsearch server mappings fails

Sponsored-by: Gothenburg University Library
Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

diff --git a/Koha/SearchEngine/Elasticsearch/Indexer.pm b/Koha/SearchEngine/Elasticsearch/Indexer.pm
index 0b7605a..38b3982 100644 (file)
--- a/Koha/SearchEngine/Elasticsearch/Indexer.pm
+++ b/Koha/SearchEngine/Elasticsearch/Indexer.pm
@@ -19,6 +19,8 @@ package Koha::SearchEngine::Elasticsearch::Indexer;
 
 use Carp;
 use Modern::Perl;
+use Try::Tiny;
+use List::Util qw(any);
 use base qw(Koha::SearchEngine::Elasticsearch);
 use Data::Dumper;
 
@@ -26,6 +28,9 @@ use Data::Dumper;
 use Catmandu::Importer::MARC;
 use Catmandu::Store::ElasticSearch;
 
+use Koha::Exceptions;
+use C4::Context;
+
 Koha::SearchEngine::Elasticsearch::Indexer->mk_accessors(qw( store ));
 
 =head1 NAME
@@ -57,6 +62,12 @@ If that's a problem, clone them first.
 
 =cut
 
+use constant {
+    INDEX_STATUS_OK => 0,
+    INDEX_STATUS_REINDEX_REQUIRED => 1, # Not currently used, but could be useful later, for example if can detect when new field or mapping added
+    INDEX_STATUS_RECREATE_REQUIRED => 2,
+};
+
 sub update_index {
     my ($self, $biblionums, $records) = @_;
 
@@ -66,7 +77,6 @@ sub update_index {
         $self->_sanitise_records($biblionums, $records);
     }
 
-    $self->ensure_mappings_updated();
     $self->bulk_index($records);
     return 1;
 }
@@ -98,10 +108,45 @@ sub bulk_index {
     return 1;
 }
 
-sub ensure_mappings_updated {
-    my ($self) = @_;
-    unless ($self->{_mappings_updated}) {
-        $self->update_mappings();
+sub index_status_ok {
+    my ($self, $set) = @_;
+    return defined $set ?
+        $self->index_status(INDEX_STATUS_OK) :
+        $self->index_status == INDEX_STATUS_OK;
+}
+
+sub index_status_reindex_required {
+    my ($self, $set) = @_;
+    return defined $set ?
+        $self->index_status(INDEX_STATUS_REINDEX_REQUIRED) :
+        $self->index_status == INDEX_STATUS_REINDEX_REQUIRED;
+}
+
+sub index_status_recreate_required {
+    my ($self, $set) = @_;
+    return defined $set ?
+        $self->index_status(INDEX_STATUS_RECREATE_REQUIRED) :
+        $self->index_status == INDEX_STATUS_RECREATE_REQUIRED;
+}
+
+sub index_status {
+    my ($self, $status) = @_;
+    my $key = 'ElasticsearchIndexStatus_' . $self->index;
+
+    if (defined $status) {
+        unless (any { $status == $_ } (
+                INDEX_STATUS_OK,
+                INDEX_STATUS_REINDEX_REQUIRED,
+                INDEX_STATUS_RECREATE_REQUIRED,
+            )
+        ) {
+            Koha::Exceptions::Exception->throw("Invalid index status: $status");
+        }
+        C4::Context->set_preference($key, $status);
+        return $status;
+    }
+    else {
+        return C4::Context->preference($key);
     }
 }
 
@@ -112,16 +157,23 @@ sub update_mappings {
     my $mappings = $self->get_elasticsearch_mappings();
 
     foreach my $type (keys %{$mappings}) {
-        my $response = $elasticsearch->indices->put_mapping(
-            index => $conf->{index_name},
-            type => $type,
-            body => {
-                $type => $mappings->{$type}
-            }
-        );
-        # TODO: process response, produce errors etc
+        try {
+            my $response = $elasticsearch->indices->put_mapping(
+                index => $conf->{index_name},
+                type => $type,
+                body => {
+                    $type => $mappings->{$type}
+                }
+            );
+        } catch {
+            $self->index_status_recreate_required(1);
+            my $reason = $_[0]->{vars}->{body}->{error}->{reason};
+            Koha::Exceptions::Exception->throw(
+                error => "Unable to update mappings for index \"$conf->{index_name}\". Reason was: \"$reason\". Index needs to be recreated and reindexed",
+            );
+        };
     }
-    $self->{_mappings_updated} = 1;
+    $self->index_status_ok(1);
 }
 
 =head2 $indexer->update_index_background($biblionums, $records)
@@ -181,8 +233,7 @@ sub delete_index_background {
 
 =head2 $indexer->drop_index();
 
-Drops the index from the elasticsearch server. Calling C<update_index>
-after this will recreate it again.
+Drops the index from the elasticsearch server.
 
 =cut
 
@@ -191,8 +242,7 @@ sub drop_index {
     if ($self->index_exists) {
         my $conf = $self->get_elasticsearch_params();
         my $elasticsearch = $self->get_elasticsearch();
-        my $response = $elasticsearch->indices->delete(index => $conf->{index_name});
-        # TODO: Handle response? Convert errors to exceptions/die
+        $elasticsearch->indices->delete(index => $conf->{index_name});
     }
 }
 
@@ -201,13 +251,13 @@ sub create_index {
     my $conf = $self->get_elasticsearch_params();
     my $settings = $self->get_elasticsearch_settings();
     my $elasticsearch = $self->get_elasticsearch();
-    my $response = $elasticsearch->indices->create(
+    $elasticsearch->indices->create(
         index => $conf->{index_name},
         body => {
             settings => $settings
         }
     );
-    # TODO: Handle response? Convert errors to exceptions/die
+    $self->update_mappings();
 }
 
 sub index_exists {

diff --git a/admin/searchengine/elasticsearch/mappings.pl b/admin/searchengine/elasticsearch/mappings.pl
index e42cc38..d5fef83 100755 (executable)
--- a/admin/searchengine/elasticsearch/mappings.pl
+++ b/admin/searchengine/elasticsearch/mappings.pl
@@ -23,9 +23,12 @@ use C4::Output;
 use C4::Auth;
 
 use Koha::SearchEngine::Elasticsearch;
+use Koha::SearchEngine::Elasticsearch::Indexer;
 use Koha::SearchMarcMaps;
 use Koha::SearchFields;
 
+use Try::Tiny;
+
 my $input = new CGI;
 my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
     {   template_name   => 'admin/searchengine/elasticsearch/mappings.tt',
@@ -45,6 +48,25 @@ my $schema   = $database->schema;
 
 my $marc_type = lc C4::Context->preference('marcflavour');
 
+my @index_names = ('biblios', 'authorities');
+
+my $update_mappings = sub {
+    for my $index_name (@index_names) {
+        my $indexer = Koha::SearchEngine::Elasticsearch::Indexer->new({ index => $index_name });
+        try {
+            $indexer->update_mappings();
+        } catch {
+            my $conf = $indexer->get_elasticsearch_params();
+            push @messages, {
+                type => 'error',
+                code => 'error_on_update_es_mappings',
+                message => $_[0],
+                index => $conf->{index_name},
+            };
+        };
+    }
+};
+
 if ( $op eq 'edit' ) {
 
     $schema->storage->txn_begin;
@@ -110,6 +132,7 @@ if ( $op eq 'edit' ) {
     } else {
         push @messages, { type => 'message', code => 'success_on_update' };
         $schema->storage->txn_commit;
+        $update_mappings->();
     }
 }
 elsif( $op eq 'reset_confirmed' ) {
@@ -125,7 +148,28 @@ elsif( $op eq 'reset_confirm' ) {
 
 my @indexes;
 
-for my $index_name (qw| biblios authorities |) {
+for my $index_name (@index_names) {
+    my $indexer = Koha::SearchEngine::Elasticsearch::Indexer->new({ index => $index_name });
+    if (!$indexer->index_status_ok) {
+        my $conf = $indexer->get_elasticsearch_params();
+        if ($indexer->index_status_reindex_required) {
+            push @messages, {
+                type => 'error',
+                code => 'reindex_required',
+                index => $conf->{index_name},
+            };
+        }
+        elsif($indexer->index_status_recreate_required) {
+            push @messages, {
+                type => 'error',
+                code => 'recreate_required',
+                index => $conf->{index_name},
+            };
+        }
+    }
+}
+
+for my $index_name (@index_names) {
     my $search_fields = Koha::SearchFields->search(
         { 'search_marc_map.index_name' => $index_name, 'search_marc_map.marc_type' => $marc_type, },
         {   join => { search_marc_to_fields => 'search_marc_map' },

diff --git a/installer/data/mysql/atomicupdate/bug_19893_elasticsearch_index_status_sysprefs.sql b/installer/data/mysql/atomicupdate/bug_19893_elasticsearch_index_status_sysprefs.sql
new file mode 100644 (file)
index 0000000..2412bcb
--- /dev/null
+++ b/installer/data/mysql/atomicupdate/bug_19893_elasticsearch_index_status_sysprefs.sql
@@ -0,0 +1,2 @@
+INSERT IGNORE INTO systempreferences (variable, value, explanation, options, type) VALUES ('ElasticsearchIndexStatus_biblios', '0', 'Biblios index status', NULL, NULL);
+INSERT IGNORE INTO systempreferences (variable, value, explanation, options, type) VALUES ('ElasticsearchIndexStatus_authorities', '0', 'Authorities index status', NULL, NULL);

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
index 490916a..bb2ade3 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
@@ -81,6 +81,12 @@ a.add, a.delete {
           (search field [% m.values.field_name | html %] with mapping [% m.values.marc_field | html %].)
         [% CASE 'invalid_field_weight' %]
           Invalid field weight "[% m.weight | html %]", must be a positive decimal number.
+        [% CASE 'error_on_update_es_mappings' %]
+          An error occurred when updating Elasticsearch index mappings: [% m.message | html %].
+        [% CASE 'reindex_required' %]
+          Index "[% m.index | html %]" needs to be reindexed
+        [% CASE 'recreate_required' %]
+          Index "[% m.index | html %]" needs to be recreated
         [% CASE 'success_on_update' %]
           Mappings updated successfully.
         [% CASE 'success_on_reset' %]

diff --git a/misc/search_tools/rebuild_elastic_search.pl b/misc/search_tools/rebuild_elastic_search.pl
index 589fe61..bed6b51 100755 (executable)
--- a/misc/search_tools/rebuild_elastic_search.pl
+++ b/misc/search_tools/rebuild_elastic_search.pl
@@ -168,6 +168,11 @@ sub do_reindex {
     elsif (!$indexer->index_exists()) {
         # Create index if does not exist
         $indexer->create_index();
+    } elsif ($indexer->index_status_ok) {
+        # Update mapping unless index is some kind of problematic state
+        $indexer->update_mappings();
+    } elsif ($indexer->index_status_recreate_required) {
+        warn qq/Index "$index_name" has status "recreate required", suggesting it should be recreated/;
     }
 
     my $count        = 0;