Bug 28610 - Elasticsearch 7 - hits.total is now an object
Summary: Elasticsearch 7 - hits.total is now an object
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Kevin Carnes
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 25439
  Show dependency treegraph
 
Reported: 2021-06-22 09:24 UTC by Kevin Carnes
Modified: 2022-09-08 03:27 UTC (History)
7 users (show)

See Also:
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,21.05.17


Attachments
Bug 28610: Elasticsearch 7 - hits.total is now an object (2.34 KB, patch)
2022-02-08 14:45 UTC, Kevin Carnes
Details | Diff | Splinter Review
Bug 28610: Elasticsearch 7 - hits.total is now an object (2.87 KB, patch)
2022-02-09 08:44 UTC, Kevin Carnes
Details | Diff | Splinter Review
Bug 28610: Elasticsearch 7 - hits.total is now an object (2.40 KB, patch)
2022-02-18 14:14 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 28610: Elasticsearch 7 - hits.total is now an object (2.49 KB, patch)
2022-02-22 12:35 UTC, Kevin Carnes
Details | Diff | Splinter Review
Bug 28610: Elasticsearch 7 - hits.total is now an object (2.65 KB, patch)
2022-03-07 14:21 UTC, Kevin Carnes
Details | Diff | Splinter Review
Bug 28610: Elasticsearch 7 - hits.total is now an object (2.71 KB, patch)
2022-03-10 08:09 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 28610: Elasticsearch 7 - hits.total is now an object (2.76 KB, patch)
2022-03-29 11:16 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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
Comment 25 Victor Grousset/tuxayo 2022-07-13 22:47:37 UTC
Backported: Pushed to 21.05.x branch for 21.05.17
Nothing to document (not yet full support for ES 7), marking resolved.
Comment 26 Mason James 2022-09-01 12:02:44 UTC
(In reply to Victor Grousset/tuxayo from comment #25)
> Backported: Pushed to 21.05.x branch for 21.05.17
> Nothing to document (not yet full support for ES 7), marking resolved.

hi Victor
can you push this for ES7 support, (i think it was missed)
Comment 27 Victor Grousset/tuxayo 2022-09-08 03:27:24 UTC
> (i think it was missed)

Indeed! :o
Thanks for catching that.
Pushed now.