From 0baced3c5dccac38b5682483a9ee4ed37ef99a00 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 Signed-off-by: Ere Maijala --- Koha/SearchEngine/Elasticsearch/Indexer.pm | 90 +++++++++++++++++----- admin/searchengine/elasticsearch/mappings.pl | 47 ++++++++++- ...g_19893_elasticsearch_index_status_sysprefs.sql | 2 + .../admin/searchengine/elasticsearch/mappings.tt | 6 ++ misc/search_tools/rebuild_elastic_search.pl | 7 +- 5 files changed, 129 insertions(+), 23 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 0b7605a..38b3982 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 7d63002..85bc408 100755 --- a/admin/searchengine/elasticsearch/mappings.pl +++ b/admin/searchengine/elasticsearch/mappings.pl @@ -22,9 +22,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', @@ -44,6 +47,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; @@ -95,6 +117,7 @@ if ( $op eq 'edit' ) { } else { push @messages, { type => 'message', code => 'success_on_update' }; $schema->storage->txn_commit; + $update_mappings->(); } } elsif( $op eq 'reset' ) { @@ -103,12 +126,34 @@ elsif( $op eq 'reset' ) { if ( $sure ) { Koha::SearchMarcMaps->search->delete; Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings; + $update_mappings->(); } } 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 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 1cb3c41..a78ae15 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 @@ -79,6 +79,12 @@ a.add, a.delete { [% CASE 'error_on_delete' %] An error occurred when deleting the existing mappings. Nothing has been changed! (search field [% m.values.field_name %] with mapping [% m.values.marc_field %].) + [% CASE 'error_on_update_es_mappings' %] + An error occurred when updating Elasticsearch index mappings: [% m.message %]. + [% CASE 'reindex_required' %] + Index "[% m.index %]" needs to be reindexed + [% CASE 'recreate_required' %] + Index "[% m.index %]" needs to be recreated [% CASE 'success_on_update' %] Mapping updated successfully. [% CASE %] diff --git a/misc/search_tools/rebuild_elastic_search.pl b/misc/search_tools/rebuild_elastic_search.pl index 1f620b4..6f381fc 100755 --- a/misc/search_tools/rebuild_elastic_search.pl +++ b/misc/search_tools/rebuild_elastic_search.pl @@ -92,8 +92,6 @@ use MARC::Record; use Modern::Perl; use Pod::Usage; -use Data::Dumper; # TODO remove - my $verbose = 0; my $commit = 5000; my ($delete, $help, $man); @@ -168,6 +166,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.7.4