Skip to content

Commit f44d86a

Browse files
committed
Fix CancellableTasksIT
The main task can fail if it is canceled while some of its child tasks are not forked yet. This commit relaxes the assertion for that situation. Relates #54312
1 parent d6d664c commit f44d86a

File tree

2 files changed

+20
-7
lines changed

2 files changed

+20
-7
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ private void registerCancellableTask(Task task) {
183183
if (reason != null) {
184184
try {
185185
holder.cancel(reason);
186-
throw new IllegalStateException("Task cancelled before it started: " + reason);
186+
throw new TaskCancelledException("Task cancelled before it started: " + reason);
187187
} finally {
188188
// let's clean up the registration
189189
unregister(task);

server/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/CancellableTasksIT.java

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@
6868
import java.util.stream.Collectors;
6969
import java.util.stream.StreamSupport;
7070

71+
import static org.hamcrest.Matchers.containsString;
72+
import static org.hamcrest.Matchers.either;
7173
import static org.hamcrest.Matchers.empty;
7274
import static org.hamcrest.Matchers.equalTo;
7375
import static org.hamcrest.Matchers.hasSize;
@@ -146,7 +148,7 @@ public void testBanOnlyNodesWithOutstandingChildTasks() throws Exception {
146148
beforeExecuteLatches.get(req).countDown();
147149
}
148150
cancelFuture.actionGet();
149-
mainTaskFuture.actionGet();
151+
waitForMainTask(mainTaskFuture);
150152
assertBusy(() -> {
151153
for (DiscoveryNode node : nodes) {
152154
TaskManager taskManager = internalCluster().getInstance(TransportService.class, node.getName()).getTaskManager();
@@ -177,7 +179,7 @@ public void testCancelTaskMultipleTimes() throws Exception {
177179
}
178180
assertThat(cancelFuture.actionGet().getTaskFailures(), empty());
179181
assertThat(cancelFuture.actionGet().getTaskFailures(), empty());
180-
mainTaskFuture.actionGet();
182+
waitForMainTask(mainTaskFuture);
181183
CancelTasksResponse cancelError = client().admin().cluster().prepareCancelTasks()
182184
.setTaskId(taskId).waitForCompletion(randomBoolean()).get();
183185
assertThat(cancelError.getNodeFailures(), hasSize(1));
@@ -204,7 +206,7 @@ public void testDoNotWaitForCompletion() throws Exception {
204206
for (ChildRequest r : childRequests) {
205207
beforeExecuteLatches.get(r).countDown();
206208
}
207-
mainTaskFuture.actionGet();
209+
waitForMainTask(mainTaskFuture);
208210
}
209211

210212
TaskId getMainTaskId() {
@@ -214,6 +216,17 @@ TaskId getMainTaskId() {
214216
return listTasksResponse.getTasks().get(0).getTaskId();
215217
}
216218

219+
void waitForMainTask(ActionFuture<MainResponse> mainTask) {
220+
try {
221+
mainTask.actionGet();
222+
} catch (Exception e) {
223+
final Throwable cause = ExceptionsHelper.unwrap(e, TaskCancelledException.class);
224+
assertThat(cause.getMessage(),
225+
either(equalTo("The parent task was cancelled, shouldn't start any child tasks"))
226+
.or(containsString("Task cancelled before it started:")));
227+
}
228+
}
229+
217230
public static class MainRequest extends ActionRequest {
218231
final List<ChildRequest> childRequests;
219232

@@ -302,7 +315,7 @@ public Task createTask(long id, String type, String action, TaskId parentTaskId,
302315
return new CancellableTask(id, type, action, getDescription(), parentTaskId, headers) {
303316
@Override
304317
public boolean shouldCancelChildrenOnCancellation() {
305-
return false;
318+
return shouldCancelChildrenOnCancellation;
306319
}
307320
};
308321
} else {
@@ -364,15 +377,15 @@ protected void doExecute(Task task, MainRequest request, ActionListener<MainResp
364377
protected void startChildTask(TaskId parentTaskId, ChildRequest childRequest, ActionListener<ChildResponse> listener) {
365378
childRequest.setParentTask(parentTaskId);
366379
final CountDownLatch completeLatch = completedLatches.get(childRequest);
380+
LatchedActionListener<ChildResponse> latchedListener = new LatchedActionListener<>(listener, completeLatch);
367381
transportService.getThreadPool().generic().execute(new AbstractRunnable() {
368382
@Override
369383
public void onFailure(Exception e) {
370-
throw new AssertionError(e);
384+
listener.onFailure(e);
371385
}
372386

373387
@Override
374388
protected void doRun() {
375-
LatchedActionListener<ChildResponse> latchedListener = new LatchedActionListener<>(listener, completeLatch);
376389
if (client.getLocalNodeId().equals(childRequest.targetNode.getId()) && randomBoolean()) {
377390
try {
378391
client.executeLocally(TransportChildAction.ACTION, childRequest, latchedListener);

0 commit comments

Comments
 (0)