Skip to content

Commit a89a4a1

Browse files
Remove needless use of assertBusy polling Future.isDone (#119078) (#119138)
Polling `Future.isDone` is obviously nonesense. Suprisingly enough, We can save a couple seconds fixing up these spots (especially the CCS ones).
1 parent fb45fd7 commit a89a4a1

File tree

7 files changed

+27
-24
lines changed

7 files changed

+27
-24
lines changed

modules/reindex/src/test/java/org/elasticsearch/reindex/AsyncBulkByScrollActionTests.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,6 @@ public void testStartRetriesOnRejectionButFailsOnTooManyRejections() throws Exce
199199
DummyAsyncBulkByScrollAction action = new DummyActionWithoutBackoff();
200200
action.start();
201201
assertBusy(() -> assertEquals(testRequest.getMaxRetries() + 1, client.searchAttempts.get()));
202-
assertBusy(() -> assertTrue(listener.isDone()));
203202
ExecutionException e = expectThrows(ExecutionException.class, () -> listener.get());
204203
assertThat(ExceptionsHelper.stackTrace(e), containsString(EsRejectedExecutionException.class.getSimpleName()));
205204
assertNull("There shouldn't be a search attempt pending that we didn't reject", client.lastSearch.get());

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,11 +252,13 @@ public void testDoNotWaitForCompletion() throws Exception {
252252
if (waitForCompletion) {
253253
assertFalse(cancelFuture.isDone());
254254
} else {
255-
assertBusy(() -> assertTrue(cancelFuture.isDone()));
255+
cancelFuture.get();
256256
}
257257
allowEntireRequest(rootRequest);
258258
waitForRootTask(mainTaskFuture, false);
259-
cancelFuture.actionGet();
259+
if (waitForCompletion) {
260+
cancelFuture.actionGet();
261+
}
260262
ensureBansAndCancellationsConsistency();
261263
}
262264

server/src/internalClusterTest/java/org/elasticsearch/search/ccs/CrossClusterIT.java

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,11 @@ public void testProxyConnectionDisconnect() throws Exception {
183183
}
184184
}
185185
});
186-
assertBusy(() -> assertTrue(future.isDone()));
186+
try {
187+
future.get();
188+
} catch (ExecutionException e) {
189+
// ignored
190+
}
187191
configureAndConnectsToRemoteClusters();
188192
} finally {
189193
SearchListenerPlugin.allowQueryPhase();
@@ -298,20 +302,21 @@ public void testCancel() throws Exception {
298302
}
299303

300304
SearchListenerPlugin.allowQueryPhase();
301-
assertBusy(() -> assertTrue(queryFuture.isDone()));
302-
assertBusy(() -> assertTrue(cancelFuture.isDone()));
305+
try {
306+
queryFuture.get();
307+
fail("query should have failed");
308+
} catch (ExecutionException e) {
309+
assertNotNull(e.getCause());
310+
Throwable t = ExceptionsHelper.unwrap(e, TaskCancelledException.class);
311+
assertNotNull(t);
312+
}
313+
cancelFuture.get();
303314
assertBusy(() -> {
304315
final Iterable<TransportService> transportServices = cluster("cluster_a").getInstances(TransportService.class);
305316
for (TransportService transportService : transportServices) {
306317
assertThat(transportService.getTaskManager().getBannedTaskIds(), Matchers.empty());
307318
}
308319
});
309-
310-
ExecutionException e = expectThrows(ExecutionException.class, () -> queryFuture.result());
311-
assertNotNull(e);
312-
assertNotNull(e.getCause());
313-
Throwable t = ExceptionsHelper.unwrap(e, TaskCancelledException.class);
314-
assertNotNull(t);
315320
}
316321

317322
/**

server/src/internalClusterTest/java/org/elasticsearch/search/ccs/CrossClusterSearchIT.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -379,11 +379,9 @@ public void testClusterDetailsAfterCCSWithFailuresOnRemoteClusterOnly() throws E
379379
r.incRef();
380380
l.onResponse(r);
381381
}));
382-
assertBusy(() -> assertTrue(queryFuture.isDone()));
383-
384382
// dfs=true overrides the minimize_roundtrips=true setting and does not minimize roundtrips
385383
if (skipUnavailable == false && minimizeRoundtrips && dfs == false) {
386-
ExecutionException ee = expectThrows(ExecutionException.class, () -> queryFuture.get());
384+
ExecutionException ee = expectThrows(ExecutionException.class, queryFuture::get);
387385
assertNotNull(ee.getCause());
388386
assertThat(ee.getCause(), instanceOf(RemoteTransportException.class));
389387
Throwable rootCause = ExceptionsHelper.unwrap(ee.getCause(), IllegalStateException.class);
@@ -622,10 +620,8 @@ public void testRemoteClusterOnlyCCSWithFailuresOnAllShards() throws Exception {
622620
r.incRef();
623621
l.onResponse(r);
624622
}));
625-
assertBusy(() -> assertTrue(queryFuture.isDone()));
626-
627623
if (skipUnavailable == false || minimizeRoundtrips == false) {
628-
ExecutionException ee = expectThrows(ExecutionException.class, () -> queryFuture.get());
624+
ExecutionException ee = expectThrows(ExecutionException.class, queryFuture::get);
629625
assertNotNull(ee.getCause());
630626
Throwable rootCause = ExceptionsHelper.unwrap(ee, IllegalStateException.class);
631627
assertThat(rootCause.getMessage(), containsString("index corrupted"));

server/src/internalClusterTest/java/org/elasticsearch/snapshots/ConcurrentSnapshotsIT.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1361,9 +1361,12 @@ public void testConcurrentOperationsLimit() throws Exception {
13611361
if (deleteAndAbortAll) {
13621362
awaitNumberOfSnapshotsInProgress(0);
13631363
for (ActionFuture<CreateSnapshotResponse> snapshotFuture : snapshotFutures) {
1364-
// just check that the futures resolve, whether or not things worked out with the snapshot actually finalizing or failing
1365-
// due to the abort does not matter
1366-
assertBusy(() -> assertTrue(snapshotFuture.isDone()));
1364+
try {
1365+
snapshotFuture.get();
1366+
} catch (ExecutionException e) {
1367+
// just check that the futures resolve, whether or not things worked out with the snapshot actually finalizing or
1368+
// failing due to the abort does not matter
1369+
}
13671370
}
13681371
assertThat(getRepositoryData(repoName).getSnapshotIds(), empty());
13691372
} else {

server/src/test/java/org/elasticsearch/action/admin/indices/diskusage/TransportAnalyzeIndexDiskUsageActionTests.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,6 @@ public void testSimpleLimitRequests() throws Exception {
132132
assertBusy(() -> assertThat(transportService.getRequestsSentPerNode(), equalTo(expectedRequestCounts)));
133133
pendingRequests.addAll(transportService.getCapturedRequests(true));
134134
}
135-
assertBusy(future::isDone);
136135
AnalyzeIndexDiskUsageResponse response = future.actionGet();
137136
assertThat(response.getTotalShards(), equalTo(numberOfShards));
138137
assertThat(response.getFailedShards(), equalTo(0));

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plugin/ComputeListenerTests.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -460,8 +460,7 @@ public void testCancelOnFailure() throws Exception {
460460
}), TimeValue.timeValueNanos(between(0, 100)), threadPool.generic());
461461
}
462462
}
463-
assertBusy(rootListener::isDone);
464-
ExecutionException failure = expectThrows(ExecutionException.class, () -> rootListener.get(1, TimeUnit.SECONDS));
463+
ExecutionException failure = expectThrows(ExecutionException.class, () -> rootListener.get(10, TimeUnit.SECONDS));
465464
Throwable cause = failure.getCause();
466465
assertNotNull(failure);
467466
assertThat(cause, instanceOf(CircuitBreakingException.class));

0 commit comments

Comments
 (0)