Bug 28610

Summary: Elasticsearch 7 - hits.total is now an object
Product: Koha Reporter: Kevin Carnes <kevin.carnes>
Component: Searching - ElasticsearchAssignee: Kevin Carnes <kevin.carnes>
Status: Pushed to stable --- QA Contact:
Severity: critical    
Priority: P5 - low CC: fridolin.somers, jonathan.druart+koha, julian.maurice, kyle, mtj, nick, victor
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This is one of the changes to have Koha compatible with ElasticSearch 7. This one also causes the full end of compatibility with ElasticSearch 5. Users are advised to upgrade as soon as possible to ElasticSearch 7 since version 5 and 6 are not supported anymore by their developers.
Version(s) released in:
22.05.00,21.11.05
Bug Depends on:    
Bug Blocks: 25439    
Attachments: Bug 28610: Elasticsearch 7 - hits.total is now an object
Bug 28610: Elasticsearch 7 - hits.total is now an object
Bug 28610: Elasticsearch 7 - hits.total is now an object
Bug 28610: Elasticsearch 7 - hits.total is now an object
Bug 28610: Elasticsearch 7 - hits.total is now an object
Bug 28610: Elasticsearch 7 - hits.total is now an object
Bug 28610: Elasticsearch 7 - hits.total is now an object

Description Kevin Carnes 2021-06-22 09:24:51 UTC
In Elasticsearch 7 hits.total is now an object (https://www.elastic.co/guide/en/elasticsearch/reference/7.0/breaking-changes-7.0.html#hits-total-now-object-search-response).

The total is now in $result->{hits}->{total}->{value} instead of $result->{hits}->{total}.

There is also $result->{hits}->{total}->{relation} which specifies if the value is exact or if it's a lower bound. So it's possible to say "Your search returned > 10000 results."

You can always get an exact total by adding a track_total_hits parameter set to true when using the Elasticsearch search method (https://www.elastic.co/guide/en/elasticsearch/reference/7.0/breaking-changes-7.0.html#track-total-hits-10000-default).
Comment 1 Kevin Carnes 2022-02-08 14:45:58 UTC
Created attachment 130291 [details] [review]
Bug 28610: Elasticsearch 7 - hits.total is now an object

In Elasticsearch 7 hits.total is now an object which is not always an exact
value. You can always get an exact total by adding a track_total_hits
parameter set to true when using the Elasticsearch search method

To test:
1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
2) If you observe an error about types, apply patch for bug 25669
3) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
4) Observe that tests with count fail
5) Apply patch
6) Observe that tests with count pass
7) Sign off

Sponsored-by: Lund University Library
Comment 2 Kevin Carnes 2022-02-09 08:44:16 UTC
Created attachment 130343 [details] [review]
Bug 28610: Elasticsearch 7 - hits.total is now an object

In Elasticsearch 7 hits.total is now an object which is not always an exact
value. You can always get an exact total by adding a track_total_hits
parameter set to true when using the Elasticsearch search method

To test:
1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
2) If you observe an error about types, apply patch for bug 25669
3) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
4) Observe that tests with count fail
5) Apply patch
6) Observe that tests with count pass
7) Sign off

Sponsored-by: Lund University Library
Comment 3 Victor Grousset/tuxayo 2022-02-18 14:14:56 UTC
Created attachment 130844 [details] [review]
Bug 28610: Elasticsearch 7 - hits.total is now an object

In Elasticsearch 7 hits.total is now an object which is not always an exact
value. You can always get an exact total by adding a track_total_hits
parameter set to true when using the Elasticsearch search method

To test:
1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
2) If you observe an error about types, apply patch for bug 25669
3) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
4) Observe that tests with count fail
5) Apply patch
6) Observe that tests with count pass
7) Sign off

Sponsored-by: Lund University Library
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 4 Victor Grousset/tuxayo 2022-02-18 14:18:56 UTC
It work on ES 6 and ES 7
Comment 5 Jonathan Druart 2022-02-18 15:37:05 UTC
+    if (version->parse($self->{es_version}) >= version->parse('7.0.0')) {

It's going to be mess very quickly if we are starting to add conditions on the version. Cannot we avoid that?
Comment 6 Jonathan Druart 2022-02-18 15:39:33 UTC
I've removed the condition, and the tests are passing under ES6.
Comment 7 Fridolin Somers 2022-02-18 21:01:41 UTC
(In reply to Jonathan Druart from comment #5)
> +    if (version->parse($self->{es_version}) >= version->parse('7.0.0')) {
> 
> It's going to be mess very quickly if we are starting to add conditions on
> the version. Cannot we avoid that?
+1

This may impact a lot search performance.
Or we should add a cache on parsed ES version.
Comment 8 Kevin Carnes 2022-02-22 12:35:06 UTC
Created attachment 131008 [details] [review]
Bug 28610: Elasticsearch 7 - hits.total is now an object

In Elasticsearch 7 hits.total is now an object which is not always an exact
value. You can always get an exact total by adding a track_total_hits
parameter set to true when using the Elasticsearch search method

To test:
1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
2) If you observe an error about types, apply patch for bug 25669
3) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
4) Observe that tests with count fail
5) Apply patch
6) Observe that tests with count pass
7) Sign off

Sponsored-by: Lund University Library
Comment 9 Kevin Carnes 2022-02-22 12:37:08 UTC
(In reply to Jonathan Druart from comment #6)
> I've removed the condition, and the tests are passing under ES6.

Great, ES6 supports track_total_hits. That makes it much easier.
Comment 10 Katrin Fischer 2022-02-22 22:37:54 UTC
(In reply to Kevin Carnes from comment #9)
> (In reply to Jonathan Druart from comment #6)
> > I've removed the condition, and the tests are passing under ES6.
> 
> Great, ES6 supports track_total_hits. That makes it much easier.

Not sure someone told you yet, but great to see these updates for Elasticsearch 7 going on - thx Kevin!
Comment 11 Julian Maurice 2022-03-07 13:51:55 UTC
Shouldn't 'track_total_hits' option be set in the 'count' subroutine too ?
Comment 12 Kevin Carnes 2022-03-07 14:21:17 UTC
Created attachment 131436 [details] [review]
Bug 28610: Elasticsearch 7 - hits.total is now an object

In Elasticsearch 7 hits.total is now an object which is not always an exact
value. You can always get an exact total by adding a track_total_hits
parameter set to true when using the Elasticsearch search method

To test:
1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
2) If you observe an error about types, apply patch for bug 25669
3) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
4) Observe that tests with count fail
5) Apply patch
6) Observe that tests with count pass
7) Sign off

Sponsored-by: Lund University Library
Comment 13 Kevin Carnes 2022-03-07 14:21:55 UTC
(In reply to Julian Maurice from comment #11)
> Shouldn't 'track_total_hits' option be set in the 'count' subroutine too ?

Yes, thank you for catching that.
Comment 14 Julian Maurice 2022-03-10 08:09:24 UTC
Tested with bug 25669 applied.

It works great with ES 6 and ES 7.
It looks like the version of Search::Elasticsearch (6.x or 7.x) doesn't matter  too much. I tried both and everything went well.
Comment 15 Julian Maurice 2022-03-10 08:09:51 UTC
Created attachment 131542 [details] [review]
Bug 28610: Elasticsearch 7 - hits.total is now an object

In Elasticsearch 7 hits.total is now an object which is not always an exact
value. You can always get an exact total by adding a track_total_hits
parameter set to true when using the Elasticsearch search method

To test:
1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
2) If you observe an error about types, apply patch for bug 25669
3) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
4) Observe that tests with count fail
5) Apply patch
6) Observe that tests with count pass
7) Sign off

Sponsored-by: Lund University Library
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 16 Jonathan Druart 2022-03-21 13:45:29 UTC
-requires 'Search::Elasticsearch', '5.01';
+requires 'Search::Elasticsearch', '6.00';

Why this change in this patch? We certainly should upgrade our minimum version, but it should be done on its own bug report.
Comment 17 Kevin Carnes 2022-03-21 14:53:13 UTC
(In reply to Jonathan Druart from comment #16)
> -requires 'Search::Elasticsearch', '5.01';
> +requires 'Search::Elasticsearch', '6.00';
> 
> Why this change in this patch? We certainly should upgrade our minimum
> version, but it should be done on its own bug report.

6.00 is the minimum version that has track_total_hits in its API for search (look for track_total_hits in https://metacpan.org/release/DRTECH/Search-Elasticsearch-6.00/source/lib/Search/Elasticsearch/Client/6_0/Role/API.pm and https://metacpan.org/release/DRTECH/Search-Elasticsearch-5.02/source/lib/Search/Elasticsearch/Client/5_0/Role/API.pm)
Comment 18 Fridolin Somers 2022-03-21 22:24:58 UTC
As Release Manager, I'm OK to change min version here.
As long as we tell it in release notes.

It is already ES 6 in https://wiki.koha-community.org/wiki/System_requirements_and_recommendations
Comment 19 Victor Grousset/tuxayo 2022-03-22 20:08:11 UTC
It has already been dropped officially with Bug 27252
But actually with syspref ElasticsearchCrossFields off, ES 5 still worked [1] so this time will be the real end of support of ES 5. Hopefully not much instances use it as it's EOL since 2019-03-11 https://www.elastic.co/support/eol

I gave a go at release notes. Tell me if info is missing or if it's not the right writing style.

[1] https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27770#c9
Comment 20 Fridolin Somers 2022-03-23 00:38:57 UTC
Release notes looks great.
Thanks a lot.
Comment 21 Julian Maurice 2022-03-28 09:47:57 UTC
Just tested this with Elasticsearch 8.1.1 (with bug 25669) and it seems to work well :)
Comment 22 Nick Clemens 2022-03-29 11:16:46 UTC
Created attachment 132389 [details] [review]
Bug 28610: Elasticsearch 7 - hits.total is now an object

In Elasticsearch 7 hits.total is now an object which is not always an exact
value. You can always get an exact total by adding a track_total_hits
parameter set to true when using the Elasticsearch search method

To test:
1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
2) If you observe an error about types, apply patch for bug 25669
3) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
4) Observe that tests with count fail
5) Apply patch
6) Observe that tests with count pass
7) Sign off

Sponsored-by: Lund University Library
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 23 Fridolin Somers 2022-04-05 15:18:43 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 24 Kyle M Hall 2022-04-08 14:41:11 UTC
Pushed to 21.11.x for 21.11.05