Skip to content

Commit f2ac594

Browse files
committed
Use test cluster exclusive in CancellableTasksIT (#67058)
testRemoveBanParentsOnDisconnect disconnects some nodes and leaves the cluster in an unhealthy state, which fails other tests. We can reconnect nodes after that test. However, this commit chooses to run this suite test-cluster exclusive instead because it is simpler and more stable. Closes #6652
1 parent 8ef5d3b commit f2ac594

File tree

1 file changed

+40
-43
lines changed

1 file changed

+40
-43
lines changed

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

Lines changed: 40 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858
import org.elasticsearch.transport.TransportException;
5959
import org.elasticsearch.transport.TransportResponseHandler;
6060
import org.elasticsearch.transport.TransportService;
61-
import org.junit.Before;
61+
import org.junit.After;
6262

6363
import java.io.IOException;
6464
import java.util.ArrayList;
@@ -81,6 +81,7 @@
8181
import static org.hamcrest.Matchers.hasSize;
8282
import static org.hamcrest.Matchers.instanceOf;
8383

84+
@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST)
8485
public class CancellableTasksIT extends ESIntegTestCase {
8586

8687
static int idGenerator = 0;
@@ -89,13 +90,14 @@ public class CancellableTasksIT extends ESIntegTestCase {
8990
static final Map<TestRequest, CountDownLatch> beforeExecuteLatches = ConcurrentCollections.newConcurrentMap();
9091
static final Map<TestRequest, CountDownLatch> completedLatches = ConcurrentCollections.newConcurrentMap();
9192

92-
@Before
93-
public void resetTestStates() {
94-
idGenerator = 0;
95-
beforeSendLatches.clear();
96-
arrivedLatches.clear();
97-
beforeExecuteLatches.clear();
98-
completedLatches.clear();
93+
@After
94+
public void ensureAllBansRemoved() throws Exception {
95+
assertBusy(() -> {
96+
for (String node : internalCluster().getNodeNames()) {
97+
TaskManager taskManager = internalCluster().getInstance(TransportService.class, node).getTaskManager();
98+
assertThat("node " + node, taskManager.getBannedTaskIds(), empty());
99+
}
100+
}, 30, TimeUnit.SECONDS);
99101
}
100102

101103
static TestRequest generateTestRequest(Set<DiscoveryNode> nodes, int level, int maxLevel) {
@@ -138,7 +140,7 @@ static Set<TestRequest> allowPartialRequest(TestRequest request) throws Exceptio
138140
beforeSendLatches.get(req).countDown();
139141
}
140142
for (TestRequest req : sentRequests) {
141-
arrivedLatches.get(req).await();
143+
assertTrue(arrivedLatches.get(req).await(60, TimeUnit.SECONDS));
142144
}
143145
Set<TestRequest> completedRequests = new HashSet<>();
144146
for (TestRequest req : randomSubsetOf(sentRequests)) {
@@ -151,7 +153,7 @@ static Set<TestRequest> allowPartialRequest(TestRequest request) throws Exceptio
151153
beforeExecuteLatches.get(req).countDown();
152154
}
153155
for (TestRequest req : completedRequests) {
154-
completedLatches.get(req).await();
156+
assertTrue(completedLatches.get(req).await(60, TimeUnit.SECONDS));
155157
}
156158
return Sets.difference(sentRequests, completedRequests);
157159
}
@@ -164,21 +166,12 @@ static void allowEntireRequest(TestRequest request) {
164166
}
165167
}
166168

167-
void ensureAllBansRemoved() throws Exception {
168-
assertBusy(() -> {
169-
for (String node : internalCluster().getNodeNames()) {
170-
TaskManager taskManager = internalCluster().getInstance(TransportService.class, node).getTaskManager();
171-
assertThat("node " + node, taskManager.getBannedTaskIds(), empty());
172-
}
173-
}, 30, TimeUnit.SECONDS);
174-
}
175-
176169
public void testBanOnlyNodesWithOutstandingDescendantTasks() throws Exception {
177170
if (randomBoolean()) {
178-
internalCluster().startNodes(randomIntBetween(1, 3));
171+
internalCluster().startNodes(1);
179172
}
180173
Set<DiscoveryNode> nodes = StreamSupport.stream(clusterService().state().nodes().spliterator(), false).collect(Collectors.toSet());
181-
final TestRequest rootRequest = generateTestRequest(nodes, 0, between(1, 4));
174+
final TestRequest rootRequest = generateTestRequest(nodes, 0, between(1, 3));
182175
ActionFuture<TestResponse> rootTaskFuture = client().execute(TransportTestAction.ACTION, rootRequest);
183176
Set<TestRequest> pendingRequests = allowPartialRequest(rootRequest);
184177
TaskId rootTaskId = getRootTaskId(rootRequest);
@@ -191,28 +184,31 @@ public void testBanOnlyNodesWithOutstandingDescendantTasks() throws Exception {
191184
client().admin().cluster().prepareCancelTasks().setTaskId(subTask.getTaskId()).waitForCompletion(false).get();
192185
}
193186
}
194-
assertBusy(() -> {
195-
for (DiscoveryNode node : nodes) {
196-
TaskManager taskManager = internalCluster().getInstance(TransportService.class, node.getName()).getTaskManager();
197-
Set<TaskId> expectedBans = new HashSet<>();
198-
for (TestRequest req : pendingRequests) {
199-
if (req.node.equals(node)) {
200-
List<Task> childTasks = taskManager.getTasks().values().stream()
201-
.filter(t -> t.getParentTaskId() != null && t.getDescription().equals(req.taskDescription()))
202-
.collect(Collectors.toList());
203-
assertThat(childTasks, hasSize(1));
204-
CancellableTask childTask = (CancellableTask) childTasks.get(0);
205-
assertTrue(childTask.isCancelled());
206-
expectedBans.add(childTask.getParentTaskId());
187+
try {
188+
assertBusy(() -> {
189+
for (DiscoveryNode node : nodes) {
190+
TaskManager taskManager = internalCluster().getInstance(TransportService.class, node.getName()).getTaskManager();
191+
Set<TaskId> expectedBans = new HashSet<>();
192+
for (TestRequest req : pendingRequests) {
193+
if (req.node.equals(node)) {
194+
List<Task> childTasks = taskManager.getTasks().values().stream()
195+
.filter(t -> t.getParentTaskId() != null && t.getDescription().equals(req.taskDescription()))
196+
.collect(Collectors.toList());
197+
assertThat(childTasks, hasSize(1));
198+
CancellableTask childTask = (CancellableTask) childTasks.get(0);
199+
assertTrue(childTask.isCancelled());
200+
expectedBans.add(childTask.getParentTaskId());
201+
}
207202
}
203+
assertThat(taskManager.getBannedTaskIds(), equalTo(expectedBans));
208204
}
209-
assertThat(taskManager.getBannedTaskIds(), equalTo(expectedBans));
210-
}
211-
}, 30, TimeUnit.SECONDS);
212-
allowEntireRequest(rootRequest);
213-
cancelFuture.actionGet();
214-
waitForRootTask(rootTaskFuture);
215-
ensureAllBansRemoved();
205+
}, 30, TimeUnit.SECONDS);
206+
} finally {
207+
allowEntireRequest(rootRequest);
208+
cancelFuture.actionGet();
209+
waitForRootTask(rootTaskFuture);
210+
ensureAllBansRemoved();
211+
}
216212
}
217213

218214
public void testCancelTaskMultipleTimes() throws Exception {
@@ -326,6 +322,7 @@ static void waitForRootTask(ActionFuture<TestResponse> rootTask) {
326322
rootTask.actionGet();
327323
} catch (Exception e) {
328324
final Throwable cause = ExceptionsHelper.unwrap(e, TaskCancelledException.class);
325+
assertNotNull(cause);
329326
assertThat(cause.getMessage(), anyOf(
330327
equalTo("The parent task was cancelled, shouldn't start any child tasks"),
331328
containsString("Task cancelled before it started:"),
@@ -441,7 +438,7 @@ protected void doExecute(Task task, TestRequest request, ActionListener<TestResp
441438
GroupedActionListener<TestResponse> groupedListener =
442439
new GroupedActionListener<>(listener.map(r -> new TestResponse()), subRequests.size() + 1);
443440
transportService.getThreadPool().generic().execute(ActionRunnable.supply(groupedListener, () -> {
444-
beforeExecuteLatches.get(request).await();
441+
assertTrue(beforeExecuteLatches.get(request).await(60, TimeUnit.SECONDS));
445442
if (((CancellableTask) task).isCancelled()) {
446443
throw new TaskCancelledException("Task was cancelled while executing");
447444
}
@@ -465,7 +462,7 @@ public void onFailure(Exception e) {
465462

466463
@Override
467464
protected void doRun() throws Exception {
468-
beforeSendLatches.get(subRequest).await();
465+
assertTrue(beforeSendLatches.get(subRequest).await(60, TimeUnit.SECONDS));
469466
if (client.getLocalNodeId().equals(subRequest.node.getId()) && randomBoolean()) {
470467
try {
471468
client.executeLocally(TransportTestAction.ACTION, subRequest, latchedListener);

0 commit comments

Comments
 (0)