Use removeTask instead of finishTask in PersistentTasksClusterService (#29055)

The method `PersistentTasksClusterService.finishTask()` has been
modified since it was added and does not use any `removeOncompletion`
flag anymore. Its behavior is now similar to `removeTask()` and can be
replaced by this one. When a non existing task is removed, the cluster
state update task will fail and its `source` will still indicate
`finish persistent task`/`remove persistent task`.

Author: tlrx

server/src/main/java/org/elasticsearch/persistent/PersistentTasksClusterService.java

@@ -117,7 +117,7 @@ public void completePersistentTask(String id, long allocationId, Exception failu
             public ClusterState execute(ClusterState currentState) throws Exception {
                 PersistentTasksCustomMetaData.Builder tasksInProgress = builder(currentState);
                 if (tasksInProgress.hasTask(id, allocationId)) {
-                    tasksInProgress.finishTask(id);
+                    tasksInProgress.removeTask(id);
                     return update(currentState, tasksInProgress);
                 } else {
                     if (tasksInProgress.hasTask(id)) {

server/src/main/java/org/elasticsearch/persistent/PersistentTasksCustomMetaData.java

@@ -609,7 +609,7 @@ public Builder reassignTask(String taskId, Assignment assignment) {
                 changed = true;
                 tasks.put(taskId, new PersistentTask<>(taskInProgress, getNextAllocationId(), assignment));
             } else {
-                throw new ResourceNotFoundException("cannot reassign task with id {" + taskId + "}, the task no longer exits");
+                throw new ResourceNotFoundException("cannot reassign task with id {" + taskId + "}, the task no longer exists");
             }
             return this;
         }
@@ -623,7 +623,7 @@ public Builder updateTaskStatus(String taskId, Status status) {
                 changed = true;
                 tasks.put(taskId, new PersistentTask<>(taskInProgress, status));
             } else {
-                throw new ResourceNotFoundException("cannot update task with id {" + taskId + "}, the task no longer exits");
+                throw new ResourceNotFoundException("cannot update task with id {" + taskId + "}, the task no longer exists");
             }
             return this;
         }
@@ -635,23 +635,7 @@ public Builder removeTask(String taskId) {
             if (tasks.remove(taskId) != null) {
                 changed = true;
             } else {
-                throw new ResourceNotFoundException("cannot remove task with id {" + taskId + "}, the task no longer exits");
-            }
-            return this;
-        }
-
-        /**
-         * Finishes the task
-         * <p>
-         * If the task is marked with removeOnCompletion flag, it is removed from the list, otherwise it is stopped.
-         */
-        public Builder finishTask(String taskId) {
-            PersistentTask<?> taskInProgress = tasks.get(taskId);
-            if (taskInProgress != null) {
-                changed = true;
-                tasks.remove(taskId);
-            } else {
-                throw new ResourceNotFoundException("cannot finish task with id {" + taskId + "}, the task no longer exits");
+                throw new ResourceNotFoundException("cannot remove task with id {" + taskId + "}, the task no longer exists");
             }
             return this;
         }

server/src/test/java/org/elasticsearch/persistent/PersistentTasksCustomMetaDataTests.java

@@ -191,7 +191,7 @@ public void testBuilder() {
             }
             boolean changed = false;
             for (int j = 0; j < randomIntBetween(1, 10); j++) {
-                switch (randomInt(4)) {
+                switch (randomInt(3)) {
                     case 0:
                         lastKnownTask = addRandomTask(builder);
                         changed = true;
@@ -223,15 +223,6 @@ public void testBuilder() {
                             expectThrows(ResourceNotFoundException.class, () -> builder.removeTask(fLastKnownTask));
                         }
                         break;
-                    case 4:
-                        if (builder.hasTask(lastKnownTask)) {
-                            changed = true;
-                            builder.finishTask(lastKnownTask);
-                        } else {
-                            String fLastKnownTask = lastKnownTask;
-                            expectThrows(ResourceNotFoundException.class, () -> builder.finishTask(fLastKnownTask));
-                        }
-                        break;
                 }
             }
             assertEquals(changed, builder.isChanged());