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

[ML] Fix submit after shutdown in process worker service #83645

Merged
merged 2 commits into from
Feb 8, 2022

Conversation

davidkyle
Copy link
Member

Runnables submitted after shutdown should not be executed.

Closes #83633

@elasticmachine elasticmachine added the Team:ML Meta label for the ML team label Feb 8, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

@elasticsearchmachine
Copy link
Collaborator

Hi @davidkyle, I've created a changelog YAML for you.

Copy link
Contributor

@przemekwitek przemekwitek left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -95,6 +95,7 @@ public synchronized void execute(Runnable command) {
EsRejectedExecutionException rejected = new EsRejectedExecutionException(processName + " worker service has shutdown", true);
if (command instanceof AbstractRunnable runnable) {
runnable.onRejection(rejected);
return;
Copy link
Member

Choose a reason for hiding this comment

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

🤦 sneaky

@davidkyle davidkyle merged commit 2970c3d into elastic:master Feb 8, 2022
davidkyle added a commit to davidkyle/elasticsearch that referenced this pull request Feb 8, 2022
davidkyle added a commit that referenced this pull request Feb 8, 2022
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.1

elasticsearchmachine pushed a commit that referenced this pull request Feb 8, 2022
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Feb 9, 2022
* upstream/master: (166 commits)
  Bind host all instead of just _site_ when needed (elastic#83145)
  [DOCS] Fix min/max agg snippets for histograms (elastic#83695)
  [DOCS] Add deprecation notice for system indices (elastic#83688)
  Cache ILM policy name on IndexMetadata (elastic#83603)
  [DOCS] Fix 8.0 breaking changes sort order (elastic#83685)
  [ML] fix random sampling background query consistency (elastic#83676)
  Move internal APIs into their own namespace '_internal'
  Runtime fields core-with-mapped tests support tsdb (elastic#83577)
  Optimize calculating the presence of a quorum (elastic#83638)
  Use switch expressions in EnableAllocationDecider and NodeShutdownAllocationDecider (elastic#83641)
  Note libffi error message in tmpdir docs (elastic#83662)
  Fix TransportDesiredNodesActionsIT batch tests (elastic#83659)
  [DOCS] Remove unused upgrade doc files (elastic#83617)
  [ML] Wait for model process to stop in stop deployment (elastic#83644)
  [ML] Fix submit after shutdown in process worker service (elastic#83645)
  Remove req/resp classes associated with HLRC (elastic#83599)
  Introduce index.version.compatibility setting (elastic#83264)
  Rename InternalTestCluster#getMasterNodeInstance (elastic#83407)
  Mute TimeSeriesIndexSearcherTests testCollectInOrderAcrossSegments (elastic#83648)
  Add rollover add max_primary_shard_docs condition (elastic#80981)
  ...

# Conflicts:
#	x-pack/plugin/rollup/build.gradle
#	x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/v2/RollupActionSingleNodeTests.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :ml Machine learning Team:ML Meta label for the ML team v8.1.0 v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] ProcessWorkerExecutorServiceTests testAutodetectWorkerExecutorService_SubmitAfterShutdown failing
5 participants