fix: add opensearch compatibility guardrails - #51
Conversation
| ruby_version: ["3.1"] | ||
| search_image: | ||
| - "docker.elastic.co/elasticsearch/elasticsearch:7.17.18" | ||
| - "opensearchproject/opensearch:2.19.1" |
There was a problem hiding this comment.
question: do we need to maintain the gem on opensearch v2.19.1? We only have v3.3.0, so I think we don't need to support it.
There was a problem hiding this comment.
Good spot! Not at all!
Have we decided to keep maintaining Estella with OpenSearch? I think there's another option worth considering: moving new code into Gravity rather than modifying Estella. Since the gem is just a thin wrapper around ES/OS, that should be achievable. |
95b0d87 to
68cf19d
Compare
Thanks for bringing this up 🙏 @narikazu Yeah, I wasn't sure whether we'd want another repo (Ostella) or even a small wrapper around Gravity. This seemed like the simplest and most common approach, which is why I included it as an option. Thanks for calling it out! |
| ruby_version: ["3.1"] | ||
| search_image: | ||
| - "docker.elastic.co/elasticsearch/elasticsearch:7.17.18" | ||
| - "opensearchproject/opensearch:3.3.0" |
There was a problem hiding this comment.
I'm not clear on how this is working. Does the circle image key take an array and then just run the test
suite against each of the passed in images?
There was a problem hiding this comment.
Exactly. The matrix key generates the cross-product of all parameter values and produces one independent job per combination. search_image is interpolated into the docker: services list at the top of the job:
docker:
- image: "cimg/ruby:<< parameters.ruby_version >>"
- image: "<< parameters.search_image >>" # the engine under test
So each generated job boots a different search engine as the second service container, all reachable on localhost:9200.
There was a problem hiding this comment.
Neat. Didn't know about that feature.
mzikherman
left a comment
There was a problem hiding this comment.
Nice! As long as we keep using estella this makes sense, but also agree it might be worth deciding on the future of estella and if it's worth having it be separate from Gravity or not.
Description
Our OpenSearch cluster is now on v3, but estella's compatibility with it depends entirely on consuming apps pinning the right client version themselves. Three changes to lock in what production already relies on:
Pin the transitive
elasticsearchclient to< 7.14in the gemspec. Clients 7.14+ run a product check that refuses to connect to OpenSearch (elastic/elasticsearch-ruby#1429). Gravity already carries this pin in its own Gemfile; this moves it upstream so no consumer can resolve a broken client viabundle updateor a missing pin. With it, bundler lockselasticsearch 7.13.3alongsideelasticsearch-model 7.2.1.Fix an operator-precedence bug in
DEFAULT_SETTINGS(lib/estella/analysis.rb).defined? Rails && Rails.env == 'test'parses asdefined?((Rails && ...)), which is always truthy, even when Rails isn't loaded. The "test-only"number_of_shards: 1, number_of_replicas: 1settings were therefore applied in every environment, including production. Now they only apply in a Rails test env.Run CI against OpenSearch. The test now covers Elasticsearch 7.17.18 (unchanged), OpenSearch 2.19.1, and OpenSearch 3.3.0 (our live), so CI tests against what production actually runs. If a future OpenSearch version breaks the pinned client, the build goes red instead of production.
Context
OpenSearch 3 removed
compatibility.override_main_response_version(opensearch#18228), so there is no server-side compatibility fallback anymore: everything rests on the client staying below 7.14. That client (7.13.3) is end-of-life, which is fine as a stopgap but not a destination.Note: the two OpenSearch CI jobs are new, so this PR's build is the first real evidence of whether the suite passes against 2.x and 3.x.
Heads-up for consumers
The settings fix changes behavior for models calling bare
searchable do ... endwithout explicit settings outside of tests: new indices get cluster defaults instead of a forced 1 shard / 1 replica. OpenSearch defaults are also 1/1, so nothing changes in practice unless cluster-level index templates say otherwise. In Gravity, all searchable models pass explicit settings exceptVideo, which deserves a small follow-up.Next steps: estella 8.0 on opensearch-ruby or different approach TBD
Slack context here.
The real fix is dropping
elasticsearch-modelandelasticsearchentirely and rebuilding estella's thin integration layer onopensearch-ruby, released as estella 8.0. The surface estella actually uses fromelasticsearch-modelis small:index_namestorage, thesettings/mappingcapture,import, the client proxy, and the hits-to-records loader. That's a few hundred lines to reimplement.Gravity already has
opensearch-ruby3.4.0 for its recommendation and duplicate-detection services, so 8.0 would consolidate two client stacks into one that's already proven in production. It's also the right moment to add alias support (create-with-alias, swap-based reindexing): Gravity's indices are now aliased on the cluster, which breaksreload_index!— see #48 for an earlier attempt at a partial fix. Since the index lifecycle methods get rewritten in 8.0 anyway, that's where the complete fix belongs.