Skip to content

Commit

Permalink
Bug 19502: Retrieve index.max_result_window from ES
Browse files Browse the repository at this point in the history
This avoid hardcoding '10000' in two different places and allow users to
adjust this setting.

Also, this patch fixes a bug when the search return less than 10000
results

Test plan:
1. Do a search that returns 10000+ records.
2. Note the warning above the pagination buttons
3. Go to the last page, no error
4. Change the ES setting:
   curl -XPUT http://elasticsearch/koha_master_biblios/_settings -d \
     '{"index": {"max_result_window": 20000}}'
5. Do another search that returns more than 10000 but less than 20000
6. Note that the warning does not show up
7. Go to the last page, still no error

Signed-off-by: Nick Clemens <[email protected]>
Signed-off-by: Alex Arnaud <[email protected]>

Signed-off-by: Nick Clemens <[email protected]>
  • Loading branch information
jajm authored and kidclamp committed Jul 12, 2018
1 parent 78b9a3e commit 95b1983
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 4 deletions.
19 changes: 19 additions & 0 deletions Koha/SearchEngine/Elasticsearch/Search.pm
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,25 @@ sub json2marc {
return $marc;
}

sub max_result_window {
my ($self) = @_;

$self->store(
Catmandu::Store::ElasticSearch->new(%{ $self->get_elasticsearch_params })
) unless $self->store;

my $index_name = $self->store->index_name;
my $settings = $self->store->es->indices->get_settings(
index => $index_name,
params => { include_defaults => 1, flat_settings => 1 },
);

my $max_result_window = $settings->{$index_name}->{settings}->{'index.max_result_window'};
$max_result_window //= $settings->{$index_name}->{defaults}->{'index.max_result_window'};

return $max_result_window;
}

=head2 _convert_facets
my $koha_facets = _convert_facets($es_facets, $expanded_facet);
Expand Down
2 changes: 2 additions & 0 deletions Koha/SearchEngine/Zebra/Search.pm
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,6 @@ sub search_auth_compat {
C4::AuthoritiesMarc::SearchAuthorities(@params);
}

sub max_result_window { undef }

1;
3 changes: 2 additions & 1 deletion catalogue/search.pl
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,8 @@ =head3 Additional Notes
## FIXME: add a global function for this, it's better than the current global one
## Build the page numbers on the bottom of the page
my @page_numbers;
my $hits_to_paginate = C4::Context->preference('SearchEngine') eq 'Elasticsearch' ? 10000 : $hits;
my $max_result_window = $searcher->max_result_window;
my $hits_to_paginate = ($max_result_window && $max_result_window < $hits) ? $max_result_window : $hits;
$template->param( hits_to_paginate => $hits_to_paginate );
# total number of pages there will be
my $pages = ceil($hits_to_paginate / $results_per_page);
Expand Down
3 changes: 2 additions & 1 deletion opac/opac-search.pl
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,8 @@ sub _input_cgi_parse {
}
## Build the page numbers on the bottom of the page
my @page_numbers;
my $hits_to_paginate = C4::Context->preference('SearchEngine') eq 'Elasticsearch' ? 10000 : $hits;
my $max_result_window = $searcher->max_result_window;
my $hits_to_paginate = ($max_result_window && $max_result_window < $hits) ? $max_result_window : $hits;
$template->param( hits_to_paginate => $hits_to_paginate );
# total number of pages there will be
my $pages = ceil($hits_to_paginate / $results_per_page);
Expand Down
15 changes: 13 additions & 2 deletions t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@

use Modern::Perl;

use Test::More tests => 13;
use Test::More tests => 15;
use t::lib::Mocks;

use Koha::SearchEngine::Elasticsearch::QueryBuilder;
use Koha::SearchEngine::Elasticsearch::Indexer;


my $builder = Koha::SearchEngine::Elasticsearch::QueryBuilder->new( { index => 'mydb' } );

Expand All @@ -41,9 +43,11 @@ SKIP: {

eval { $builder->get_elasticsearch_params; };

skip 'ElasticSeatch configuration not available', 6
skip 'ElasticSeatch configuration not available', 8
if $@;

Koha::SearchEngine::Elasticsearch::Indexer->new({ index => 'mydb' })->drop_index;

ok( my $results = $searcher->search( $query) , 'Do a search ' );

ok( my $marc = $searcher->json2marc( $results->first ), 'Convert JSON to MARC');
Expand All @@ -56,6 +60,13 @@ SKIP: {

is ( $count = $searcher->count_auth_use($searcher,1), 0, 'Testing count_auth_use');

is ($searcher->max_result_window, 10000, 'By default, max_result_window is 10000');
$searcher->store->es->indices->put_settings(index => $searcher->store->index_name, body => {
'index' => {
'max_result_window' => 12000,
},
});
is ($searcher->max_result_window, 12000, 'max_result_window returns the correct value');
}

subtest 'json2marc' => sub {
Expand Down

0 comments on commit 95b1983

Please sign in to comment.