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

Add infrastructure to upgrade settings #33536

Merged
merged 18 commits into from
Sep 10, 2018

Conversation

jasontedor
Copy link
Member

@jasontedor jasontedor commented Sep 7, 2018

In some cases we want to deprecate a setting, and then automatically upgrade uses of that setting to a replacement setting. This commit adds infrastructure for this so that we can upgrade settings when recovering the cluster state, as well as when such settings are dynamically applied on cluster update settings requests. This commit only focuses on cluster settings, index settings can build on this infrastructure in a follow-up.

@jasontedor jasontedor added the WIP label Sep 7, 2018
@jasontedor jasontedor force-pushed the upgrade-settings branch 2 times, most recently from 924f4ea to 50bd53e Compare September 8, 2018 17:53
In some cases we want to deprecate a setting, and then automatically
upgrade uses of that setting to a replacement setting. This commit adds
infrastructure for this so that we can upgrade settings when recovering
the cluster state, as well as when such settings are dynamically applied
on cluster update settings requests. This commit only focuses on cluster
settings, index settings can build on this infrastructure in a
follow-up.
@jasontedor jasontedor changed the title WIP - Add infrastructure to upgrade settings Add infrastructure to upgrade settings Sep 8, 2018
@jasontedor jasontedor added review :Core/Infra/Settings Settings infrastructure and APIs v7.0.0 v6.5.0 and removed WIP labels Sep 8, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

This looks fine, but I wonder if these could be registered like Setting objects themselves, through a Plugin pull based method, instead of manipulating AbstractScopedSettings?

@jasontedor
Copy link
Member Author

This looks fine, but I wonder if these could be registered like Setting objects themselves, through a Plugin pull based method, instead of manipulating AbstractScopedSettings?

Sure, we can do that (I think it might make it easier for the index settings case). Yet, the approach here is consistent with how we register cluster settings update listeners hence why I ended up favoring this way.

@jasontedor
Copy link
Member Author

@rjernst I pushed a change to a plugin architecture. Would you take a look?

* master:
  CORE: Make Pattern Exclusion Work with Aliases (elastic#33518)
  Reverse logic for CCR license checks (elastic#33549)
  Add latch countdown on failure in CCR license tests (elastic#33548)
  HLRC: Add put stored script support to high-level rest client (elastic#31323)
  Create temporary directory if needed in CCR test
  Add license checks for auto-follow implementation (elastic#33496)
  Bootstrap a new history_uuid when force allocating a stale primary (elastic#33432)
  INGEST: Remove Outdated TODOs (elastic#33458)
  Logging: Clean up skipping test
  Logging: Skip test if it'd fail
  CRUD: AwaitsFix entire wait_for_refresh close test
  Painless: Add Imported Static Method (elastic#33440)
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

Two more questions about the api signatures, but this looks better to me.

* master:
  Remove underscore from auto-follow API (elastic#33550)
  CCR: Use single global checkpoint to normalize range (elastic#33545)
@jasontedor
Copy link
Member Author

@rjernst I responded to your feedback, would you take another look?

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

@jasontedor jasontedor merged commit 6bb8170 into elastic:master Sep 10, 2018
jasontedor added a commit that referenced this pull request Sep 10, 2018
In some cases we want to deprecate a setting, and then automatically
upgrade uses of that setting to a replacement setting. This commit adds
infrastructure for this so that we can upgrade settings when recovering
the cluster state, as well as when such settings are dynamically applied
on cluster update settings requests. This commit only focuses on cluster
settings, index settings can build on this infrastructure in a
follow-up.
@jasontedor jasontedor deleted the upgrade-settings branch September 10, 2018 01:55
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.

4 participants