For the past several years, we have been running on Elasticsearch 0.90.7. We'd like to upgrade to a newer version in order to take advantage of bug fixes, performance enhancements, and security fixes that have been added to newer versions of Elasticsearch.
We would also like to add exclusion options to the filter interface.
More specific details will be provided in the issues belonging to this epic.
I assigned this to myself as it is associated with pull #2700 that I am to review
A couple of general questions and comments:
Why do all the models associated with ElasticSearch still Tire?
used in: bookmark, pseud, tag, work
I would think they'd `include Elasticsearch::Model`
although the following directly uses ES:
app/models/bookmark_search.rb|53 col 16| response = ElasticsearchSimpleClient.perform_count(Bookmark.index_name, 'bookmark', query)
app/models/indexing/index_subqueue.rb|107 col 5| ElasticsearchSimpleClient.send_batch(@batch)
app/models/work_search.rb|75 col 16| response = ElasticsearchSimpleClient.perform_count(Work.index_name, 'work', query)
Some of the Cucumber tests fail with IndexMissingException
I'm resisting the urge to create those indexes.
Note: In my last project where I converted from the Tire to to ElasticSearch gems, creation and population of an index from the model was easy (when all rows went into the ES index:
`Incident._elasticsearch_.create_index! force: true`
When the put to ES was conditional, I'd use callbacks to get/put JSON for associated objects on local setup:
`Scene._elasticsearch_.create_index! force: true`
`Scene.where(updated_at: (DateTime.now-1.year)..(DateTime.now)).update_all(updated_at: DateTime.now.to_s)`
First: The application was looking for ElasticSearch indexes with the prefix name of `otwarchive_`
(see ` curl http://localhost:9200/_aliases`)
but `app/models/search/indexer.rb`'s returned `ao3_` until I temporarily changed it:
Second. After the above "fix" I was getting `Parse Failure [No parser for element [aggs]]`
aggs is short for aggregates. and those are only available starting in ES 1.x while the Vagrant I installed is at 0.90.7
(see `curl http://localhost:9200/`)
So I'm going to be loading ES 2.0 on my Vagrant instance
I'm still not ready to approve this pull request. A visual review of the code shows that the Ruby is well formed. But, as I do with all pull request reviews, I like to run tests against it. My first pass at the tests showed an issue with a no longer supported feature of Elasticsearch query. For that reason I upgrade my copy of Vagrant to use the version of Elasticsearch that is most readily available – 2.3.4. Elasticsearch is currently at 5.1 (it jumped from 2.4 to 5.0.) But the tests still failed. I looked at more detail at the use of the rollout gem in this pull request. The ApplicationController does a use_new_search? on every requests, which uses a rollout facility to check if the current_user is to use the new ES search. The Cucumber tests seem to always have a nil current_user so that test was not of much help in Cucumber. So, to test the new ES search (and being as I only had ES 2.3.5 on my Vagrant) I temporarily hard-coded the check for `$rollout.active?` to true.
But the tests still failed. It failed for a seemingly non-ES-upgrade reason ([bookmarkable] is not a valid type) but I resisted the urge to chase that issue down. When I toggled that hard-coded `$rollout.active?` value to false, it failed for the obvious reason of the use use a bad ES query (unknown search element [facets]).
I have four problems with the pull request:
1) Cucumber tests do not appropriately use `$rollout.activate_user(:use_new_search, <current_user_under_test>)`
2) Cucumber tests, hard-coded to use ES 2.3.4 fail for seemingly non-ES reasons
3) I see nothing in the code to support the use of two ES servers: one for 0.90.7 and one for 2.x or 5.1
4) OTW's Vagrant currently has only ES 0.90.7 and arguably needs 2.x or 5.1 as well
Were I do work further on this task, I would want to first create RSpec tests as I looked for and perhaps enhanced the code and then I'd clean up the Cucumber tests.