Bug 21974

Summary: cxn_pool must be configurable
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Searching - ElasticsearchAssignee: Ere Maijala <ere.maijala>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: major    
Priority: P5 - low CC: ere.maijala, josef.moravec, lucas, martin.renvoize, nick
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21675
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 22705, 26309    
Attachments: Bug 21974: Make Elasticsearch connection settings configurable
Bug 21974: Make Elasticsearch connection settings configurable
Bug 21974: Make Elasticsearch connection settings configurable

Description Jonathan Druart 2018-12-07 13:16:34 UTC
On Koha::SearchEngine::Elasticsearch:92 Search::Elasticsearch is instantiate with cxn_pool => 'Sniff'

The doc says:

"""
The Sniff connection pool should be used when you do have direct access to the Elasticsearch cluster, eg when your web servers and Elasticsearch servers are on the same network.
"""
https://metacpan.org/pod/Search::Elasticsearch#cxn_pool


We must make this option configurable to allow elastic cluster running on a different network.

Original issue reporter on bug 21675 comment 8, they are connecting to the Amazon Elastic cluster.
Comment 1 Ere Maijala 2018-12-17 14:25:49 UTC
Created attachment 83291 [details] [review]
Bug 21974: Make Elasticsearch connection settings configurable

Default settings are appended in get_elasticsearch_params so that there's a central place for them. Defaults can be overridden in koha-conf.xml.

Test plan:

1. Verify that searches still work with the patch applied.
2. Verify that search fails after adding the following under <elasticsearch> in koha-conf.xml:
<cxn_pool>Static</cxn_pool>
3. Verify that search fails after adding the following under <elasticsearch> in koha-conf.xml:
<cxn_pool>FOO</cxn_pool>
Comment 2 Josef Moravec 2019-03-05 11:03:27 UTC
Created attachment 86052 [details] [review]
Bug 21974: Make Elasticsearch connection settings configurable

Default settings are appended in get_elasticsearch_params so that there's a central place for them. Defaults can be overridden in koha-conf.xml.

Test plan:

1. Verify that searches still work with the patch applied.
2. Verify that search fails after adding the following under <elasticsearch> in koha-conf.xml:
<cxn_pool>Static</cxn_pool>
3. Verify that search fails after adding the following under <elasticsearch> in koha-conf.xml:
<cxn_pool>FOO</cxn_pool>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 3 Martin Renvoize 2019-03-12 12:12:11 UTC
No tests here.. and we've snuck in configuration of not just cxn_pool, but also a couple of other variables.

I'd also like to see these newly exposed variables documented somewhere..

Failing QA for now.
Comment 4 Ere Maijala 2019-03-12 12:17:30 UTC
Documentation is one thing and I can take care of it. I'm not sure how to write any meaningful tests for this, however. Note that in practice the configurability changes nothing unless you actually add the settings to koha-conf.xml.
Comment 5 Martin Renvoize 2019-03-13 11:49:33 UTC
Interesting.. I can't find tests for this module at all.. only the sub-modules.  Perhaps I'm missing something..

Either way.. lets see some minimal documentation added and we can get this moving again.

Thanks Ere.
Comment 6 Ere Maijala 2019-03-13 12:19:49 UTC
I've now added a bit of documentation: https://wiki.koha-community.org/wiki/Elasticsearch#Advanced_Connection_Settings
Comment 7 Martin Renvoize 2019-03-26 07:57:34 UTC
Created attachment 87006 [details] [review]
Bug 21974: Make Elasticsearch connection settings configurable

Default settings are appended in get_elasticsearch_params so that there's a central place for them. Defaults can be overridden in koha-conf.xml.

Test plan:

1. Verify that searches still work with the patch applied.
2. Verify that search fails after adding the following under <elasticsearch> in koha-conf.xml:
<cxn_pool>Static</cxn_pool>
3. Verify that search fails after adding the following under <elasticsearch> in koha-conf.xml:
<cxn_pool>FOO</cxn_pool>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2019-03-26 07:59:05 UTC
OK, So I'm yielding a bit on the tests requirement here as there's not a precedent I can find for testing config checks.

Thanks for the documentation and my apologies for not getting back to this sooner.

Passed QA
Comment 9 Nick Clemens 2019-03-28 13:28:42 UTC
Awesome work all!

Pushed to master for 19.05
Comment 10 Martin Renvoize 2019-04-01 10:42:28 UTC
Pushed to 18.11.x for 18.11.05
Comment 11 Lucas Gass 2019-04-01 16:01:53 UTC
not backporting ES bug fixes/features to 18.05.x
Comment 12 Jonathan Druart 2019-05-13 00:54:55 UTC
On bug 22705 there is a complain about the default value of cxn_pool.