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

Only bootstrap and elect node in current voting configuration #37712

Merged
merged 3 commits into from
Jan 23, 2019

Conversation

ywelsch
Copy link
Contributor

@ywelsch ywelsch commented Jan 22, 2019

Adapts bootstrapping and leader election to only trigger on nodes that are actually part of the voting configuration. As a follow-up, we will switch master-ship when the current leader is reconfigured out of the voting configuration.

@ywelsch ywelsch added >non-issue v7.0.0 :Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. labels Jan 22, 2019
@ywelsch ywelsch requested a review from DaveCTurner January 22, 2019 16:19
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM. I left an optional suggestion to move the check in ClusterBootstrapService.

@@ -114,6 +114,13 @@ void onFoundPeersUpdated() {
logger.trace("nodesMatchingRequirements={}, unsatisfiedRequirements={}, bootstrapRequirements={}",
nodesMatchingRequirements, unsatisfiedRequirements, bootstrapRequirements);

if (nodesMatchingRequirements.contains(transportService.getLocalNode()) == false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can check this in the constructor and just assert it here. Also we use the word cancelling rather than skipping indicating that we won't retry.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The local discoverynode object is not available in the constructor yet (i.e. the transport has not bound at this point yet)

Copy link
Contributor

Choose a reason for hiding this comment

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

Blasted laziness. Ok.

@ywelsch
Copy link
Contributor Author

ywelsch commented Jan 23, 2019

@elasticmachine please run elasticsearch-ci/2

@ywelsch ywelsch merged commit d5139e0 into elastic:master Jan 23, 2019
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 23, 2019
…ead-de-duplication

* elastic/master: (24 commits)
  [TEST] Mute MlMappingsUpgradeIT testMappingsUpgrade
  Streamline skip_unavailable handling (elastic#37672)
  Only bootstrap and elect node in current voting configuration (elastic#37712)
  Ensure either success or failure path for SearchOperationListener is called (elastic#37467)
  Target only specific index in update settings test
  Add a note how to benchmark Elasticsearch
  Don't use Groovy's `withDefault` (elastic#37726)
  Adapt SyncedFlushService (elastic#37691)
  Mute FilterAggregatorTests#testRandom
  Switch mapping/aggregations over to java time (elastic#36363)
  [ML] Update ML results mappings on process start (elastic#37706)
  Modify removal_of_types.asciidoc (elastic#37648)
  Fix edge case in PutMappingRequestTests (elastic#37665)
  Use new bulk API endpoint in the docs (elastic#37698)
  Expose sequence number and primary terms in search responses (elastic#37639)
  Remove LicenseServiceClusterNotRecoveredTests (elastic#37528)
  Migrate SpecificMasterNodesIT to Zen2 (elastic#37532)
  Fix MetaStateFormat tests
  Use plain text instead of latexmath
  Fix a typo in a warning message in TestFixturesPlugin (elastic#37631)
  ...
@ywelsch ywelsch mentioned this pull request Jan 23, 2019
61 tasks
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 24, 2019
* elastic/master: (85 commits)
  Use explicit version for build-tools in example plugin integ tests (elastic#37792)
  Change `rational` to `saturation` in script_score (elastic#37766)
  Deprecate types in get field mapping API (elastic#37667)
  Add ability to listen to group of affix settings (elastic#37679)
  Ensure changes requests return the latest mapping version (elastic#37633)
  Make Minio Setup more Reliable (elastic#37747)
  Liberalize StreamOutput#writeStringList (elastic#37768)
  Add PersistentTasksClusterService::unassignPersistentTask method (elastic#37576)
  Tests: disable testRandomGeoCollectionQuery on tiny polygons (elastic#37579)
  Use ILM for Watcher history deletion (elastic#37443)
  Make sure PutMappingRequest accepts content types other than JSON. (elastic#37720)
  Retry ILM steps that fail due to SnapshotInProgressException (elastic#37624)
  Use disassociate in preference to deassociate (elastic#37704)
  Delete Redundant RoutingServiceTests (elastic#37750)
  Always return metadata version if metadata is requested (elastic#37674)
  [TEST] Mute MlMappingsUpgradeIT testMappingsUpgrade
  Streamline skip_unavailable handling (elastic#37672)
  Only bootstrap and elect node in current voting configuration (elastic#37712)
  Ensure either success or failure path for SearchOperationListener is called (elastic#37467)
  Target only specific index in update settings test
  ...
ywelsch added a commit that referenced this pull request Jan 29, 2019
Abdicates to another master-eligible node once the active master is reconfigured out of the voting
configuration, for example through the use of voting configuration exclusions.

Follow-up to #37712
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Jun 14, 2019
Today we suppress election attempts on master-eligible nodes that are not in
the voting configuration. In fact this restriction is not necessary: any
master-eligible node can safely become master as long as it has a fresh enough
cluster state and can gather a quorum of votes. Moreover, this restriction is
sometimes undesirable: there may be a reason why we do not want any of the
nodes in the voting configuration to become master.

The reason for this restriction is as follows. If you want to shut the master
down then you might first exclude it from the voting configuration. When this
exclusion succeeds you might reasonably expect that a new master has been
elected, since the voting config exclusion is almost always a step towards
shutting the node down. If we allow nodes outside the voting configuration to
be the master then the excluded node will continue to be master, which is
confusing.

This commit adjusts the logic to allow master-eligible nodes to attempt an
election even if they are not in the voting configuration. If such a master is
successfully elected then it adds itself to the voting configuration. This
commit also adjusts the logic that causes master nodes to abdicate when they
are excluded from the voting configuration, to avoid the confusion described
above.

Relates elastic#37712, elastic#37802.
DaveCTurner added a commit that referenced this pull request Jun 18, 2019
Today we suppress election attempts on master-eligible nodes that are not in
the voting configuration. In fact this restriction is not necessary: any
master-eligible node can safely become master as long as it has a fresh enough
cluster state and can gather a quorum of votes. Moreover, this restriction is
sometimes undesirable: there may be a reason why we do not want any of the
nodes in the voting configuration to become master.

The reason for this restriction is as follows. If you want to shut the master
down then you might first exclude it from the voting configuration. When this
exclusion succeeds you might reasonably expect that a new master has been
elected, since the voting config exclusion is almost always a step towards
shutting the node down. If we allow nodes outside the voting configuration to
be the master then the excluded node will continue to be master, which is
confusing.

This commit adjusts the logic to allow master-eligible nodes to attempt an
election even if they are not in the voting configuration. If such a master is
successfully elected then it adds itself to the voting configuration. This
commit also adjusts the logic that causes master nodes to abdicate when they
are excluded from the voting configuration, to avoid the confusion described
above.

Relates #37712, #37802.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. >non-issue v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants