[ML] Audit job open failures and stop any corresponding datafeed #80665

droberts195 · 2021-11-11T13:54:28Z

The anomaly detection code contained an assumption dating back
to 2016 that if a job failed then its datafeed would notice and
stop itself. That works if the job fails on a node after it has
successfully started up. But it doesn't work if the job fails
during the startup sequence. If the job is being started for the
first time then the datafeed won't be running, so there's no
problem, but if the job fails when it's being reassigned to a
new node then it breaks down, because the datafeed is started
but not assigned to any node at that instant. This leads to the
situation where a datafeed is started but its job is not opened,
which is supposed to be impossible.

This PR addresses this by making the job force-stop its own
datafeed if it fails during its startup sequence and the datafeed
is started.

Fixes #48934

Additionally, auditing of job failures during the startup
sequence is moved so that it happens for all failure scenarios
instead of just one.

Fixes #80621

The anomaly detection code contained an assumption dating back to 2016 that if a job failed then its datafeed would notice and stop itself. That works if the job fails on a node after it has successfully started up. But it doesn't work if the job fails during the startup sequence. If the job is being started for the first time then the datafeed won't be running, so there's no problem, but if the job fails when it's being reassigned to a new node then it breaks down, because the datafeed is started by not assigned to any node at that instant. This PR addresses this by making the job force-stop its own datafeed if it fails during its startup sequence and the datafeed is started. Fixes elastic#48934 Additionally, auditing of job failures during the startup sequence is moved so that it happens for all failure scenarios instead of just one. Fixes elastic#80621

elasticmachine · 2021-11-11T13:54:32Z

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

benwtrent · 2021-11-11T14:00:09Z

...rc/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectProcessManager.java

-                    handler.accept(e);
-                }
-            });
+            communicator.writeUpdateProcessMessage(updateProcessMessage.build(), (aVoid, e) -> handler.accept(e));


lulz, accept null if null

benwtrent · 2021-11-11T14:07:01Z

...gin/ml/src/main/java/org/elasticsearch/xpack/ml/job/task/OpenJobPersistentTasksExecutor.java

+        jobTask.updatePersistentTaskState(failedState, ActionListener.wrap(r -> {
+            logger.debug(() -> new ParameterizedMessage("[{}] updated task state to failed", jobTask.getJobId()));
+            stopAssociatedDatafeedForFailedJob(jobTask.getJobId());
+        }, e -> {
+            logger.error(
+                new ParameterizedMessage("[{}] error while setting task state to failed; marking task as failed", jobTask.getJobId()),
+                e
+            );
+            jobTask.markAsFailed(e);
+        }));


We are blowing away the task here with jobTask.markAsFailed(e);

So, does it make sense to also stopAssociatedDatafeedForFailedJob(jobTask.getJobId()); when we failed to set the job to failed?

Otherwise, we have a datafeed task around but the job task is gone.

Yes, sure, I'll add stopAssociatedDatafeedForFailedJob here too.

I am not sure if it will make any difference in practice though, because the only likely reason for failing to update the persistent task status is if the node is shutting down, and if that's happening then stopping the associated datafeed will also fail.

benwtrent · 2021-11-11T14:09:40Z

...gin/ml/src/main/java/org/elasticsearch/xpack/ml/job/task/OpenJobPersistentTasksExecutor.java

+                    e -> {
+                        if (autodetectProcessManager.isNodeDying() == false) {
+                            logger.error(
+                                new ParameterizedMessage(
+                                    "[{}] failed to stop associated datafeed [{}] after job failure",
+                                    jobId,
+                                    runningDatafeedId
+                                ),
+                                e
+                            );
+                        }
+                    }


What do you think of auditing a failure to stop?

TransportStopDatafeedAction already audits on stop success. I am just wondering if we should alert the user of a weird state.

…stic#80665) The anomaly detection code contained an assumption dating back to 2016 that if a job failed then its datafeed would notice and stop itself. That works if the job fails on a node after it has successfully started up. But it doesn't work if the job fails during the startup sequence. If the job is being started for the first time then the datafeed won't be running, so there's no problem, but if the job fails when it's being reassigned to a new node then it breaks down, because the datafeed is started by not assigned to any node at that instant. This PR addresses this by making the job force-stop its own datafeed if it fails during its startup sequence and the datafeed is started. Fixes elastic#48934 Additionally, auditing of job failures during the startup sequence is moved so that it happens for all failure scenarios instead of just one. Fixes elastic#80621

elasticsearchmachine · 2021-11-11T20:40:09Z

💚 Backport successful

Status	Branch	Result
✅	8.0
✅	7.16

) (#80677) The anomaly detection code contained an assumption dating back to 2016 that if a job failed then its datafeed would notice and stop itself. That works if the job fails on a node after it has successfully started up. But it doesn't work if the job fails during the startup sequence. If the job is being started for the first time then the datafeed won't be running, so there's no problem, but if the job fails when it's being reassigned to a new node then it breaks down, because the datafeed is started by not assigned to any node at that instant. This PR addresses this by making the job force-stop its own datafeed if it fails during its startup sequence and the datafeed is started. Fixes #48934 Additionally, auditing of job failures during the startup sequence is moved so that it happens for all failure scenarios instead of just one. Fixes #80621

…ed (#80665) (#80678) * [ML] Audit job open failures and stop any corresponding datafeed (#80665) The anomaly detection code contained an assumption dating back to 2016 that if a job failed then its datafeed would notice and stop itself. That works if the job fails on a node after it has successfully started up. But it doesn't work if the job fails during the startup sequence. If the job is being started for the first time then the datafeed won't be running, so there's no problem, but if the job fails when it's being reassigned to a new node then it breaks down, because the datafeed is started by not assigned to any node at that instant. This PR addresses this by making the job force-stop its own datafeed if it fails during its startup sequence and the datafeed is started. Fixes #48934 Additionally, auditing of job failures during the startup sequence is moved so that it happens for all failure scenarios instead of just one. Fixes #80621 * Removing 8.0+ test that won't work on 7.16

droberts195 added >bug :ml Machine learning v8.0.0 v7.16.1 v8.1.0 labels Nov 11, 2021

elasticmachine added the Team:ML Meta label for the ML team label Nov 11, 2021

droberts195 mentioned this pull request Nov 11, 2021

[ML] anomaly detection Job failed without reason audit message when model snapshot is too old #80621

Closed

benwtrent reviewed Nov 11, 2021

View reviewed changes

Addressing review comments

1aaa10a

benwtrent approved these changes Nov 11, 2021

View reviewed changes

droberts195 added the auto-backport-and-merge label Nov 11, 2021

droberts195 added 3 commits November 11, 2021 18:45

Adding an integration test

9c9e55d

Merge branch 'master' into fail_datafeed_if_job_open_fails

cb8e7e7

Fix typos

38ff595

droberts195 merged commit d95bca8 into elastic:master Nov 11, 2021

droberts195 deleted the fail_datafeed_if_job_open_fails branch November 11, 2021 20:39

droberts195 mentioned this pull request Nov 11, 2021

[8.0] [ML] Audit job open failures and stop any corresponding datafeed (#80665) #80677

Merged

droberts195 mentioned this pull request Nov 11, 2021

[7.16] [ML] Audit job open failures and stop any corresponding datafeed (#80665) #80678

Merged

danhermann added v7.16.0 and removed v7.16.1 labels Nov 16, 2021

mark-vieira added v8.0.0-rc1 and removed v8.0.0 labels Jan 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ML] Audit job open failures and stop any corresponding datafeed #80665

[ML] Audit job open failures and stop any corresponding datafeed #80665

Uh oh!

droberts195 commented Nov 11, 2021

Uh oh!

elasticmachine commented Nov 11, 2021

Uh oh!

benwtrent Nov 11, 2021

Uh oh!

benwtrent Nov 11, 2021

Uh oh!

droberts195 Nov 11, 2021

Uh oh!

benwtrent Nov 11, 2021

Uh oh!

elasticsearchmachine commented Nov 11, 2021

Uh oh!

Uh oh!

[ML] Audit job open failures and stop any corresponding datafeed #80665

[ML] Audit job open failures and stop any corresponding datafeed #80665

Uh oh!

Conversation

droberts195 commented Nov 11, 2021

Uh oh!

elasticmachine commented Nov 11, 2021

Uh oh!

benwtrent Nov 11, 2021

Choose a reason for hiding this comment

Uh oh!

benwtrent Nov 11, 2021

Choose a reason for hiding this comment

Uh oh!

droberts195 Nov 11, 2021

Choose a reason for hiding this comment

Uh oh!

benwtrent Nov 11, 2021

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Nov 11, 2021

💚 Backport successful

Uh oh!

Uh oh!