Skip to content

Conversation

@bleskes
Copy link
Contributor

@bleskes bleskes commented Nov 6, 2017

The TemplateUpgradeService allows plugins to register a call back that mutates index templates upon recovery. This is handy for upgrade logic that needs to make sure that an existing index template is updated once the cluster is upgraded to a new version of the plugin (and ES).

Currently, the service has complicated logic to decide which node should perform the upgrade. It will prefer the master node, if it is of the highest version of the cluster and otherwise it will fall back to one of the non-coordinating nodes which are on the latest version. While this attempts to make sure that new nodes can assume their template version is in place (but old node still need to be able to operate under both old and new template), it has an inherent problem in that the master (on an old version) may not be able to process the put template request with the new template - it may miss certain features.

This PR changes the logic to be simpler and always rely on the current master nodes. This comes at the price that new nodes need to operate both with old templates and new. That price is small as they need to operate with old indices regardless of the template. On the flip side we reduce a lot of complexity in what can happen in the cluster.

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM.

@bleskes bleskes merged commit 95cf3df into elastic:master Nov 7, 2017
@bleskes
Copy link
Contributor Author

bleskes commented Nov 7, 2017

Thx @jasontedor

@bleskes bleskes deleted the template_upgrade_on_master branch November 7, 2017 07:36
bleskes added a commit that referenced this pull request Nov 7, 2017
The `TemplateUpgradeService` allows plugins to register a call back that mutates index templates upon recovery. This is handy for upgrade logic that needs to make sure that an existing index template is updated once the cluster is upgraded to a new version of the plugin (and ES).

Currently, the service has complicated logic to decide which node should perform the upgrade. It will prefer the master node, if it is of the highest version of the cluster and otherwise it will fall back to one of the non-coordinating nodes which are on the latest version. While this attempts to make sure that new nodes can assume their template version is in place (but old node still need to be able to operate under both old and new template), it has an inherent problem in that the master (on an old version) may not be able to process the put template request with the new template - it may miss certain features.

This PR changes the logic to be simpler and always rely on the current master nodes. This comes at the price that new nodes need to operate both with old templates and new. That price is small as they need to operate with old indices regardless of the template. On the flip side we reduce a lot of complexity in what can happen in the cluster.
jasontedor added a commit that referenced this pull request Nov 7, 2017
* master: (25 commits)
  Disable bwc tests in preparation of backporting #26931
  TemplateUpgradeService should only run on the master (#27294)
  Die with dignity while merging
  Fix profiling naming issues (#27133)
  Correctly encode warning headers
  Fixed references to Multi Index Syntax (#27283)
  Add an active Elasticsearch WordPress plugin link (#27279)
  Setting url parts as required to reflect the code base (#27263)
  keys in aggs percentiles need to be in quotes. (#26905)
  Align routing param type with search.json (#26958)
  Update to support bulk updates by query (#27172)
  Remove duplicated SnapshotStatus (#27276)
  add split index reference in indices.asciidoc
  Add ability to split shards (#26931)
  [Docs] Fix minor paragraph indentation error for multiple Indices params (#25535)
  Upgrade to Jackson 2.8.10 (#27230)
  Fix inconsistencies in the rest api specs for `tasks` (#27163)
  Adjust RestHighLevelClient method modifiers (#27238)
  Remove unused parameters in AnalysisRegistry (#27232)
  Add more information on `_failed_to_convert_` exception (#27034)
  ...
jasontedor added a commit to martijnvg/elasticsearch that referenced this pull request Nov 7, 2017
* ccr: (127 commits)
  Disable bwc tests in preparation of backporting elastic#26931
  TemplateUpgradeService should only run on the master (elastic#27294)
  Die with dignity while merging
  Fix profiling naming issues (elastic#27133)
  Correctly encode warning headers
  Fixed references to Multi Index Syntax (elastic#27283)
  Add an active Elasticsearch WordPress plugin link (elastic#27279)
  Setting url parts as required to reflect the code base (elastic#27263)
  keys in aggs percentiles need to be in quotes. (elastic#26905)
  Align routing param type with search.json (elastic#26958)
  Update to support bulk updates by query (elastic#27172)
  Remove duplicated SnapshotStatus (elastic#27276)
  add split index reference in indices.asciidoc
  Add ability to split shards (elastic#26931)
  [Docs] Fix minor paragraph indentation error for multiple Indices params (elastic#25535)
  Upgrade to Jackson 2.8.10 (elastic#27230)
  Fix inconsistencies in the rest api specs for `tasks` (elastic#27163)
  Adjust RestHighLevelClient method modifiers (elastic#27238)
  Remove unused parameters in AnalysisRegistry (elastic#27232)
  Add more information on `_failed_to_convert_` exception (elastic#27034)
  ...
jasontedor added a commit that referenced this pull request Nov 7, 2017
* 6.x:
  Update shrink's bwc version to 6.1.0
  add split index reference in indices.asciidoc
  Add ability to split shards (#26931)
  TemplateUpgradeService should only run on the master (#27294)
  Die with dignity while merging
  Fix profiling naming issues (#27133)
  Correctly encode warning headers
  Fixed references to Multi Index Syntax (#27283)
  Add an active Elasticsearch WordPress plugin link (#27279)
  Setting url parts as required to reflect the code base (#27263)
  keys in aggs percentiles need to be in quotes. (#26905)
  Align routing param type with search.json (#26958)
  Update to support bulk updates by query (#27172)
  Remove duplicated SnapshotStatus (#27276)
bleskes added a commit that referenced this pull request Nov 7, 2017
The `TemplateUpgradeService` allows plugins to register a call back that mutates index templates upon recovery. This is handy for upgrade logic that needs to make sure that an existing index template is updated once the cluster is upgraded to a new version of the plugin (and ES).

Currently, the service has complicated logic to decide which node should perform the upgrade. It will prefer the master node, if it is of the highest version of the cluster and otherwise it will fall back to one of the non-coordinating nodes which are on the latest version. While this attempts to make sure that new nodes can assume their template version is in place (but old node still need to be able to operate under both old and new template), it has an inherent problem in that the master (on an old version) may not be able to process the put template request with the new template - it may miss certain features.

This PR changes the logic to be simpler and always rely on the current master nodes. This comes at the price that new nodes need to operate both with old templates and new. That price is small as they need to operate with old indices regardless of the template. On the flip side we reduce a lot of complexity in what can happen in the cluster.
bleskes added a commit that referenced this pull request Nov 7, 2017
The `TemplateUpgradeService` allows plugins to register a call back that mutates index templates upon recovery. This is handy for upgrade logic that needs to make sure that an existing index template is updated once the cluster is upgraded to a new version of the plugin (and ES).

Currently, the service has complicated logic to decide which node should perform the upgrade. It will prefer the master node, if it is of the highest version of the cluster and otherwise it will fall back to one of the non-coordinating nodes which are on the latest version. While this attempts to make sure that new nodes can assume their template version is in place (but old node still need to be able to operate under both old and new template), it has an inherent problem in that the master (on an old version) may not be able to process the put template request with the new template - it may miss certain features.

This PR changes the logic to be simpler and always rely on the current master nodes. This comes at the price that new nodes need to operate both with old templates and new. That price is small as they need to operate with old indices regardless of the template. On the flip side we reduce a lot of complexity in what can happen in the cluster.
jasontedor added a commit to glefloch/elasticsearch that referenced this pull request Nov 9, 2017
* master: (556 commits)
  Fix find remote when building BWC
  Remove colons from task and configuration names
  Add unreleased 5.6.5 version number
  testCreateSplitIndexToN: do not set `routing_partition_size` to >= `number_of_routing_shards`
  Snapshot/Restore: better handle incorrect chunk_size settings in FS repo (elastic#26844)
  Add limits for ngram and shingle settings (elastic#27211) (elastic#27318)
  Correct comment in index shard test
  Roll translog generation on primary promotion
  ObjectParser: Replace IllegalStateException with ParsingException (elastic#27302)
  scripted_metric _agg parameter disappears if params are provided (elastic#27159)
  Update discovery-ec2.asciidoc
  Update shrink's bwc version to 6.1.0 and enabled bwc tests
  Add limits for ngram and shingle settings (elastic#27211)
  Disable bwc tests in preparation of backporting elastic#26931
  TemplateUpgradeService should only run on the master (elastic#27294)
  Die with dignity while merging
  Fix profiling naming issues (elastic#27133)
  Correctly encode warning headers
  Fixed references to Multi Index Syntax (elastic#27283)
  Add an active Elasticsearch WordPress plugin link (elastic#27279)
  ...
@lcawl lcawl removed the v6.1.0 label Dec 12, 2017
@elasticmachine elasticmachine mentioned this pull request Apr 25, 2018
11 tasks
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