Summary: | Elasticsearch timeouts when committing import batches | ||
---|---|---|---|
Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
Component: | Tools | Assignee: | Nick Clemens (kidclamp) <nick> |
Status: | Pushed to main --- | QA Contact: | David Cook <dcook> |
Severity: | normal | ||
Priority: | P5 - low | CC: | dcook, mnero, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: |
25.05.00
|
Circulation function: | |
Attachments: |
Bug 39076: Use state variable to avoid creating extra ES connections
Bug 39076: Use state variable to avoid creating extra ES connections Bug 39076: Use state variable to avoid creating extra ES connections |
Description
Nick Clemens (kidclamp)
2025-02-10 13:09:26 UTC
Created attachment 177692 [details] [review] Bug 39076: Use state variable to avoid creating extra ES connections As elsewhere in the code, we should use a state variable for our 'bib_searcher' object to avoid overhead of extra ES connections. Worth noting - we don't actually use the bib_search at all during record auto linking as we pass skipmetadata To test: 0 - Apply patch 1 - Enable biblio auto linking AutoLinkBiblios, CatalogModuleRelink, LinkerRelink 2 - Stage and import a marc file 3 - Confirm record authorities link as expected 4 - Search authorities in the staff interface 5 - Confirm usage counts are shown for results Created attachment 177695 [details] [review] Bug 39076: Use state variable to avoid creating extra ES connections As elsewhere in the code, we should use a state variable for our 'bib_searcher' object to avoid overhead of extra ES connections. Worth noting - we don't actually use the 'bib_searcher' at all during record auto linking as we pass skipmetadata To test: 0 - Apply patch 1 - Enable biblio auto linking AutoLinkBiblios, CatalogModuleRelink, LinkerRelink 2 - Stage and import a marc file 3 - Confirm record authorities link as expected 4 - Search authorities in the staff interface 5 - Confirm usage counts are shown for results Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> It doesn't break current behavior. I tested reindexing, searching and run the SearchEngine related tests. QA scripts happy. So I was tempted to say this isn't really a Koha-ish way of doing things... but it turns out that it is! It's already done heaps in C4/Heading.pm and C4/Matcher.pm Normally, I think that we'd just store the Koha::SearchEngine::Elasticsearch::Search object in $self (as an attribute of Koha::SearchEngine::Search that is), but that would take more code. It might be easier to understand for someone unfamiliar with "state", but it's less elegant for sure. While "state" seems like it could be abused in some scenarios, I think that this one is a pretty reasonable place to use it. Created attachment 177714 [details] [review] Bug 39076: Use state variable to avoid creating extra ES connections As elsewhere in the code, we should use a state variable for our 'bib_searcher' object to avoid overhead of extra ES connections. Worth noting - we don't actually use the 'bib_searcher' at all during record auto linking as we pass skipmetadata To test: 0 - Apply patch 1 - Enable biblio auto linking AutoLinkBiblios, CatalogModuleRelink, LinkerRelink 2 - Stage and import a marc file 3 - Confirm record authorities link as expected 4 - Search authorities in the staff interface 5 - Confirm usage counts are shown for results Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> It passes all my manual and automated tests. If it does cause problems, it'll be easy enough to revert. Pushed for 25.05! Well done everyone, thank you! On a side note, does this fix your prod issue, Nick? |