Skip to content

Commit bc27ad8

Browse files
Fix flaky tests in GoogleCloudStorageBlobStoreRepositoryTests, S3BlobStoreRepositoryTests, AzureBlobStoreRepositoryTests (#18290)
Signed-off-by: kkewwei <kewei.11@bytedance.com> Signed-off-by: kkewwei <kkewwei@163.com> (cherry picked from commit bde7db5) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent d21c4c9 commit bc27ad8

File tree

4 files changed

+17
-14
lines changed

4 files changed

+17
-14
lines changed

plugins/repository-azure/src/internalClusterTest/java/org/opensearch/repositories/azure/AzureBlobStoreRepositoryTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ protected Map<String, HttpHandler> createHttpHandlers() {
9595

9696
@Override
9797
protected HttpHandler createErroneousHttpHandler(final HttpHandler delegate) {
98-
return new AzureErroneousHttpHandler(delegate, randomIntBetween(2, 3));
98+
return new AzureErroneousHttpHandler(delegate, randomDoubleBetween(0, 0.25, false));
9999
}
100100

101101
@Override
@@ -163,8 +163,8 @@ private static class AzureBlobStoreHttpHandler extends AzureHttpHandler implemen
163163
@SuppressForbidden(reason = "this test uses a HttpServer to emulate an Azure endpoint")
164164
private static class AzureErroneousHttpHandler extends ErroneousHttpHandler {
165165

166-
AzureErroneousHttpHandler(final HttpHandler delegate, final int maxErrorsPerRequest) {
167-
super(delegate, maxErrorsPerRequest);
166+
AzureErroneousHttpHandler(final HttpHandler delegate, final double maxErrorsPercentage) {
167+
super(delegate, maxErrorsPercentage);
168168
}
169169

170170
@Override

plugins/repository-gcs/src/internalClusterTest/java/org/opensearch/repositories/gcs/GoogleCloudStorageBlobStoreRepositoryTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ protected Map<String, HttpHandler> createHttpHandlers() {
112112

113113
@Override
114114
protected HttpHandler createErroneousHttpHandler(final HttpHandler delegate) {
115-
return new GoogleErroneousHttpHandler(delegate, randomIntBetween(2, 3));
115+
return new GoogleErroneousHttpHandler(delegate, randomDoubleBetween(0, 0.25, false));
116116
}
117117

118118
@Override
@@ -305,8 +305,8 @@ private static class GoogleCloudStorageBlobStoreHttpHandler extends GoogleCloudS
305305
@SuppressForbidden(reason = "this test uses a HttpServer to emulate a Google Cloud Storage endpoint")
306306
private static class GoogleErroneousHttpHandler extends ErroneousHttpHandler {
307307

308-
GoogleErroneousHttpHandler(final HttpHandler delegate, final int maxErrorsPerRequest) {
309-
super(delegate, maxErrorsPerRequest);
308+
GoogleErroneousHttpHandler(final HttpHandler delegate, final double maxErrorsPercentage) {
309+
super(delegate, maxErrorsPercentage);
310310
}
311311

312312
@Override

plugins/repository-s3/src/internalClusterTest/java/org/opensearch/repositories/s3/S3BlobStoreRepositoryTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ protected Map<String, HttpHandler> createHttpHandlers() {
150150

151151
@Override
152152
protected HttpHandler createErroneousHttpHandler(final HttpHandler delegate) {
153-
return new S3StatsCollectorHttpHandler(new S3ErroneousHttpHandler(delegate, randomIntBetween(2, 3)));
153+
return new S3StatsCollectorHttpHandler(new S3ErroneousHttpHandler(delegate, randomDoubleBetween(0, 0.25, false)));
154154
}
155155

156156
@Override
@@ -395,8 +395,8 @@ private void validateAuthHeader(HttpExchange exchange) {
395395
@SuppressForbidden(reason = "this test uses a HttpServer to emulate an S3 endpoint")
396396
private static class S3ErroneousHttpHandler extends ErroneousHttpHandler {
397397

398-
S3ErroneousHttpHandler(final HttpHandler delegate, final int maxErrorsPerRequest) {
399-
super(delegate, maxErrorsPerRequest);
398+
S3ErroneousHttpHandler(final HttpHandler delegate, final double maxErrorsPercentage) {
399+
super(delegate, maxErrorsPercentage);
400400
}
401401

402402
@Override

test/framework/src/main/java/org/opensearch/repositories/blobstore/OpenSearchMockAPIBasedRepositoryIntegTestCase.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -277,14 +277,15 @@ protected abstract static class ErroneousHttpHandler implements DelegatingHttpHa
277277
private final Map<String, AtomicInteger> requests;
278278

279279
private final HttpHandler delegate;
280-
private final int maxErrorsPerRequest;
280+
private final double maxErrorsPercentage;
281281

282282
@SuppressForbidden(reason = "this test uses a HttpServer to emulate a cloud-based storage service")
283-
protected ErroneousHttpHandler(final HttpHandler delegate, final int maxErrorsPerRequest) {
283+
protected ErroneousHttpHandler(final HttpHandler delegate, final double maxErrorsPercentage) {
284284
this.requests = new ConcurrentHashMap<>();
285285
this.delegate = delegate;
286-
this.maxErrorsPerRequest = maxErrorsPerRequest;
287-
assert maxErrorsPerRequest > 1;
286+
this.maxErrorsPercentage = maxErrorsPercentage;
287+
// We don't want to fail too often as it will cost too much time, which will lead to flaky tests
288+
assert maxErrorsPercentage >= 0 && maxErrorsPercentage <= 0.25;
288289
}
289290

290291
@Override
@@ -295,7 +296,9 @@ public void handle(final HttpExchange exchange) throws IOException {
295296

296297
final boolean canFailRequest = canFailRequest(exchange);
297298
final int count = requests.computeIfAbsent(requestId, req -> new AtomicInteger(0)).incrementAndGet();
298-
if (count >= maxErrorsPerRequest || canFailRequest == false) {
299+
// We should not fail more than 3 times as the default max retry count is 3 (see SdkDefaultRetrySetting.maxAttempts), the
300+
// request will fail when retry count > 3.
301+
if (random().nextDouble() > maxErrorsPercentage || count >= 3 || canFailRequest == false) {
299302
requests.remove(requestId);
300303
delegate.handle(exchange);
301304
} else {

0 commit comments

Comments
 (0)