From 53cf97bb2dc213936f8787f907fb79d3dc849b0b Mon Sep 17 00:00:00 2001 From: David Gustafsson Date: Tue, 29 May 2018 17:57:31 +0200 Subject: [PATCH] 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 Signed-off-by: Martin Renvoize Signed-off-by: Nick Clemens --- Koha/SearchEngine/Elasticsearch/Indexer.pm | 90 ++++++++++++++----- admin/searchengine/elasticsearch/mappings.pl | 46 +++++++++- ...93_elasticsearch_index_status_sysprefs.sql | 2 + .../searchengine/elasticsearch/mappings.tt | 6 ++ misc/search_tools/rebuild_elastic_search.pl | 5 ++ 5 files changed, 128 insertions(+), 21 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_19893_elasticsearch_index_status_sysprefs.sql diff --git a/Koha/SearchEngine/Elasticsearch/Indexer.pm b/Koha/SearchEngine/Elasticsearch/Indexer.pm index 0b7605a07a..38b3982b48 100644 --- 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 -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 e42cc38e29..d5fef83c38 100755 --- 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 index 0000000000..2412bcbc18 --- /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 490916a4b0..bb2ade3ea6 100644 --- 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 589fe6171a..bed6b5197c 100755 --- 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; -- 2.39.5