[Bug 42540] New: Trap and throw elastic too_many_clauses errors instead of just dying
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 Bug ID: 42540 Summary: Trap and throw elastic too_many_clauses errors instead of just dying Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org This started out as an issue with Connexion gateway, but it's not actually directly related. The issue is that during import, a record may contain so many fields to match on that it exceeds the max clauses an es server is configured for. In this case Koha 500's and connex emits the generic "Unsuccessful request" error. It seems the best patch forward would be to trap this error and propagate it as a Koha Exception, rather than simply die. There may be other elastic errors we could add to this pattern but I've not encountered any others so far. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #1 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 198687 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198687&action=edit Bug 42540: Trap and throw elastic too_many_clauses errors instead of just dying This started out as an issue with Connexion gateway, but it's not actually directly related. The issue is that during import, a record may contain so many fields to match on that it exceeds the max clauses an es server is configured for. In this case Koha 500's and connex emits the generic "Unsuccessful request" error. It seems the best patch forward would be to trap this error and propagate it as a Koha Exception, rather than simply die. There may be other elastic errors we could add to this pattern but I've not encountered any others so far. Test Plan: 1) Ensure your Koha instance is configured to use Elastic, and the maxClauseCount is set to 1024 or less 2) Apply this patch 3) Prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #2 from David Nind <david@davidnind.com> --- No idea how to do step 1 using KTD. Some hints on how to do that using KTD would be helpful 8-). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #3 from David Nind <david@davidnind.com> --- Also, is Bug 42174 a duplicate, or just related to this issue? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com --- Comment #4 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- *** Bug 42174 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198687|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #5 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 202886 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202886&action=edit Bug 42540: Add TooManyClauses exception -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #6 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 202887 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202887&action=edit Bug 42540: Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #7 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 202888 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202888&action=edit Bug 42540: Trap and throw elastic too_many_clauses errors instead of just dying This started out as an issue with Connexion gateway, but it's not actually directly related. The issue is that during import, a record may contain so many fields to match on that it exceeds the max clauses an es server is configured for. In this case Koha 500's and connex emits the generic "Unsuccessful request" error. It seems the best patch forward would be to trap this error and propagate it as a Koha Exception, rather than simply die. There may be other elastic errors we could add to this pattern but I've not encountered any others so far. Test Plan: 1) Ensure your Koha instance is configured to use Elastic, and the maxClauseCount is set to 1024 or less (You can spin up a KTD with es7 which uses this as default) 2) Apply the first two patches (exceptions and unit tests) 3) Prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 4) It fails 5) Apply this patch 6) Prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 7) It succeeds! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #202886|0 |1 is obsolete| | Attachment #202887|0 |1 is obsolete| | Attachment #202888|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 202892 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202892&action=edit Bug 42540: Add TooManyClauses exception Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #9 from David Nind <david@davidnind.com> --- Created attachment 202893 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202893&action=edit Bug 42540: Unit tests Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #10 from David Nind <david@davidnind.com> --- Created attachment 202894 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202894&action=edit Bug 42540: Trap and throw elastic too_many_clauses errors instead of just dying This started out as an issue with Connexion gateway, but it's not actually directly related. The issue is that during import, a record may contain so many fields to match on that it exceeds the max clauses an es server is configured for. In this case Koha 500's and connex emits the generic "Unsuccessful request" error. It seems the best patch forward would be to trap this error and propagate it as a Koha Exception, rather than simply die. There may be other elastic errors we could add to this pattern but I've not encountered any others so far. Test Plan: 1) Ensure your Koha instance is configured to use Elastic, and the maxClauseCount is set to 1024 or less (You can spin up a KTD with es7 which uses this as default) 2) Apply the first two patches (exceptions and unit tests) 3) Prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 4) It fails 5) Apply this patch 6) Prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 7) It succeeds! Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #11 from David Nind <david@davidnind.com> --- Thanks Nick! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |dcook@prosentient.com.au |y.org | CC| |lisette@bywatersolutions.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #12 from David Cook <dcook@prosentient.com.au> --- Test plan works as described and sounds good overall. Ideally, it would be nice to have the svc/import_bib endpoint tested too in the test plan. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #13 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 202894 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202894 Bug 42540: Trap and throw elastic too_many_clauses errors instead of just dying Review of attachment 202894: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=42540&attachment=202894) ----------------------------------------------------------------- ::: svc/import_bib @@ +97,5 @@
+ my $number_of_matches; + try { + $number_of_matches = BatchFindDuplicates( $batch_id, $matcher ); + } catch { + if ( blessed $_ and $_->isa('Koha::Exceptions::Elasticsearch::TooManyClauses') ) {
The blessed here is bugging me... If you tried to run svc/import_bib in CGI mode it would throw an error like this: Can't locate object method "blessed" via package "CGI" I'm guessing it might work under Plack because some package loaded in plack.psgi is importing "blessed" into the "main" namespace, but... that's a recipe for disaster in the long term, so I think it would be necessary to add 'use Scalar::Util qw( blessed );' to the top of the file... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #14 from David Cook <dcook@prosentient.com.au> --- Created attachment 203326 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203326&action=edit Bug 42540: Trap and throw elastic too_many_clauses errors instead of just dying This started out as an issue with Connexion gateway, but it's not actually directly related. The issue is that during import, a record may contain so many fields to match on that it exceeds the max clauses an es server is configured for. In this case Koha 500's and connex emits the generic "Unsuccessful request" error. It seems the best patch forward would be to trap this error and propagate it as a Koha Exception, rather than simply die. There may be other elastic errors we could add to this pattern but I've not encountered any others so far. Test Plan: 1) Ensure your Koha instance is configured to use Elastic, and the maxClauseCount is set to 1024 or less (You can spin up a KTD with es7 which uses this as default) 2) Apply the first two patches (exceptions and unit tests) 3) Prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 4) It fails 5) Apply this patch 6) Prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 7) It succeeds! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #15 from David Cook <dcook@prosentient.com.au> --- Created attachment 203327 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203327&action=edit Bug 42540: Make import on "blessed" explicit Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #203326|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #203327|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #202892|0 |1 is obsolete| | Attachment #202893|0 |1 is obsolete| | Attachment #202894|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #16 from David Cook <dcook@prosentient.com.au> --- Created attachment 203328 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203328&action=edit Bug 42540: Add TooManyClauses exception Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #17 from David Cook <dcook@prosentient.com.au> --- Created attachment 203329 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203329&action=edit Bug 42540: Unit tests Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #18 from David Cook <dcook@prosentient.com.au> --- Created attachment 203330 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203330&action=edit Bug 42540: Trap and throw elastic too_many_clauses errors instead of just dying This started out as an issue with Connexion gateway, but it's not actually directly related. The issue is that during import, a record may contain so many fields to match on that it exceeds the max clauses an es server is configured for. In this case Koha 500's and connex emits the generic "Unsuccessful request" error. It seems the best patch forward would be to trap this error and propagate it as a Koha Exception, rather than simply die. There may be other elastic errors we could add to this pattern but I've not encountered any others so far. Test Plan: 1) Ensure your Koha instance is configured to use Elastic, and the maxClauseCount is set to 1024 or less (You can spin up a KTD with es7 which uses this as default) 2) Apply the first two patches (exceptions and unit tests) 3) Prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 4) It fails 5) Apply this patch 6) Prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 7) It succeeds! Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #19 from David Cook <dcook@prosentient.com.au> --- Created attachment 203331 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203331&action=edit Bug 42540: Add explicit "blessed" import Patch from commit 87833c9 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #21 from David Cook <dcook@prosentient.com.au> --- (In reply to Pedro Amorim (ammopt) from comment #20)
Hi guys, Search.t fails for me running es8:
prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
Can or should we make this test not rely on ElasticSearch version?
What's the failure message when you use "prove -v"? I don't think we should make the test rely on ES version but rather address the underlying issue and make sure we can support multiple ES versions. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #22 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- (In reply to David Cook from comment #21)
(In reply to Pedro Amorim (ammopt) from comment #20)
Hi guys, Search.t fails for me running es8:
prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
Can or should we make this test not rely on ElasticSearch version?
What's the failure message when you use "prove -v"? I don't think we should make the test rely on ES version but rather address the underlying issue and make sure we can support multiple ES versions.
ok 14 - _convert_facets # Subtest: search() throws TooManyClauses on oversized bool query 1..1 not ok 1 - Oversized bool query throws TooManyClauses # Failed test 'Oversized bool query throws TooManyClauses' # at t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t line 186. # expecting: Koha::Exceptions::Elasticsearch::TooManyClauses # found: normal exit # Looks like you failed 1 test of 1. not ok 15 - search() throws TooManyClauses on oversized bool query # Failed test 'search() throws TooManyClauses on oversized bool query' # at t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t line 188. ok 16 - no warnings # Looks like you failed 1 test of 16. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/16 subtests Test Summary Report ------------------- t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t (Wstat: 256 (exited 1) Tests: 16 Failed: 1) Failed test: 15 Non-zero exit status: 1 Files=1, Tests=16, 3 wallclock secs ( 0.02 usr 0.01 sys + 1.94 cusr 0.29 csys = 2.26 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42540 --- Comment #23 from David Cook <dcook@prosentient.com.au> --- (In reply to Pedro Amorim (ammopt) from comment #20)
Hi guys, Search.t fails for me running es8:
prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
Can or should we make this test not rely on ElasticSearch version?
Thanks for providing that output. I see what you mean now... in Elasticsearch 7 there was a setting indices.query.bool.max_clause_count which defaulted to 1024. But that setting was deprecated in Elasticsearch 8 and the limit gets set dynamically... https://www.elastic.co/guide/en/elasticsearch/reference/7.17/search-settings... https://www.elastic.co/docs/reference/elasticsearch/configuration-reference/... So... we'll need to mock the exception... probably using a mock for Search::Elasticsearch or a mock function for Koha::SearchEngine::Elasticsearch->get_elasticsearch so that we get an object with a search method that reliably throws the exception when running '$searcher->search($query)'. Nice one, Pedro. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org