Skip to content

Commit 69dc9e2

Browse files
kkewweikaushalmahi12
authored andcommitted
print reason why parent task was cancelled (opensearch-project#14604)
Signed-off-by: kkewwei <[email protected]> Signed-off-by: Kaushal Kumar <[email protected]>
1 parent c9b075b commit 69dc9e2

File tree

5 files changed

+17
-8
lines changed

5 files changed

+17
-8
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
1515
- Fix race condition while parsing derived fields from search definition ([14445](https://github.com/opensearch-project/OpenSearch/pull/14445))
1616
- Add allowlist setting for ingest-common and search-pipeline-common processors ([#14439](https://github.com/opensearch-project/OpenSearch/issues/14439))
1717
- Create SystemIndexRegistry with helper method matchesSystemIndex ([#14415](https://github.com/opensearch-project/OpenSearch/pull/14415))
18+
- Print reason why parent task was cancelled ([#14604](https://github.com/opensearch-project/OpenSearch/issues/14604))
1819

1920
### Dependencies
2021
- Bump `org.gradle.test-retry` from 1.5.8 to 1.5.9 ([#13442](https://github.com/opensearch-project/OpenSearch/pull/13442))

server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/node/tasks/CancellableTasksIT.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ public void testFailedToStartChildTaskAfterCancelled() throws Exception {
327327
mainAction.startSubTask(taskId, subRequest, future);
328328
TransportException te = expectThrows(TransportException.class, future::actionGet);
329329
assertThat(te.getCause(), instanceOf(TaskCancelledException.class));
330-
assertThat(te.getCause().getMessage(), equalTo("The parent task was cancelled, shouldn't start any child tasks"));
330+
assertThat(te.getCause().getMessage(), equalTo("The parent task was cancelled, shouldn't start any child tasks, by user request"));
331331
allowEntireRequest(rootRequest);
332332
waitForRootTask(rootTaskFuture);
333333
ensureAllBansRemoved();
@@ -386,7 +386,7 @@ static void waitForRootTask(ActionFuture<TestResponse> rootTask) {
386386
assertThat(
387387
cause.getMessage(),
388388
anyOf(
389-
equalTo("The parent task was cancelled, shouldn't start any child tasks"),
389+
equalTo("The parent task was cancelled, shouldn't start any child tasks, by user request"),
390390
containsString("Task cancelled before it started:"),
391391
equalTo("Task was cancelled while executing")
392392
)

server/src/main/java/org/opensearch/tasks/TaskCancellationService.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ void cancelTaskAndDescendants(CancellableTask task, String reason, boolean waitF
9292
Collection<DiscoveryNode> childrenNodes = taskManager.startBanOnChildrenNodes(task.getId(), () -> {
9393
logger.trace("child tasks of parent [{}] are completed", taskId);
9494
groupedListener.onResponse(null);
95-
});
95+
}, reason);
9696
taskManager.cancel(task, reason, () -> {
9797
logger.trace("task [{}] is cancelled", taskId);
9898
groupedListener.onResponse(null);

server/src/main/java/org/opensearch/tasks/TaskManager.java

+12-4
Original file line numberDiff line numberDiff line change
@@ -510,17 +510,22 @@ public Set<TaskId> getBannedTaskIds() {
510510
return Collections.unmodifiableSet(banedParents.keySet());
511511
}
512512

513+
public Collection<DiscoveryNode> startBanOnChildrenNodes(long taskId, Runnable onChildTasksCompleted) {
514+
return startBanOnChildrenNodes(taskId, onChildTasksCompleted, "unknown");
515+
}
516+
513517
/**
514518
* Start rejecting new child requests as the parent task was cancelled.
515519
*
516520
* @param taskId the parent task id
517521
* @param onChildTasksCompleted called when all child tasks are completed or failed
522+
* @param reason the ban reason
518523
* @return the set of current nodes that have outstanding child tasks
519524
*/
520-
public Collection<DiscoveryNode> startBanOnChildrenNodes(long taskId, Runnable onChildTasksCompleted) {
525+
public Collection<DiscoveryNode> startBanOnChildrenNodes(long taskId, Runnable onChildTasksCompleted, String reason) {
521526
final CancellableTaskHolder holder = cancellableTasks.get(taskId);
522527
if (holder != null) {
523-
return holder.startBan(onChildTasksCompleted);
528+
return holder.startBan(onChildTasksCompleted, reason);
524529
} else {
525530
onChildTasksCompleted.run();
526531
return Collections.emptySet();
@@ -585,6 +590,7 @@ private static class CancellableTaskHolder {
585590
private List<Runnable> cancellationListeners = null;
586591
private Map<DiscoveryNode, Integer> childTasksPerNode = null;
587592
private boolean banChildren = false;
593+
private String banReason;
588594
private List<Runnable> childTaskCompletedListeners = null;
589595

590596
CancellableTaskHolder(CancellableTask task) {
@@ -662,7 +668,7 @@ public CancellableTask getTask() {
662668

663669
synchronized void registerChildNode(DiscoveryNode node) {
664670
if (banChildren) {
665-
throw new TaskCancelledException("The parent task was cancelled, shouldn't start any child tasks");
671+
throw new TaskCancelledException("The parent task was cancelled, shouldn't start any child tasks, " + banReason);
666672
}
667673
if (childTasksPerNode == null) {
668674
childTasksPerNode = new HashMap<>();
@@ -686,11 +692,13 @@ void unregisterChildNode(DiscoveryNode node) {
686692
notifyListeners(listeners);
687693
}
688694

689-
Set<DiscoveryNode> startBan(Runnable onChildTasksCompleted) {
695+
Set<DiscoveryNode> startBan(Runnable onChildTasksCompleted, String reason) {
690696
final Set<DiscoveryNode> pendingChildNodes;
691697
final Runnable toRun;
692698
synchronized (this) {
693699
banChildren = true;
700+
assert reason != null;
701+
banReason = reason;
694702
if (childTasksPerNode == null) {
695703
pendingChildNodes = Collections.emptySet();
696704
} else {

server/src/test/java/org/opensearch/action/admin/cluster/node/tasks/CancellableTasksTests.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,7 @@ public void testRegisterAndExecuteChildTaskWhileParentTaskIsBeingCanceled() thro
428428
);
429429
assertThat(cancelledException.getMessage(), startsWith("Task cancelled before it started:"));
430430
CountDownLatch latch = new CountDownLatch(1);
431-
taskManager.startBanOnChildrenNodes(parentTaskId.getId(), latch::countDown);
431+
taskManager.startBanOnChildrenNodes(parentTaskId.getId(), latch::countDown, cancelledException.getMessage());
432432
assertTrue("onChildTasksCompleted() is not invoked", latch.await(1, TimeUnit.SECONDS));
433433
}
434434

0 commit comments

Comments
 (0)