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 noop detection to node shutdown actions #85914

Merged
merged 3 commits into from
Apr 18, 2022

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Apr 14, 2022

When node shutdown apis are called to add or remove a node to be
shutdown, it is possible the given node is already shutting down. In
this case, there is no need to submit a cluster state update. This
commit detects when this no-op case occurs and simply returns.

relates #84847

When node shutdown apis are called to add or remove a node to be
shutdown, it is possible the given node is already shutting down. In
this case, there is no need to submit a cluster state update. This
commit detects when this no-op case occurs and simply returns.

relates elastic#84847
@rjernst rjernst added >refactoring :Core/Infra/Node Lifecycle Node startup, bootstrapping, and shutdown v8.3.0 labels Apr 14, 2022
@rjernst rjernst requested a review from pgomulka April 14, 2022 19:21
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Apr 14, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Copy link
Contributor

@pgomulka pgomulka left a comment

Choose a reason for hiding this comment

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

LGTM

@rjernst rjernst merged commit a672936 into elastic:master Apr 18, 2022
@rjernst rjernst deleted the shutdown/noop branch April 18, 2022 13:07
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Apr 21, 2022
* master: (104 commits)
  fix: ordering terms aggregation on top metrics null values (elastic#85774)
  Fix up whitespace error introduced in elastic#85948
  More docs re. removing cluster.initial_master_nodes (elastic#85948)
  [Test] Remove API key methods from HLRC (elastic#85802)
  Remove references to bootstrap.system_call_filter (elastic#85964)
  Move docker cgroup override to SystemJvmOptions (elastic#85960)
  Add connection accounting tests (elastic#85966)
  Remove MacOS from platform support testing matrix
  Remove custom KnnVectorFieldExistsQuery (elastic#85945)
  Relax data path deprecations from critical to warn (elastic#85952)
  Remove hppc from some "common" classes (elastic#85957)
  Move docker env var settings handling out of bash (elastic#85913)
  Remove hppc from task manager (elastic#85889)
  [ML] rename trained model allocations to assignments (elastic#85503)
  Remove hppc from multi*shard request and responses (elastic#85888)
  Consolidating logging initialization in cli launcher (elastic#85920)
  Convert license tools to use unified cli entrypoint (elastic#85919)
  Add noop detection to node shutdown actions (elastic#85914)
  Adjust SQL expended test output
  TSDB: Add timestamp provider to AggregationExecutionContext (elastic#85850)
  ...

# Conflicts:
#	server/src/main/java/org/elasticsearch/search/aggregations/AggregationExecutionContext.java
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Apr 27, 2022
In elastic#85914 we adjusted the shutdown metadata actions to avoid triggering
unnecessary cluster state publications. This commit reworks the
corresponding tests to use a real cluster service which decouple them
from the cluster service API.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Node Lifecycle Node startup, bootstrapping, and shutdown >refactoring Team:Core/Infra Meta label for core/infra team v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants