Bug 30882 - Add max_result_window to index config
Summary: Add max_result_window to index config
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact: Victor Grousset/tuxayo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-06-02 02:20 UTC by Fridolin Somers
Modified: 2023-06-08 22:26 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This updates the number of results set by default in Elasticsearch for the setting "index.max-result-window" from 10,000 to 1,000,000. This can be useful for really large catalogs.
Version(s) released in:
22.11.00, 22.05.05, 21.11.12


Attachments
Bug 30882: Add max_result_window to index config (1.56 KB, patch)
2022-06-02 06:30 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30882: Add max_result_window to index config (1.80 KB, patch)
2022-06-07 10:32 UTC, David Nind
Details | Diff | Splinter Review
Bug 30882: Add max_result_window to index config (1.85 KB, patch)
2022-07-09 05:09 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30882: (QA follow-up) Fix tests (1.06 KB, patch)
2022-07-18 17:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2022-06-02 02:20:45 UTC
Elasticsearch number of results is by default limited by setting "index.max-result-window", default value is 10000.
https://www.elastic.co/guide/en/elasticsearch/reference/current/index-modules.html#index-max-result-window

We use this setting :
https://git.koha-community.org/Koha-community/Koha/src/commit/44d6528b566e5c16eba9b15f8aa24667293766c3/Koha/SearchEngine/Elasticsearch/Search.pm#L411

I propose we add this setting in index config.
Comment 1 Fridolin Somers 2022-06-02 06:27:05 UTC
This can be usefull for really big catalogs
Comment 2 Fridolin Somers 2022-06-02 06:30:30 UTC
Created attachment 135572 [details] [review]
Bug 30882: Add max_result_window to index config

Elasticsearch number of results is by default limited by setting "index.max-result-window", default value is 10000.
https://www.elastic.co/guide/en/elasticsearch/reference/current/index-modules.html#index-max-result-window

We use this setting :
https://git.koha-community.org/Koha-community/Koha/src/commit/44d6528b566e5c16eba9b15f8aa24667293766c3/Koha/SearchEngine/Elasticsearch/Search.pm#L411

I propose we add this setting in index config.

Test plan :
1) Use Elasticsearch
2) Apply patch and flush memcached
3) Rebuild indexes : misc/search_tools/rebuild_elasticsearch.pl -v -b -d
4) Check the settings of index :
   curl '<cluster>:9200/<myindex>_biblios/_settings?pretty&filter_path=**.max_result_window'
5) You should see :
   "max_result_window" : "1000000"
Comment 3 David Nind 2022-06-05 03:12:55 UTC
Hi Fridolin.

Using koha-testing-docker, how do I get step 4 to work?

The curl command provided does not work for me.

Thanks!

David
Comment 4 Fridolin Somers 2022-06-07 06:31:17 UTC
(In reply to David Nind from comment #3)
> Hi Fridolin.
> 
> Using koha-testing-docker, how do I get step 4 to work?
> 
> The curl command provided does not work for me.
> 
> Thanks!
> 
> David

In KTD, use :
curl 'es:9200/koha_kohadev_biblios/_settings?pretty&filter_path=**.max_result_window'
Comment 5 David Nind 2022-06-07 08:52:33 UTC
(In reply to Fridolin Somers from comment #4)
> In KTD, use :
> curl
> 'es:9200/koha_kohadev_biblios/_settings?pretty&filter_path=**.
> max_result_window'

Thanks Fridolin! That worked after adding this to the es section in koha-testing-docker's docker-compose.yml (after the networks configuration):

        ports:
            - "9200:9300"

However, the result I got was:

        "max_result_window" : "10000"

I think that is because this is the value in the patch (index.max_result_window: 10000)! Should that be index.max_result_window: 1000000? 8-)
Comment 6 Fridolin Somers 2022-06-07 09:46:08 UTC
Ahhh good catch.
Could you fix the test plan when you push your patch ?
Comment 7 David Nind 2022-06-07 09:54:34 UTC
(In reply to Fridolin Somers from comment #6)
> Ahhh good catch.
> Could you fix the test plan when you push your patch ?

No problem!
Comment 8 David Nind 2022-06-07 10:32:39 UTC
Created attachment 135751 [details] [review]
Bug 30882: Add max_result_window to index config

Elasticsearch number of results is by default limited by setting "index.max-result-window", default value is 10000.
https://www.elastic.co/guide/en/elasticsearch/reference/current/index-modules.html#index-max-result-window

We use this setting:
https://git.koha-community.org/Koha-community/Koha/src/commit/44d6528b566e5c16eba9b15f8aa24667293766c3/Koha/SearchEngine/Elasticsearch/Search.pm#L411

I propose we add this setting in index config.

Test plan:
1) Use Elasticsearch
2) Apply patch and flush memcached
3) Rebuild indexes: misc/search_tools/rebuild_elasticsearch.pl -v -b -d
4) Check the settings of index (when using koha-testing-docker*):
   curl 'es:9200/koha_kohadev_biblios/_settings?pretty&filter_path=**.max_result_window'
5) You should see:
   "max_result_window" : "1000000"

* You also need to add this setting to the es section in koha-testing-docker's
docker-compose.yml (after the networks configuration):
     ports:
         - "9200:9300"

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2022-06-07 10:36:33 UTC
I amended the patch to change 10000 to 1000000.

I also amended the commit message to update the test plan - updated the curl statement and added koha-testing-docker information.
Comment 10 Marcel de Rooy 2022-06-07 11:25:55 UTC
(In reply to David Nind from comment #9)
> I amended the patch to change 10000 to 1000000.

Thats a courageous change!
I agree that 10K is too low.
Comment 11 David Nind 2022-06-07 18:45:18 UTC
(In reply to Marcel de Rooy from comment #10)
> Thats a courageous change!

Fridolin is the courageous one!
Comment 12 Victor Grousset/tuxayo 2022-07-09 05:09:01 UTC
Created attachment 137445 [details] [review]
Bug 30882: Add max_result_window to index config

Elasticsearch number of results is by default limited by setting "index.max-result-window", default value is 10000.
https://www.elastic.co/guide/en/elasticsearch/reference/current/index-modules.html#index-max-result-window

We use this setting:
https://git.koha-community.org/Koha-community/Koha/src/commit/44d6528b566e5c16eba9b15f8aa24667293766c3/Koha/SearchEngine/Elasticsearch/Search.pm#L411

I propose we add this setting in index config.

Test plan:
1) Use Elasticsearch
2) Apply patch and flush memcached
3) Rebuild indexes: misc/search_tools/rebuild_elasticsearch.pl -v -b -d
4) Check the settings of index (when using koha-testing-docker*):
   curl 'es:9200/koha_kohadev_biblios/_settings?pretty&filter_path=**.max_result_window'
5) You should see:
   "max_result_window" : "1000000"

* You also need to add this setting to the es section in koha-testing-docker's
docker-compose.yml (after the networks configuration):
     ports:
         - "9200:9300"

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 13 Victor Grousset/tuxayo 2022-07-09 05:12:14 UTC
I don't know what value should we have so I'm following Fridolin and Marcel ^^
Works, qa script happy, code looks good, passing QA :)
Comment 14 Tomás Cohen Arazi 2022-07-18 14:53:44 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 15 Tomás Cohen Arazi 2022-07-18 17:37:11 UTC
Created attachment 137834 [details] [review]
Bug 30882: (QA follow-up) Fix tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Lucas Gass 2022-08-23 17:38:39 UTC
Backported to 22.05.x for 22.05.05
Comment 17 Arthur Suzuki 2022-08-30 09:21:21 UTC
thx pushed to 21.11.x for 21.11.12
Comment 18 Victor Grousset/tuxayo 2022-09-10 13:45:59 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document, marking resolved.