Skip to content

Fix flaky testWriteLargeBlob in AzureBlobContainerRetriesTests #145748

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ankurs47 wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Fix flaky testWriteLargeBlob in AzureBlobContainerRetriesTests #145748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dc2312c
fix flaky test
ankurs47 Apr 6, 2026
5acc548
Address pr comments
ankurs47 Apr 8, 2026
8f87bb1
Address pr comments related to unused code and timeout tests.
ankurs47 Apr 9, 2026
eab07dc
[CI] Auto commit changes from spotless
Apr 9, 2026
8dcd429
Merge branch 'main' into 145654_AzureBlobContainerRetriesTests_fix
ankurs47 Apr 9, 2026
6d62690
Merge branch 'main' into 145654_AzureBlobContainerRetriesTests_fix
ankurs47 Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions ...re/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ public void testWriteBlobWithRetries() throws Exception {
}

public void testWriteLargeBlob() throws Exception {
final int maxRetries = randomIntBetween(2, 5);
final int maxRetries = randomIntBetween(4, 8);
logger.info("--> max retries: {}", maxRetries);

final byte[] data = randomBytes(ByteSizeUnit.MB.toIntBytes(10) + randomIntBetween(0, ByteSizeUnit.MB.toIntBytes(1)));
Expand Down Expand Up @@ -427,7 +427,6 @@ public void testWriteLargeBlob() throws Exception {
try (InputStream stream = new InputStreamIndexInput(new ByteArrayIndexInput("desc", data), data.length)) {
blobContainer.writeBlob(randomPurpose(), "write_large_blob", stream, data.length, false);
}

assertThat(countDownUploads.get(), equalTo(0));
assertThat(countDownComplete.isCountedDown(), is(true));
assertThat(blocks.isEmpty(), is(true));
Expand Down Expand Up @@ -669,11 +668,11 @@ public void testRetriesAreTerminatedWhenClientProviderIsClosed() {
}

private BlobContainer createBlobContainer(int maxRetries, String secondaryHost, LocationMode locationMode) {
return createBlobContainer(maxRetries, null, null, null, null, null, BlobPath.EMPTY, secondaryHost, locationMode);
return createBlobContainer(maxRetries, null, null, null, null, null, null, BlobPath.EMPTY, secondaryHost, locationMode);
}

private BlobContainer createBlobContainer(int maxRetries) {
return createBlobContainer(maxRetries, null, null, null, null, null, null);
return createBlobContainer(maxRetries, null, null, null, null, null, null, null);
}

@Override
Expand All @@ -694,17 +693,19 @@ protected Class<? extends Exception> unresponsiveExceptionType() {

@Override
protected BlobContainer createBlobContainer(
@Nullable Integer maxRetries,
@Nullable TimeValue readTimeout,
@Nullable Boolean disableChunkedEncoding,
@Nullable Integer maxConnections,
@Nullable ByteSizeValue bufferSize,
@Nullable Integer maxBulkDeletes,
@Nullable BlobPath blobContainerPath
final @Nullable Integer maxRetries,
final @Nullable TimeValue readTimeout,
final @Nullable TimeValue requestTimeout,
final @Nullable Boolean disableChunkedEncoding,
final @Nullable Integer maxConnections,
final @Nullable ByteSizeValue bufferSize,
final @Nullable Integer maxBulkDeletes,
final @Nullable BlobPath blobContainerPath
) {
return createBlobContainer(
maxRetries,
readTimeout,
requestTimeout,
disableChunkedEncoding,
maxConnections,
bufferSize,
Expand All @@ -718,6 +719,7 @@ protected BlobContainer createBlobContainer(
private BlobContainer createBlobContainer(
@Nullable Integer maxRetries,
@Nullable TimeValue readTimeout,
@Nullable TimeValue timeout,
@Nullable Boolean disableChunkedEncoding,
@Nullable Integer maxConnections,
@Nullable ByteSizeValue bufferSize,
Expand All @@ -742,7 +744,11 @@ private BlobContainer createBlobContainer(
if (maxRetries != null) {
clientSettings.put(MAX_RETRIES_SETTING.getConcreteSettingForNamespace(clientName).getKey(), maxRetries);
}
clientSettings.put(TIMEOUT_SETTING.getConcreteSettingForNamespace(clientName).getKey(), TimeValue.timeValueSeconds(1));
if (timeout != null) {
Copy link
Copy Markdown
Member

@DaveCTurner DaveCTurner Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ever not null?

clientSettings.put(TIMEOUT_SETTING.getConcreteSettingForNamespace(clientName).getKey(), timeout);
} else {
clientSettings.put(TIMEOUT_SETTING.getConcreteSettingForNamespace(clientName).getKey(), SAFE_AWAIT_TIMEOUT);
Copy link
Copy Markdown
Member

@DaveCTurner DaveCTurner Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ok for the timeout-related tests or does it slow them down too far?

Copy link
Copy Markdown
Member

@DaveCTurner DaveCTurner Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes before this change testReadBlobWithReadTimeouts takes about 15s, but now it takes way way longer. I think we should use SAFE_AWAIT_TIMEOUT by default, but a much shorter timeout for the tests that expect timeouts.

}
if (readTimeout != null) {
clientSettings.put(READ_TIMEOUT_SETTING.getConcreteSettingForNamespace(clientName).getKey(), readTimeout);
}
Expand Down
3 changes: 2 additions & 1 deletion .../java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ protected Class<? extends Exception> unresponsiveExceptionType() {
protected BlobContainer createBlobContainer(
final @Nullable Integer maxRetries,
final @Nullable TimeValue readTimeout,
final @Nullable TimeValue requestTimeout,
final @Nullable Boolean disableChunkedEncoding,
final @Nullable Integer maxConnections,
final @Nullable ByteSizeValue bufferSize,
Expand Down Expand Up @@ -633,7 +634,7 @@ public void testContentsChangeWhileStreaming() throws IOException {
// The blob needs to be large enough that it won't be entirely buffered on the first request
final int enoughBytesToNotBeEntirelyBuffered = Math.toIntExact(ByteSizeValue.ofMb(30).getBytes());

final BlobContainer container = createBlobContainer(1, null, null, null, null, null, null);
final BlobContainer container = createBlobContainer(1, null, null, null, null, null, null, null);

final String key = randomIdentifier();
byte[] initialValue = randomByteArrayOfLength(enoughBytesToNotBeEntirelyBuffered);
Expand Down
13 changes: 7 additions & 6 deletions ...itory-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobContainerRetriesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ protected Class<? extends Exception> unresponsiveExceptionType() {
protected BlobContainer createBlobContainer(
final @Nullable Integer maxRetries,
final @Nullable TimeValue readTimeout,
final @Nullable TimeValue requestTimeout,
final @Nullable Boolean disableChunkedEncoding,
final @Nullable Integer maxConnections,
final @Nullable ByteSizeValue bufferSize,
Expand Down Expand Up @@ -599,7 +600,7 @@ public void testMaxConnections() throws InterruptedException, IOException {
final CountDownLatch requestReceived = new CountDownLatch(1);
final CountDownLatch releaseRequest = new CountDownLatch(1);
int maxConnections = 1;
final BlobContainer blobContainer = createBlobContainer(null, null, null, maxConnections, null, null, null);
final BlobContainer blobContainer = createBlobContainer(null, null, null, null, maxConnections, null, null, null);

// Setting up a simple request handler that returns NOT_FOUND, so as to avoid setting up a response.
@SuppressForbidden(reason = "use a http server")
Expand Down Expand Up @@ -1267,7 +1268,7 @@ public void testRetryOn403InStateless() {
final var denyAccessAfterAttempt = between(1, 5);
logger.info("--> maxRetries = {}, denyAccessAfterAttempt = {}", maxRetries, denyAccessAfterAttempt);
final var blobContainerPath = BlobPath.EMPTY.add(getTestName());
final var statefulBlobContainer = createBlobContainer(maxRetries, null, null, null, null, null, blobContainerPath);
final var statefulBlobContainer = createBlobContainer(maxRetries, null, null, null, null, null, null, blobContainerPath);
final var requestCount = new AtomicInteger();
final var invalidAccessKeyIdResponseCount = new AtomicInteger();
final var accessDeniedResponseCount = new AtomicInteger();
Expand Down Expand Up @@ -1330,7 +1331,7 @@ public void testRetryOn403InStateless() {
);
service.start();
recordingMeterRegistry = new RecordingMeterRegistry();
final var statelessBlobContainer = createBlobContainer(maxRetries, null, null, null, null, null, blobContainerPath);
final var statelessBlobContainer = createBlobContainer(maxRetries, null, null, null, null, null, null, blobContainerPath);

assertThat(
ExceptionsHelper.unwrap(
Expand All @@ -1349,7 +1350,7 @@ public void testRetryOn403InStateless() {

public void testUploadNotFoundInCompareAndExchange() {
final var blobContainerPath = BlobPath.EMPTY.add(getTestName());
final var statefulBlobContainer = createBlobContainer(1, null, null, null, null, null, blobContainerPath);
final var statefulBlobContainer = createBlobContainer(1, null, null, null, null, null, null, blobContainerPath);

@SuppressForbidden(reason = "use a http server")
class RejectsUploadPartRequests extends S3HttpHandler {
Expand Down Expand Up @@ -1386,7 +1387,7 @@ public void handle(HttpExchange exchange) throws IOException {

public void testCompareAndExchangeWithConcurrentPutObject() throws Exception {
final var blobContainerPath = BlobPath.EMPTY.add(getTestName());
final var statefulBlobContainer = createBlobContainer(1, null, null, null, null, null, blobContainerPath);
final var statefulBlobContainer = createBlobContainer(1, null, null, null, null, null, null, blobContainerPath);

final var objectContentsRequestedLatch = new CountDownLatch(1);

Expand Down Expand Up @@ -1460,7 +1461,7 @@ public void testContentsChangeWhileStreaming() throws IOException {
// The blob needs to be large enough that it won't be entirely buffered on the first request
final int enoughBytesToNotBeEntirelyBuffered = Math.toIntExact(ByteSizeValue.ofMb(30).getBytes());

final BlobContainer container = createBlobContainer(1, null, null, null, null, null, null);
final BlobContainer container = createBlobContainer(1, null, null, null, null, null, null, null);

final String key = randomIdentifier();
byte[] initialValue = randomByteArrayOfLength(enoughBytesToNotBeEntirelyBuffered);
Expand Down
1 change: 1 addition & 0 deletions ...rl/src/test/java/org/elasticsearch/common/blobstore/url/URLBlobContainerRetriesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ protected Matcher<Object> readTimeoutExceptionMatcher() {
protected BlobContainer createBlobContainer(
Integer maxRetries,
TimeValue readTimeout,
TimeValue requestTimeout,
Boolean disableChunkedEncoding,
Integer maxConnections,
ByteSizeValue bufferSize,
Expand Down
3 changes: 0 additions & 3 deletions muted-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,6 @@ tests:
- class: org.elasticsearch.xpack.esql.heap_attack.HeapAttackLimitByIT
method: testTopNByKeyEncoderTooMuchMemory
issue: https://github.com/elastic/elasticsearch/issues/145627
- class: org.elasticsearch.repositories.azure.AzureBlobContainerRetriesTests
method: testWriteLargeBlob
issue: https://github.com/elastic/elasticsearch/issues/145654
- class: org.elasticsearch.xpack.esql.CsvIT
method: test {csv-spec:spatial_shapes.convertFromStringParseError}
issue: https://github.com/elastic/elasticsearch/issues/145655
Expand Down
10 changes: 10 additions & 0 deletions ...n/java/org/elasticsearch/repositories/blobstore/AbstractBlobContainerRetriesTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ public void testReadBlobWithReadTimeouts() {
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(maxRetries)
.bufferSize(bufferSize)
.readTimeout(readTimeout)
.requestTimeout(TimeValue.timeValueSeconds(1))
.build();

// HTTP server does not send a response
Expand Down Expand Up @@ -585,6 +586,7 @@ private void ensureOpen() throws IOException {
protected abstract BlobContainer createBlobContainer(
@Nullable Integer maxRetries,
@Nullable TimeValue readTimeout,
@Nullable TimeValue requestTimeout,
@Nullable Boolean disableChunkedEncoding,
@Nullable Integer maxConnections,
@Nullable ByteSizeValue bufferSize,
Expand All @@ -598,6 +600,8 @@ protected final class TestBlobContainerBuilder {
@Nullable
private TimeValue readTimeout;
@Nullable
private TimeValue requestTimeout;
@Nullable
private Boolean disableChunkedEncoding;
@Nullable
private Integer maxConnections;
Expand All @@ -618,6 +622,11 @@ public TestBlobContainerBuilder readTimeout(@Nullable TimeValue readTimeout) {
return this;
}

public TestBlobContainerBuilder requestTimeout(@Nullable TimeValue requestTimeout) {
this.requestTimeout = requestTimeout;
return this;
}

public TestBlobContainerBuilder disableChunkedEncoding(@Nullable Boolean disableChunkedEncoding) {
this.disableChunkedEncoding = disableChunkedEncoding;
return this;
Expand Down Expand Up @@ -647,6 +656,7 @@ public BlobContainer build() {
return createBlobContainer(
maxRetries,
readTimeout,
requestTimeout,
disableChunkedEncoding,
maxConnections,
bufferSize,
Expand Down
Loading