Skip to content

Conversation

@s1monw
Copy link
Contributor

@s1monw s1monw commented Sep 12, 2017

Today we don't have a pluggable way to validate if the cluster state
is compatible with the node that joins. We already apply some checks for index
compatibility that prevents nodes to join a cluster with indices it doesn't support
but for plugins this isn't possible. This change adds a cluster state validator that
allows plugins to prevent a join if the clusterstate is incompatible.

Today we don't have a pluggable way to validate if the cluster state
is compatible with the node that joins. We already apply some checks for index
compatibility that prevents nodes to join a cluster with indices it doesn't support
but for plugins this isn't possible. This change adds a cluster state validator that
allows plugins to prevent a join if the clusterstate is incompatible.
@s1monw s1monw added :Distributed Coordination/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure >enhancement v6.0.0 v6.1.0 v7.0.0 labels Sep 12, 2017
Copy link
Contributor

@bleskes bleskes left a comment

Choose a reason for hiding this comment

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

Looks good. I would love to add a test ala org.elasticsearch.discovery.zen.ZenDiscoveryIT#testHandleNodeJoin_incompatibleClusterState which uses the plugin mechanism. No need to block the PR on this


}

public final Collection<BiConsumer<DiscoveryNode, ClusterState>> getOnJoinValidators() {
Copy link
Contributor

Choose a reason for hiding this comment

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

where is this used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I missed to git add one file that uses it. sorry for the noise... it's in a test

/**
* Returns a consumer that validate the initial join cluster state. The validator, unless <code>null</code> is called exactly once per
* join attempt but might be called multiple times during the lifetime of a node. Validators are expected to throw a
* {@link IllegalStateException} if the incoming cluster states is invalid.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit - since we call it both on the master and the joining node, I think we should say "if the node and the cluster state are incompatible"

@s1monw
Copy link
Contributor Author

s1monw commented Sep 12, 2017

I would love to add a test ala org.elasticsearch.discovery.zen.ZenDiscoveryIT#testHandleNodeJoin_incompatibleClusterState which uses the plugin mechanism. No need to block the PR on this

I think we have tests for this since we also use the same mechanism for the compatibility and I have a test for the DiscoveryModule as well

@bleskes
Copy link
Contributor

bleskes commented Sep 12, 2017

I have a test for the DiscoveryModule as well

Fair enough. The extra test (in that forgotten add) is good. thx.

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.

@s1monw s1monw merged commit 42f3129 into elastic:master Sep 12, 2017
@s1monw s1monw deleted the add_join_validator branch September 12, 2017 13:32
@s1monw
Copy link
Contributor Author

s1monw commented Sep 12, 2017

thanks @jasontedor @bleskes

s1monw added a commit that referenced this pull request Sep 12, 2017
Today we don't have a pluggable way to validate if the cluster state
is compatible with the node that joins. We already apply some checks for index
compatibility that prevents nodes to join a cluster with indices it doesn't support
but for plugins this isn't possible. This change adds a cluster state validator that
allows plugins to prevent a join if the cluster-state is incompatible.
s1monw added a commit that referenced this pull request Sep 12, 2017
Today we don't have a pluggable way to validate if the cluster state
is compatible with the node that joins. We already apply some checks for index
compatibility that prevents nodes to join a cluster with indices it doesn't support
but for plugins this isn't possible. This change adds a cluster state validator that
allows plugins to prevent a join if the cluster-state is incompatible.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 12, 2017
…rflow

* origin/master: (59 commits)
  Fix Lucene version of 5.6.1.
  Remove azure deprecated settings (elastic#26099)
  Handle the 5.6.0 release
  Allow plugins to validate cluster-state on join (elastic#26595)
  Remove index mapper dynamic settings (elastic#25734)
  update AWS SDK for ECS Task IAM support in discovery-ec2 (elastic#26479)
  Azure repository: Accelerate the listing of files (used in delete snapshot) (elastic#25710)
  Build: Remove norelease from forbidden patterns (elastic#26592)
  Fix reference to painless inside expression engine (elastic#26528)
  Build: Move javadoc linking to root build.gradle (elastic#26529)
  Test: Remove leftover static bwc test case (elastic#26584)
  Docs: Remove remaining references to file and native scripts (elastic#26580)
  Snapshot fallback should consider build.snapshot
  elastic#26496: Set the correct bwc version after backport to 6.x
  Fix the MapperFieldType.rangeQuery API. (elastic#26552)
  Deduplicate `_field_names`. (elastic#26550)
  [Docs] Update method setSource(byte[] source) (elastic#26561)
  [Docs] Fix typo in javadocs (elastic#26556)
  Allow multiple digits in Vagrant 2.x minor versions
  Support Vagrant 2.x
  ...
jasontedor added a commit to jpountz/elasticsearch that referenced this pull request Sep 13, 2017
* master: (21 commits)
  Ensure module is bundled before installing in tests
  Add boolean similarity to built in similarity types (elastic#26613)
  [Tests] Remove skip tests in search/30_limits.yml
  Let search phases override max concurrent requests
  Add a soft limit for the number of requested doc-value fields (elastic#26574)
  Support for accessing Azure repositories through a proxy (elastic#23518)
  Add beta tag to MSI Windows Installer (elastic#26616)
  Fix Lucene version of 5.6.1.
  Remove azure deprecated settings (elastic#26099)
  Handle the 5.6.0 release
  Allow plugins to validate cluster-state on join (elastic#26595)
  Remove index mapper dynamic settings (elastic#25734)
  update AWS SDK for ECS Task IAM support in discovery-ec2 (elastic#26479)
  Azure repository: Accelerate the listing of files (used in delete snapshot) (elastic#25710)
  Build: Remove norelease from forbidden patterns (elastic#26592)
  Fix reference to painless inside expression engine (elastic#26528)
  Build: Move javadoc linking to root build.gradle (elastic#26529)
  Test: Remove leftover static bwc test case (elastic#26584)
  Docs: Remove remaining references to file and native scripts (elastic#26580)
  Snapshot fallback should consider build.snapshot
  ...
@lcawl lcawl removed the v6.1.0 label Dec 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Coordination/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure >enhancement v6.0.0-rc1 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants