Skip to content
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

Remove obsolete index setting index.version.minimum_compatible. #23593

Merged
merged 1 commit into from
Mar 27, 2017
Merged

Remove obsolete index setting index.version.minimum_compatible. #23593

merged 1 commit into from
Mar 27, 2017

Conversation

matriv
Copy link
Contributor

@matriv matriv commented Mar 15, 2017

I discovered this obsolete setting with the following:
Create a data dir with an index created with ES 2.4.2.
Start ES 5.x with this data dir and it upgrades the index.
Use the upgrade segments functionality to upgrade the lucene segments of the index to the current lucene version.
Restart the ES 5.x node -> then the following assertion is failing:
https://github.com/crate/elasticsearch/blob/master/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java#L77

because the setting is not added here: https://github.com/crate/elasticsearch/blob/master/core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java#L186

After searching a bit I saw that the setting is actually not really used...

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@jasontedor
Copy link
Member

@matriv Can you please update the body of the PR to provide some context for why you're proposing this change?

@matriv
Copy link
Contributor Author

matriv commented Mar 15, 2017

@jasontedor Updated description

@s1monw
Copy link
Contributor

s1monw commented Mar 15, 2017

@elasticmachine ok to test

Copy link
Contributor

@s1monw s1monw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM provided tests pass

@matriv
Copy link
Contributor Author

matriv commented Mar 27, 2017

@s1monw Just a reminder, that it's not yet merged ;-)

@s1monw s1monw merged commit 4f694a3 into elastic:master Mar 27, 2017
@s1monw
Copy link
Contributor

s1monw commented Mar 27, 2017

@matriv why do you run with -ea in production?

jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Mar 28, 2017
* master: (2923 commits)
  Add 5.3.0 version for packaging tests
  Require explicit query in _delete_by_query API (elastic#23632)
  Adds boolean similarity to Elasticsearch (elastic#23637)
  Revert "Skip 5.4 bwc test for new name for now"
  Improve error handling for epoch format parser with time zone (elastic#23689)
  Docs: fix health response test
  Docs: Clean up response test in getting_started
  Validate top-level keys when parsing mget requests
  Reflect cross-cluster search in "dedicated" terminology (elastic#23771)
  Fix serialization for plugin info
  Update contributing docs for line-length change
  Remove line-length violations in SmokeTestClientIT
  Reenable smoke test client tests on JDK 9
  Build: Use GradleBuild task for invoking 5.x checkout build (elastic#23770)
  Introduce translog generation rolling
  Modify permissions dialog for plugins
  FuzzyQueryBuilder should error when parsing array of values (elastic#23762)
  Remove obsolete index setting `index.version.minimum_compatible`. (elastic#23593)
  Upgrade to Lucene 6.5.0 (elastic#23750)
  Fix docs for plugin install via proxy on Windows
  ...
@matriv
Copy link
Contributor Author

matriv commented Mar 28, 2017

@s1monw it was not in production, the issue came up when running a backwards compatibility test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants