Make enrich policy execution cancelable#77188
Conversation
The policy cancellation implementation is best-effort. Prior to each transport action call the policy runner checks whether the corresponding task has been cancelled. If so no further action is performed, otherwise execution is continued as planned. The policy execution tasks is also made the parent task of transport action requests that are being executed by the policy runner. This will allow cancellation when certain transport actions are being executed (e.g. reindex). Also it should provide better insight which other tasks are related to a policy execution task. Additionally, to this change a `step` field is added to the enrich policy status. This field will contain the name of the currently executing transport action request. This will help, giving better insight, what a policy execution is doing. Relates elastic#48988
|
Pinging @elastic/es-data-management (Team:Data Management) |
| private static final String STEP_FIELD = "step"; | ||
|
|
||
| private final String phase; | ||
| private final String step; |
There was a problem hiding this comment.
Maybe a different name?
There was a problem hiding this comment.
It's a weird situation because the phases here are more like a "status" than a "phase", but naming things is the worst. Step is a fine name in my opinion
| ensureEnrichIndexIsReadOnly(createdEnrichIndex); | ||
| } | ||
|
|
||
| public void testRunnerCancel() throws Exception { |
There was a problem hiding this comment.
I think truly integration testing policy execution cancellation isn't possible (without avoiding test related failures)?
jbaiera
left a comment
There was a problem hiding this comment.
LGTM! Just a few comments/questions for my own knowledge here.
| if (action.equals(randomActionType)) { | ||
| testTaskManager.getCancellableTasks() | ||
| .values() | ||
| .stream() | ||
| .filter(cancellableTask -> cancellableTask instanceof ExecuteEnrichPolicyTask) | ||
| .forEach(task -> testTaskManager.cancel(task, "", () -> {})); | ||
| } |
| if (task.isCancelled()) { | ||
| String message = "cancelled policy execution [" + policyName + "], status [" + Strings.toString(task.getStatus()) + "]"; | ||
| listener.onFailure(new TaskCancelledException(message)); | ||
| return; | ||
| } |
There was a problem hiding this comment.
There isn't a way to propagate a task cancellation to its child tasks by default is there? Just asking for my own knowledge here, not suggesting any changes.
There was a problem hiding this comment.
Via this if clause, the node task let's any thread that continues execution know that the node task has been cancelled and the on failure invocation triggers info log in InternalExecutePolicyAction line 121. The TaskCancellationService#doCancelTaskAndDescendants(...) method handles cancellation of child tasks (which in case of ExecuteEnrichPolicyTaskany cancellable task backed by transport action calls invoked from the wrapped client inEnrichPolicyRunner(e.g. reindex) ). The TaskCancellationServiceknows about this because in line 540 ofEnrichPolicyRunner` class the ExecuteEnrichPolicyTask's id is set as parent task.
The policy cancellation implementation is best-effort. Prior to each transport action call the policy runner checks whether the corresponding task has been cancelled. If so no further action is performed, otherwise execution is continued as planned. The policy execution tasks is also made the parent task of transport action requests that are being executed by the policy runner. This will allow cancellation when certain transport actions are being executed (e.g. reindex). Also it should provide better insight which other tasks are related to a policy execution task. Additionally, to this change a `step` field is added to the enrich policy status. This field will contain the name of the currently executing transport action request. This will help, giving better insight, what a policy execution is doing. Relates elastic#48988
Backport #77188 to 7.x branch. The policy cancellation implementation is best-effort. Prior to each transport action call the policy runner checks whether the corresponding task has been cancelled. If so no further action is performed, otherwise execution is continued as planned. The policy execution tasks is also made the parent task of transport action requests that are being executed by the policy runner. This will allow cancellation when certain transport actions are being executed (e.g. reindex). Also it should provide better insight which other tasks are related to a policy execution task. Additionally, to this change a `step` field is added to the enrich policy status. This field will contain the name of the currently executing transport action request. This will help, giving better insight, what a policy execution is doing. Relates #48988
The policy cancellation implementation is best-effort.
Prior to each transport action call the policy runner
checks whether the corresponding task has been cancelled.
If so no further action is performed, otherwise
execution is continued as planned.
The policy execution tasks is also made the parent task
of transport action requests that are being executed
by the policy runner. This will allow cancellation when
certain transport actions are being executed (e.g. reindex).
Also it should provide better insight which other tasks
are related to a policy execution task.
Additionally, to this change a
stepfield is added tothe enrich policy status. This field will contain
the name of the currently executing transport action
request. This will help, giving better insight, what
a policy execution is doing.
Relates #48988
Example usage
(assuming policy with name
my-policyexists)Execute policy in async style manner:
Response:
Get task information about policy execution:
Response:
Cancel the policy execution:
Response: