Skip to content

Commit

Permalink
fix: Fix flaky test ScheduledRetryingExecutorTest.testCancelOuterFutu…
Browse files Browse the repository at this point in the history
…reAfterStart (#3335)

The test was flaky because the
[Thread.sleep(150)](https://github.com/googleapis/sdk-platform-java/blob/b031b18a9bb35b77ca21d3665217aa4c219ced57/gax-java/gax/src/test/java/com/google/api/gax/retrying/ScheduledRetryingExecutorTest.java#L288)
does not guarantee the main thread would wake up after exactly 150ms, it
might take much longer if the resource is limited on the machine or we
are running tests in parallel.
Hence if the thread sleeps more than
[1525ms](https://github.com/googleapis/sdk-platform-java/blob/0cddadb8ad3eddfffa356a479964d8a720937503/gax-java/gax/src/test/java/com/google/api/gax/retrying/ScheduledRetryingExecutorTest.java#L262),
the future would be completed already, and the cancellation of the
future would result in false.
Update the total request duration to 32500ms now, it is still not bullet
proof but we would have much less chance to run into flaky tests now.

fixes: #2669
  • Loading branch information
blakeli0 authored Nov 6, 2024
1 parent 0cddadb commit e73740d
Showing 1 changed file with 6 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,13 @@ void testCancelGetAttempt(boolean withCustomRetrySettings) throws Exception {
setUp(withCustomRetrySettings);
for (int executionsCount = 0; executionsCount < EXECUTIONS_COUNT; executionsCount++) {
ScheduledExecutorService localExecutor = Executors.newSingleThreadScheduledExecutor();
final int maxRetries = 100;
final int maxRetries = 20;

FailingCallable callable = new FailingCallable(maxRetries - 1, "request", "SUCCESS", tracer);
RetrySettings retrySettings =
FAST_RETRY_SETTINGS
.toBuilder()
.setTotalTimeoutDuration(java.time.Duration.ofMillis(1000L))
.setTotalTimeoutDuration(java.time.Duration.ofMillis(5000L))
.setMaxAttempts(maxRetries)
.build();

Expand Down Expand Up @@ -259,10 +259,11 @@ void testCancelOuterFutureAfterStart() throws Exception {
.toBuilder()
// These params were selected to ensure that future tries to run and fail (at least
// once) but does not complete before it is cancelled. Assuming no computation time,
// it would take 25 + 100 + 400 + 1000 = 1525ms for the future to complete, which should
// it would take 2500 + 10000 + 10000 + 10000 = 32500ms for the future to complete,
// which should
// be more than enough time to cancel the future.
.setInitialRetryDelayDuration(java.time.Duration.ofMillis(25L))
.setMaxRetryDelayDuration(java.time.Duration.ofMillis(1000L))
.setInitialRetryDelayDuration(java.time.Duration.ofMillis(2500L))
.setMaxRetryDelayDuration(java.time.Duration.ofMillis(10000L))
.setRetryDelayMultiplier(4.0)
.setTotalTimeoutDuration(java.time.Duration.ofMillis(60000L))
// Set this test to not use jitter as the randomized retry delay (RRD) may introduce
Expand Down

0 comments on commit e73740d

Please sign in to comment.