[ML] Option to delete user-added annotations for the reset/delete job APIs (#91698)

Currently there is no way to remove user-added annotations when a job is deleted or reset.
This change adds an option - delete_user_annotations - to both the delete and reset job APIs.
The default value is false, to keep the behaviour of these calls as it is currently.

docs/changelog/91698.yaml

@@ -0,0 +1,6 @@
+pr: 91698
+summary: Option to delete user-added annotations for the reset/delete job APIs
+area: Machine Learning
+type: enhancement
+issues:
+ - 74310

docs/reference/ml/anomaly-detection/apis/delete-job.asciidoc

@@ -56,6 +56,11 @@ include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=job-id-anomaly-detection]
   (Optional, Boolean) Specifies whether the request should return immediately or
   wait until the job deletion completes. Defaults to `true`.
 
+`delete_user_annotations`::
+  (Optional, Boolean) Specifies whether annotations that have been added by the 
+  user should be deleted along with any auto-generated annotations when the job is
+  reset. Defaults to `false`.
+
 [[ml-delete-job-example]]
 == {api-examples-title}
 

docs/reference/ml/anomaly-detection/apis/reset-job.asciidoc

@@ -44,6 +44,11 @@ include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=job-id-anomaly-detection]
   (Optional, Boolean) Specifies whether the request should return immediately or
   wait until the job reset completes. Defaults to `true`.
 
+`delete_user_annotations`::
+  (Optional, Boolean) Specifies whether annotations that have been added by the 
+  user should be deleted along with any auto-generated annotations when the job is
+  reset. Defaults to `false`.
+
 [[ml-reset-job-example]]
 == {api-examples-title}
 

rest-api-spec/src/main/resources/rest-api-spec/api/ml.delete_job.json

@@ -35,6 +35,11 @@
         "type":"boolean",
         "description":"Should this request wait until the operation has completed before returning",
         "default":true
+      },
+      "delete_user_annotations":{
+        "type":"boolean",
+        "description":"Should annotations added by the user be deleted",
+        "default":false
       }
     }
   }

rest-api-spec/src/main/resources/rest-api-spec/api/ml.reset_job.json

@@ -30,6 +30,11 @@
         "type":"boolean",
         "description":"Should this request wait until the operation has completed before returning",
         "default":true
+      },
+      "delete_user_annotations":{
+        "type":"boolean",
+        "description":"Should annotations added by the user be deleted",
+        "default":false
       }
     }
   }

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteJobAction.java

@@ -6,6 +6,7 @@
  */
 package org.elasticsearch.xpack.core.ml.action;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.ActionType;
 import org.elasticsearch.action.support.master.AcknowledgedRequest;
@@ -38,6 +39,11 @@ public class DeleteJobAction extends ActionType<AcknowledgedResponse> {
          */
         private boolean shouldStoreResult;
 
+        /**
+         * Should user added annotations be removed when the job is deleted?
+         */
+        private boolean deleteUserAnnotations;
+
         public Request(String jobId) {
             this.jobId = ExceptionsHelper.requireNonNull(jobId, Job.ID.getPreferredName());
         }
@@ -46,6 +52,11 @@ public class DeleteJobAction extends ActionType<AcknowledgedResponse> {
             super(in);
             jobId = in.readString();
             force = in.readBoolean();
+            if (in.getVersion().onOrAfter(Version.V_8_7_0)) {
+                deleteUserAnnotations = in.readBoolean();
+            } else {
+                deleteUserAnnotations = false;
+            }
         }
 
         public String getJobId() {
@@ -76,6 +87,14 @@ public class DeleteJobAction extends ActionType<AcknowledgedResponse> {
             return shouldStoreResult;
         }
 
+        public void setDeleteUserAnnotations(boolean deleteUserAnnotations) {
+            this.deleteUserAnnotations = deleteUserAnnotations;
+        }
+
+        public boolean getDeleteUserAnnotations() {
+            return deleteUserAnnotations;
+        }
+
         @Override
         public ActionRequestValidationException validate() {
             return null;
@@ -91,11 +110,14 @@ public class DeleteJobAction extends ActionType<AcknowledgedResponse> {
             super.writeTo(out);
             out.writeString(jobId);
             out.writeBoolean(force);
+            if (out.getVersion().onOrAfter(Version.V_8_7_0)) {
+                out.writeBoolean(deleteUserAnnotations);
+            }
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(jobId, force);
+            return Objects.hash(jobId, force, deleteUserAnnotations);
         }
 
         @Override
@@ -107,7 +129,9 @@ public class DeleteJobAction extends ActionType<AcknowledgedResponse> {
                 return false;
             }
             DeleteJobAction.Request other = (DeleteJobAction.Request) obj;
-            return Objects.equals(jobId, other.jobId) && Objects.equals(force, other.force);
+            return Objects.equals(jobId, other.jobId)
+                && Objects.equals(force, other.force)
+                && Objects.equals(deleteUserAnnotations, other.deleteUserAnnotations);
         }
     }
 }

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ResetJobAction.java

@@ -51,6 +51,11 @@ public class ResetJobAction extends ActionType<AcknowledgedResponse> {
          */
         private boolean shouldStoreResult;
 
+        /**
+         * Should user added annotations be removed when the job is reset?
+         */
+        private boolean deleteUserAnnotations;
+
         public Request(String jobId) {
             this.jobId = ExceptionsHelper.requireNonNull(jobId, Job.ID);
         }
@@ -59,6 +64,11 @@ public class ResetJobAction extends ActionType<AcknowledgedResponse> {
             super(in);
             jobId = in.readString();
             skipJobStateValidation = in.readBoolean();
+            if (in.getVersion().onOrAfter(Version.V_8_7_0)) {
+                deleteUserAnnotations = in.readBoolean();
+            } else {
+                deleteUserAnnotations = false;
+            }
         }
 
         @Override
@@ -66,6 +76,9 @@ public class ResetJobAction extends ActionType<AcknowledgedResponse> {
             super.writeTo(out);
             out.writeString(jobId);
             out.writeBoolean(skipJobStateValidation);
+            if (out.getVersion().onOrAfter(Version.V_8_7_0)) {
+                out.writeBoolean(deleteUserAnnotations);
+            }
         }
 
         public void setSkipJobStateValidation(boolean skipJobStateValidation) {
@@ -88,6 +101,14 @@ public class ResetJobAction extends ActionType<AcknowledgedResponse> {
             return shouldStoreResult;
         }
 
+        public void setDeleteUserAnnotations(boolean deleteUserAnnotations) {
+            this.deleteUserAnnotations = deleteUserAnnotations;
+        }
+
+        public boolean getDeleteUserAnnotations() {
+            return deleteUserAnnotations;
+        }
+
         @Override
         public Task createTask(long id, String type, String action, TaskId parentTaskId, Map<String, String> headers) {
             return new CancellableTask(id, type, action, MlTasks.JOB_TASK_ID_PREFIX + jobId, parentTaskId, headers);
@@ -104,7 +125,7 @@ public class ResetJobAction extends ActionType<AcknowledgedResponse> {
 
         @Override
         public int hashCode() {
-            return Objects.hash(jobId, skipJobStateValidation);
+            return Objects.hash(jobId, skipJobStateValidation, deleteUserAnnotations);
         }
 
         @Override
@@ -112,7 +133,9 @@ public class ResetJobAction extends ActionType<AcknowledgedResponse> {
             if (this == o) return true;
             if (o == null || o.getClass() != getClass()) return false;
             Request that = (Request) o;
-            return Objects.equals(jobId, that.jobId) && skipJobStateValidation == that.skipJobStateValidation;
+            return Objects.equals(jobId, that.jobId)
+                && skipJobStateValidation == that.skipJobStateValidation
+                && deleteUserAnnotations == that.deleteUserAnnotations;
         }
     }
 }

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportResetJobAction.java

@@ -245,7 +245,7 @@ public class TransportResetJobAction extends AcknowledgedTransportMasterNodeActi
             }, listener::onFailure));
         };
 
-        JobDataDeleter jobDataDeleter = new JobDataDeleter(taskClient, jobId);
+        JobDataDeleter jobDataDeleter = new JobDataDeleter(taskClient, jobId, request.getDeleteUserAnnotations());
         jobDataDeleter.deleteJobDocuments(
             jobConfigProvider,
             indexNameExpressionResolver,

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java

@@ -416,7 +416,7 @@ public class JobManager {
 
         // Step 2. Delete the physical storage
         ActionListener<CancelJobModelSnapshotUpgradeAction.Response> cancelUpgradesListener = ActionListener.wrap(
-            r -> new JobDataDeleter(clientToUse, jobId).deleteJobDocuments(
+            r -> new JobDataDeleter(clientToUse, jobId, request.getDeleteUserAnnotations()).deleteJobDocuments(
                 jobConfigProvider,
                 indexNameExpressionResolver,
                 state,

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleter.java

@@ -86,10 +86,16 @@ public class JobDataDeleter {
 
     private final Client client;
     private final String jobId;
+    private final boolean deleteUserAnnotations;
 
     public JobDataDeleter(Client client, String jobId) {
+        this(client, jobId, false);
+    }
+
+    public JobDataDeleter(Client client, String jobId, boolean deleteUserAnnotations) {
         this.client = Objects.requireNonNull(client);
         this.jobId = Objects.requireNonNull(jobId);
+        this.deleteUserAnnotations = deleteUserAnnotations;
     }
 
     /**
@@ -135,8 +141,11 @@ public class JobDataDeleter {
     }
 
     /**
-     * Asynchronously delete all the auto-generated (i.e. created by the _xpack user) annotations
-     *
+     * Asynchronously delete the annotations
+     * If the deleteUserAnnotations field is set to true then all
+     * annotations - both auto-generated and user-added - are removed, else
+     * only the auto-generated ones, (i.e. created by the _xpack user) are
+     * removed.
      * @param listener Response listener
      */
     public void deleteAllAnnotations(ActionListener<Boolean> listener) {
@@ -158,9 +167,10 @@ public class JobDataDeleter {
         @Nullable Set<String> eventsToDelete,
         ActionListener<Boolean> listener
     ) {
-        BoolQueryBuilder boolQuery = QueryBuilders.boolQuery()
-            .filter(QueryBuilders.termQuery(Job.ID.getPreferredName(), jobId))
-            .filter(QueryBuilders.termQuery(Annotation.CREATE_USERNAME.getPreferredName(), XPackUser.NAME));
+        BoolQueryBuilder boolQuery = QueryBuilders.boolQuery().filter(QueryBuilders.termQuery(Job.ID.getPreferredName(), jobId));
+        if (deleteUserAnnotations == false) {
+            boolQuery.filter(QueryBuilders.termQuery(Annotation.CREATE_USERNAME.getPreferredName(), XPackUser.NAME));
+        }
         if (fromEpochMs != null || toEpochMs != null) {
             boolQuery.filter(QueryBuilders.rangeQuery(Annotation.TIMESTAMP.getPreferredName()).gte(fromEpochMs).lt(toEpochMs));
         }

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestDeleteJobAction.java

@@ -49,6 +49,7 @@ public class RestDeleteJobAction extends BaseRestHandler {
         deleteJobRequest.setForce(restRequest.paramAsBoolean(CloseJobAction.Request.FORCE.getPreferredName(), deleteJobRequest.isForce()));
         deleteJobRequest.timeout(restRequest.paramAsTime("timeout", deleteJobRequest.timeout()));
         deleteJobRequest.masterNodeTimeout(restRequest.paramAsTime("master_timeout", deleteJobRequest.masterNodeTimeout()));
+        deleteJobRequest.setDeleteUserAnnotations(restRequest.paramAsBoolean("delete_user_annotations", false));
 
         if (restRequest.paramAsBoolean("wait_for_completion", true)) {
             return channel -> client.execute(DeleteJobAction.INSTANCE, deleteJobRequest, new RestToXContentListener<>(channel));

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestResetJobAction.java

@@ -42,6 +42,7 @@ public class RestResetJobAction extends BaseRestHandler {
         ResetJobAction.Request request = new ResetJobAction.Request(restRequest.param(Job.ID.getPreferredName()));
         request.timeout(restRequest.paramAsTime("timeout", request.timeout()));
         request.masterNodeTimeout(restRequest.paramAsTime("master_timeout", request.masterNodeTimeout()));
+        request.setDeleteUserAnnotations(restRequest.paramAsBoolean("delete_user_annotations", false));
 
         if (restRequest.paramAsBoolean("wait_for_completion", true)) {
             return channel -> client.execute(ResetJobAction.INSTANCE, request, new RestToXContentListener<>(channel));

x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleterTests.java

@@ -22,6 +22,7 @@ import org.junit.After;
 import org.junit.Before;
 import org.mockito.ArgumentCaptor;
 
+import java.util.Arrays;
 import java.util.Set;
 
 import static org.elasticsearch.core.Tuple.tuple;
@@ -32,6 +33,7 @@ import static org.hamcrest.Matchers.not;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
@@ -54,66 +56,110 @@ public class JobDataDeleterTests extends ESTestCase {
 
     @After
     public void verifyNoMoreInteractionsWithClient() {
-        verify(client).threadPool();
+        verify(client, times(2)).threadPool();
         verifyNoMoreInteractions(client);
     }
 
     public void testDeleteAllAnnotations() {
-        JobDataDeleter jobDataDeleter = new JobDataDeleter(client, JOB_ID);
-        jobDataDeleter.deleteAllAnnotations(ActionListener.wrap(deleteResponse -> {}, e -> fail(e.toString())));
-
-        verify(client).execute(eq(DeleteByQueryAction.INSTANCE), deleteRequestCaptor.capture(), any());
-
-        DeleteByQueryRequest deleteRequest = deleteRequestCaptor.getValue();
-        assertThat(deleteRequest.indices(), is(arrayContaining(AnnotationIndex.READ_ALIAS_NAME)));
-        String dbqQueryString = Strings.toString(deleteRequest.getSearchRequest().source().query());
-        assertThat(dbqQueryString, not(containsString("timestamp")));
-        assertThat(dbqQueryString, not(containsString("event")));
+        Arrays.asList(false, true).forEach(deleteUserAnnotations -> {
+            JobDataDeleter jobDataDeleter = new JobDataDeleter(client, JOB_ID, deleteUserAnnotations);
+            jobDataDeleter.deleteAllAnnotations(ActionListener.wrap(deleteResponse -> {}, e -> fail(e.toString())));
+
+            if (deleteUserAnnotations) {
+                verify(client, times(2)).execute(eq(DeleteByQueryAction.INSTANCE), deleteRequestCaptor.capture(), any());
+            } else {
+                verify(client).execute(eq(DeleteByQueryAction.INSTANCE), deleteRequestCaptor.capture(), any());
+            }
+
+            DeleteByQueryRequest deleteRequest = deleteRequestCaptor.getValue();
+            assertThat(deleteRequest.indices(), is(arrayContaining(AnnotationIndex.READ_ALIAS_NAME)));
+            String dbqQueryString = Strings.toString(deleteRequest.getSearchRequest().source().query());
+            assertThat(dbqQueryString, not(containsString("timestamp")));
+            assertThat(dbqQueryString, not(containsString("event")));
+            if (deleteUserAnnotations) {
+                assertThat(dbqQueryString, not(containsString("_xpack")));
+            } else {
+                assertThat(dbqQueryString, containsString("_xpack"));
+            }
+        });
     }
 
     public void testDeleteAnnotations_TimestampFiltering() {
-        JobDataDeleter jobDataDeleter = new JobDataDeleter(client, JOB_ID);
-        Tuple<Long, Long> range = randomFrom(
-            tuple(1_000_000_000L, 2_000_000_000L),
-            tuple(1_000_000_000L, null),
-            tuple(null, 2_000_000_000L)
-        );
-        jobDataDeleter.deleteAnnotations(range.v1(), range.v2(), null, ActionListener.wrap(deleteResponse -> {}, e -> fail(e.toString())));
-
-        verify(client).execute(eq(DeleteByQueryAction.INSTANCE), deleteRequestCaptor.capture(), any());
-
-        DeleteByQueryRequest deleteRequest = deleteRequestCaptor.getValue();
-        assertThat(deleteRequest.indices(), is(arrayContaining(AnnotationIndex.READ_ALIAS_NAME)));
-        String dbqQueryString = Strings.toString(deleteRequest.getSearchRequest().source().query());
-        assertThat(dbqQueryString, containsString("timestamp"));
-        assertThat(dbqQueryString, not(containsString("event")));
+        Arrays.asList(false, true).forEach(deleteUserAnnotations -> {
+            JobDataDeleter jobDataDeleter = new JobDataDeleter(client, JOB_ID, deleteUserAnnotations);
+            Tuple<Long, Long> range = randomFrom(
+                tuple(1_000_000_000L, 2_000_000_000L),
+                tuple(1_000_000_000L, null),
+                tuple(null, 2_000_000_000L)
+            );
+            jobDataDeleter.deleteAnnotations(
+                range.v1(),
+                range.v2(),
+                null,
+                ActionListener.wrap(deleteResponse -> {}, e -> fail(e.toString()))
+            );
+
+            if (deleteUserAnnotations) {
+                verify(client, times(2)).execute(eq(DeleteByQueryAction.INSTANCE), deleteRequestCaptor.capture(), any());
+            } else {
+                verify(client).execute(eq(DeleteByQueryAction.INSTANCE), deleteRequestCaptor.capture(), any());
+            }
+
+            DeleteByQueryRequest deleteRequest = deleteRequestCaptor.getValue();
+            assertThat(deleteRequest.indices(), is(arrayContaining(AnnotationIndex.READ_ALIAS_NAME)));
+            String dbqQueryString = Strings.toString(deleteRequest.getSearchRequest().source().query());
+            assertThat(dbqQueryString, containsString("timestamp"));
+            assertThat(dbqQueryString, not(containsString("event")));
+            if (deleteUserAnnotations) {
+                assertThat(dbqQueryString, not(containsString("_xpack")));
+            } else {
+                assertThat(dbqQueryString, containsString("_xpack"));
+            }
+        });
     }
 
     public void testDeleteAnnotations_EventFiltering() {
-        JobDataDeleter jobDataDeleter = new JobDataDeleter(client, JOB_ID);
-        jobDataDeleter.deleteAnnotations(
-            null,
-            null,
-            Set.of("dummy_event"),
-            ActionListener.wrap(deleteResponse -> {}, e -> fail(e.toString()))
-        );
-
-        verify(client).execute(eq(DeleteByQueryAction.INSTANCE), deleteRequestCaptor.capture(), any());
-
-        DeleteByQueryRequest deleteRequest = deleteRequestCaptor.getValue();
-        assertThat(deleteRequest.indices(), is(arrayContaining(AnnotationIndex.READ_ALIAS_NAME)));
-        String dbqQueryString = Strings.toString(deleteRequest.getSearchRequest().source().query());
-        assertThat(dbqQueryString, not(containsString("timestamp")));
-        assertThat(dbqQueryString, containsString("event"));
+        Arrays.asList(false, true).forEach(deleteUserAnnotations -> {
+            JobDataDeleter jobDataDeleter = new JobDataDeleter(client, JOB_ID, deleteUserAnnotations);
+            jobDataDeleter.deleteAnnotations(
+                null,
+                null,
+                Set.of("dummy_event"),
+                ActionListener.wrap(deleteResponse -> {}, e -> fail(e.toString()))
+            );
+
+            if (deleteUserAnnotations) {
+                verify(client, times(2)).execute(eq(DeleteByQueryAction.INSTANCE), deleteRequestCaptor.capture(), any());
+            } else {
+                verify(client).execute(eq(DeleteByQueryAction.INSTANCE), deleteRequestCaptor.capture(), any());
+            }
+
+            DeleteByQueryRequest deleteRequest = deleteRequestCaptor.getValue();
+            assertThat(deleteRequest.indices(), is(arrayContaining(AnnotationIndex.READ_ALIAS_NAME)));
+            String dbqQueryString = Strings.toString(deleteRequest.getSearchRequest().source().query());
+            assertThat(dbqQueryString, not(containsString("timestamp")));
+            assertThat(dbqQueryString, containsString("event"));
+            if (deleteUserAnnotations) {
+                assertThat(dbqQueryString, not(containsString("_xpack")));
+            } else {
+                assertThat(dbqQueryString, containsString("_xpack"));
+            }
+        });
     }
 
     public void testDeleteDatafeedTimingStats() {
-        JobDataDeleter jobDataDeleter = new JobDataDeleter(client, JOB_ID);
-        jobDataDeleter.deleteDatafeedTimingStats(ActionListener.wrap(deleteResponse -> {}, e -> fail(e.toString())));
-
-        verify(client).execute(eq(DeleteByQueryAction.INSTANCE), deleteRequestCaptor.capture(), any());
-
-        DeleteByQueryRequest deleteRequest = deleteRequestCaptor.getValue();
-        assertThat(deleteRequest.indices(), is(arrayContaining(AnomalyDetectorsIndex.jobResultsAliasedName(JOB_ID))));
+        Arrays.asList(false, true).forEach(deleteUserAnnotations -> {
+            JobDataDeleter jobDataDeleter = new JobDataDeleter(client, JOB_ID, deleteUserAnnotations);
+            jobDataDeleter.deleteDatafeedTimingStats(ActionListener.wrap(deleteResponse -> {}, e -> fail(e.toString())));
+
+            if (deleteUserAnnotations) {
+                verify(client, times(2)).execute(eq(DeleteByQueryAction.INSTANCE), deleteRequestCaptor.capture(), any());
+            } else {
+                verify(client).execute(eq(DeleteByQueryAction.INSTANCE), deleteRequestCaptor.capture(), any());
+            }
+
+            DeleteByQueryRequest deleteRequest = deleteRequestCaptor.getValue();
+            assertThat(deleteRequest.indices(), is(arrayContaining(AnomalyDetectorsIndex.jobResultsAliasedName(JOB_ID))));
+        });
     }
 }

x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/delete_job_force.yml

@@ -38,6 +38,7 @@ setup:
   - do:
       ml.delete_job:
         force: true
+        delete_user_annotations: true
         job_id: force-delete-job
   - match: { acknowledged: true }
 

x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/reset_job.yml

@@ -24,6 +24,7 @@ setup:
   - do:
       ml.reset_job:
         job_id: reset-job
+        delete_user_annotations: true
   - match: { acknowledged: true }
 
 ---