Bug 20261

Summary:	No result in some page in authority search opac and pro (ES)
Product:	Koha	Reporter:	claire.hernandez <claire.hernandez>
Component:	Searching - Elasticsearch	Assignee:	Alex Arnaud <alex.arnaud>
Status:	CLOSED FIXED	QA Contact:
Severity:	major
Priority:	P5 - low	CC:	januszop, jonathan.druart, martin.renvoize, nick, nicolas.legrand, severine.queune
Version:	Main
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	Trivial patch
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Bug Depends on:	17373
Bug Blocks:	21405
Attachments:	Bug 20261 - Fix pagination of the results in authority search Bug 20261 - Fix pagination of the results in authority search Bug 20261 - Fix pagination of the results in authority search

Description claire.hernandez@biblibre.com 2018-02-21 10:33:32 UTC

How I reproduce this issue:

* apply bug 17373 (auth unimarc mapping)
* mappings.pl?op=reset&i_know_what_i_am_doing=1
* reindex autorities - ex: rebuild -a -v -d
* authority search in opac or pro, looking for a term used
* page 1 =>  OK, I see results
* clic on page 2 => OK, I see results
* clic on page 3 => KO, no result
* clic on the lasty page displayed => KO (another), with an error

==> error.log <==
[Wed Feb 21 10:30:11 2018] authorities-home.pl: [Request] ** [http://elastic:80]-[500] {"error":{"root_cause":[{"type":"query_phase_execution_exception","reason":"Result window is too large, from + size must be less than or equal to: [10000] but was [10020]. See the scroll api for a more efficient way to request large data sets. This limit can be set by changing the [index.max_result_window] index level setting."}],"type":"search_phase_execution_exception","reason":"all shards failed","phase":"query","grouped":true,"failed_shards":[{"shard":0,"index":"koha_maedi_authorities","node":"hhR2RjsnSye_jPUnySnMkQ","reason":{"type":"query_phase_execution_exception","reason":"Result window is too large, from + size must be less than or equal to: [10000] but was [10020]. See the scroll api for a more efficient way to request large data sets. This limit can be set by changing the [index.max_result_window] index level setting."}}]},"status":500}, called from sub Search::Elasticsearch::Role::Client::Direct::__ANON__ at /usr/share/perl5/Catmandu/Store/ElasticSearch/Bag.pm line 167. With vars: {'status_code' => 500,'request' => {'method' => 'GET','path' => '/koha_maedi_authorities/data/_search','serialize' => 'std','body' => {'size' => 20,'from' => 10000,'sort' => [{'Heading__sort.phrase' => 'asc'}],'query' => {'bool' => {'should' => [{'match' => {'Heading-Main' => 'de'}}]}}},'qs' => {},'mime_type' => 'application/json','ignore' => []}}
[Wed Feb 21 10:30:11 2018] authorities-home.pl: Unable to perform your search. Please try again.

I don't know if it is a mapping problem or a page management issue in authority search. To be investigated.

Comment 1 Alex Arnaud 2018-03-28 09:06:30 UTC

Created attachment 73375 [details] [review]
Bug 20261 - Fix pagination of the results in authority search

Test plan:

  - apply this patch,
  - do a search that return more than 20 results,
  - click on page 2,
  - check that you get results,
  - check other pages if possible

Comment 2 Nicolas Legrand 2018-03-28 12:22:16 UTC

Created attachment 73381 [details] [review]
Bug 20261 - Fix pagination of the results in authority search

Test plan:

  - apply this patch,
  - do a search that return more than 20 results,
  - click on page 2,
  - check that you get results,
  - check other pages if possible

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

Comment 3 Nick Clemens (kidclamp) 2018-03-30 11:39:16 UTC

Created attachment 73469 [details] [review]
Bug 20261 - Fix pagination of the results in authority search

Test plan:

  - apply this patch,
  - do a search that return more than 20 results,
  - click on page 2,
  - check that you get results,
  - check other pages if possible

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

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

Comment 4 Jonathan Druart 2018-04-05 17:54:34 UTC

Pushed to master for 18.05, thanks to everybody involved!

Comment 5 Janusz Kaczmarek 2018-12-05 17:09:42 UTC

Did you check the impact of the patch on zebra searches? It seems that removing the line:

my $offset = ( $startfrom - 1 ) * $resultsperpage + 1;

leads to improper behavior of Koha when switching to subsequent pages of result set when zebra is used as a search engine: instead of jumping forward every 20 (or whatsoever) entries it moves forward by one entry only.

Comment 6 Janusz Kaczmarek 2018-12-05 17:26:05 UTC

(In reply to Janusz Kaczmarek from comment #5)
> Did you check the impact of the patch on zebra searches? It seems that
> removing the line:
> 
> my $offset = ( $startfrom - 1 ) * $resultsperpage + 1;
> 
> leads to improper behavior of Koha when switching to subsequent pages of
> result set when zebra is used as a search engine: instead of jumping forward
> every 20 (or whatsoever) entries it moves forward by one entry only.

NB. a similar line is to be found in authorities/auth_finder.pl (line 81 in 18.05.06): 

my $offset = $startfrom * $resultsperpage;

and works OK with zebra and search_auth_compat.

The difference is that the pages are counted 1-base in authorities-home.pl and 0-based in auth_finder.pl.

Does this line not harm ES searches?  If not why not unify the mode of operation and make authorities-home.pl 0-based with the initial phrase $offset = $startfrom * $resultsperpage ?  Would it be OK for ES?

Comment 7 Nick Clemens (kidclamp) 2018-12-06 12:52:55 UTC

(In reply to Janusz Kaczmarek from comment #6)
> (In reply to Janusz Kaczmarek from comment #5)
> > Did you check the impact of the patch on zebra searches? It seems that
> > removing the line:
> > 
> > my $offset = ( $startfrom - 1 ) * $resultsperpage + 1;
> > 
> > leads to improper behavior of Koha when switching to subsequent pages of
> > result set when zebra is used as a search engine: instead of jumping forward
> > every 20 (or whatsoever) entries it moves forward by one entry only.
> 
> NB. a similar line is to be found in authorities/auth_finder.pl (line 81 in
> 18.05.06): 
> 
> my $offset = $startfrom * $resultsperpage;
> 
> and works OK with zebra and search_auth_compat.
> 
> The difference is that the pages are counted 1-base in authorities-home.pl
> and 0-based in auth_finder.pl.
> 
> Does this line not harm ES searches?  If not why not unify the mode of
> operation and make authorities-home.pl 0-based with the initial phrase
> $offset = $startfrom * $resultsperpage ?  Would it be OK for ES?

See bugs 19365 and 21405