-
Notifications
You must be signed in to change notification settings - Fork 324
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Elasticsearch v2 Compatibility #253
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If performing a "make" on top of an existing working copy, the elasticsearch build process could leave left-over files from the previous version. These extra files could then prevent ElasticSearch from starting up (we saw this happen when upgrading from ElaticSearch 1.7 to 2.3).
Building upon dd938d6 For the terms aggregations, we still do need to escape the regex, however the "^" prefix can no longer be used. It looks like this "includes" argument was already being anchored automatically, so while the "^" prefix was unnecessary in Elasticsearch 1, it worked. In Elasticsearch 2, it now appears as though it must *not* be used (but the query is still anchored as we want). This also adds more tests to the drilldown API to test these changes, as well as more general tests for the drilldown API that we were missing.
The dot field issues with Elasticsearch 2 make upgrading a bit trickier. So for now, revert back to bundling Elasticsearch 1 as the default version, but allow for optional compatibility with Elasticsearch 2 via the "elasticsearch.api_version" configuration option. The only real compatibility issue we need to account for between versions is the use of pre_zone_adjust_large_interval. Otherwise, all the other Elasticsearch v2 compatibility changes work fine under Elasticsearch v1. The other changes in this commit are related to configuration changes so we can more easily run parts of our test suite against Elasticsearch v2 in our CI test suite. Those consist of: - Allow multiple files to be passed to the API_UMBRELLA_CONFIG environment variable to allow further overrides of the test.yml config (so we can pass in a config file for elasticsearch v2 testing). - Rework how the web-app parses config. We're no longer using the "config" gem, since how it merges array is different than the rest of the app (it combined arrays, whereas we normally overwrite when merging). This was causing more problems, and since we weren't using a lot of the Config gem's functionality anyway, we've just replaced it with some manual YAML reading and merging.
Quick heads up: v0.12.0 packages have been released, which includes this update. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This takes @ThibautGery's original Elasticsearch 2 upgrade improvements from #248, and modifies things so API Umbrella can work against either Elasticsearch v1 or v2.
This keeps the version of Elasticsearch we ship with at 1.7, but v2 compatibility can be enabled for an external Elasticsearch instance by adding this configuration option to your
/etc/api-umbrella/api-umbrella.yml
config file:The reason for keeping our default version of Elasticsearch at 1.7 is that upgrading existing installations to 2.0 won't be particularly fun. It's certainly doable, but since you have to reindex your data prior to upgrading, it gets tricky to provide an easy upgrade path for existing API Umbrella users (and if you're not careful with the upgrade, you can end up with an Elasticsearch server that won't start unless you downgrade). It's also particularly tricky to provide an upgrade path if we also have to contend with live data coming in that might also need reindexing.
But I'd still like to upgrade the bundled version to v2 sometime soonish. So my tentative thinking is that we can ship the next version of API Umbrella (v0.12) still with Elasticsearch 1.7 bundled, but with this optional Elasticsearch 2 compatibility that can be enabled in the config file. Then in the following version of API Umbrella (v0.13), we can upgrade the bundled version of Elasticsearch to v2. It will be easier to deal with the data migration at that point, since we at least won't have to worry about live incoming data that needs reindexing (since v0.12 will stop producing data with the problematic dots in field names).
@ThibautGery: Does this sound reasonable to you and give you what you need for Elasticsearch v2 compatibility now? And thanks again for all your help with this!