Bug 27770 - ES: Deprecated aggregation order key [_term] used, replaced by [_key]
Summary: ES: Deprecated aggregation order key [_term] used, replaced by [_key]
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: Bugs List
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 25439
  Show dependency treegraph
 
Reported: 2021-02-24 11:41 UTC by Victor Grousset/tuxayo
Modified: 2022-05-25 03:45 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.03,21.05.13,20.11.17


Attachments
Bug 27770: ES: Deprecated aggregation order key [_term] used, replaced by [_key] (1.96 KB, patch)
2022-02-08 14:43 UTC, Kevin Carnes
Details | Diff | Splinter Review
Bug 27770: ES: Deprecated aggregation order key [_term] used, replaced by [_key] (4.12 KB, patch)
2022-02-09 08:42 UTC, Kevin Carnes
Details | Diff | Splinter Review
Bug 27770: ES: Deprecated aggregation order key [_term] used, replaced by [_key] (2.58 KB, patch)
2022-02-14 08:24 UTC, Kevin Carnes
Details | Diff | Splinter Review
Bug 27770: ES: Deprecated aggregation order key [_term] used, replaced by [_key] (2.64 KB, patch)
2022-02-18 12:33 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Victor Grousset/tuxayo 2021-02-24 11:41:08 UTC
Will be needed for ES 7

prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t

[DEPRECATION] Deprecated aggregation order key [_term] used, replaced by [_key] - In request: {body => {aggregations => {subject => {terms => {field => "subject__facet",include => "[eE][aA][sS][yY].*",order => {_term => "asc"},size => 1000}}},from => 0,query => {query_string => {query => "*"}},size => 0},ignore => [],method => "GET",mime_type => "application/json",path => "/koha_kohadev_mydb/_search",qs => {},serialize => "std"}
Comment 1 Kevin Carnes 2022-02-08 14:43:26 UTC
Created attachment 130290 [details] [review]
Bug 27770: ES: Deprecated aggregation order key [_term] used, replaced by [_key]

Starting in Elasticsearch 6.0 _key should be used instead of _term to order
buckets by their term

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 if you get a deprecation warning about order key
5) Apply patch
6) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
7) Observe no deprecation warning about order key
8) Sign off

Sponsored-by: Lund University Library
Comment 2 Kevin Carnes 2022-02-08 14:51:24 UTC
This solution includes a check for versions prior to Elasticsearch 6. I don't know if there are any plans to drop support for Elasticsearch 5.
Comment 3 Kevin Carnes 2022-02-09 08:42:59 UTC
Created attachment 130342 [details] [review]
Bug 27770: ES: Deprecated aggregation order key [_term] used, replaced by [_key]

Starting in Elasticsearch 6.0 _key should be used instead of _term to order
buckets by their term

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 if you get a deprecation warning about order key
5) Apply patch
6) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
7) Observe no deprecation warning about order key
8) Sign off

Sponsored-by: Lund University Library
Comment 4 Victor Grousset/tuxayo 2022-02-11 19:26:27 UTC
(In reply to Kevin Carnes from comment #2)
> This solution includes a check for versions prior to Elasticsearch 6. I
> don't know if there are any plans to drop support for Elasticsearch 5.

It has been dropped from some time ago, Bug 27252
Comment 5 Kevin Carnes 2022-02-14 08:24:06 UTC
Created attachment 130543 [details] [review]
Bug 27770: ES: Deprecated aggregation order key [_term] used, replaced by [_key]

Starting in Elasticsearch 6.0 _key should be used instead of _term to order
buckets by their term

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 if you get a deprecation warning about order key
5) Apply patch
6) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
7) Observe no deprecation warning about order key
8) Sign off

Sponsored-by: Lund University Library
Comment 6 Victor Grousset/tuxayo 2022-02-18 12:33:49 UTC
Created attachment 130828 [details] [review]
Bug 27770: ES: Deprecated aggregation order key [_term] used, replaced by [_key]

Starting in Elasticsearch 6.0 _key should be used instead of _term to order
buckets by their term

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 if you get a deprecation warning about order key
5) Apply patch
6) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
7) Observe no deprecation warning about order key
8) Sign off

Sponsored-by: Lund University Library
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 7 Victor Grousset/tuxayo 2022-02-18 12:35:33 UTC
It work! :)
Comment 8 Victor Grousset/tuxayo 2022-02-18 12:45:33 UTC
Straightforward fix, nothing suspicious online found about this deprecation, no occurrence of _term remaining, tests passing, QA script happy. 

Passing QA, unless the Release Manager says otherwise ^^
Comment 9 Fridolin Somers 2022-02-22 00:35:53 UTC
(In reply to Victor Grousset/tuxayo from comment #4)
> (In reply to Kevin Carnes from comment #2)
> > This solution includes a check for versions prior to Elasticsearch 6. I
> > don't know if there are any plans to drop support for Elasticsearch 5.
> 
> It has been dropped from some time ago, Bug 27252

When turning off pref ElasticsearchCrossFields you may use ES 5 but requirements page says ES 6 :
https://wiki.koha-community.org/wiki/System_requirements_and_recommendations

I'm OK to drop support to focus on ES 7 compatibility.

And ES 8 is coming :D
Comment 10 Fridolin Somers 2022-02-22 00:36:38 UTC
Maybe only backport to stable 21.11 and not lower.
Comment 11 Fridolin Somers 2022-02-22 01:14:02 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 12 Victor Grousset/tuxayo 2022-02-23 12:54:57 UTC
(In reply to Fridolin Somers from comment #9)
> When turning off pref ElasticsearchCrossFields you may use ES 5 but
> requirements page says ES 6 :
> https://wiki.koha-community.org/wiki/System_requirements_and_recommendations

That seems like a bonus. It doesn't seem to have been mentioned and thus lead to dropping ES5 from the sys reqs. We indeed could have been more subtle in dropping ES5 if ElasticsearchCrossFields off is not a big deal for many Koha instances. Anyway, ES5 was eol for a while at that time.


> Maybe only backport to stable 21.11 and not lower.

Too much downsides IMO. ES6 is eol so if something like the log4j critical security vulnerabilities happen again, ES7 is the only fix.
So down to Koha 19.11, compat is need in order to get back to having an ES supported. (even if ES7 is not longer libre/open source but that's another topic once ES7 compat is sorted out)
Comment 13 Fridolin Somers 2022-02-23 20:08:51 UTC
Sure, get to 19.11 LTS if possible
Comment 14 Kyle M Hall 2022-02-25 13:30:45 UTC
Pushed to 21.11.x for 21.11.03
Comment 15 Andrew Fuerste-Henry 2022-03-07 21:07:04 UTC
(In reply to Fridolin Somers from comment #10)
> Maybe only backport to stable 21.11 and not lower.

Per Frido, not backporting to 21.05.x
Comment 16 Andrew Fuerste-Henry 2022-03-20 14:19:29 UTC
Backported to 21.05.x for 21.05.13.
Comment 17 Victor Grousset/tuxayo 2022-03-20 23:42:23 UTC
Backported: Pushed to 20.11.x branch for 20.11.17
Comment 18 wainuiwitikapark 2022-05-16 04:22:14 UTC
Does this need to be backported to 19.11.x?
Comment 19 Victor Grousset/tuxayo 2022-05-16 17:10:24 UTC
Yes, it has been classified as critical because it's one of the pieces for ES 7 support. Since ES 6 is EOL we are in a precarious situation where any new ES security issue will be unpatchable via updating ES.
Comment 20 wainuiwitikapark 2022-05-18 23:09:29 UTC
Backported to 19.11.x for 19.11.30
Comment 21 wainuiwitikapark 2022-05-25 03:44:20 UTC
This patch doesn't seem to work well on 19.11.x actually.

If it is necessary, please provide a patch for 19.11.x.
Comment 22 wainuiwitikapark 2022-05-25 03:45:47 UTC
I won't actually be looking after 19.11.x anymore though.

As there were no other important patches in the last month, I won't release this month.