From e6ace8a0c670be4b04421825465c2fa5c2abc1db Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 19 Mar 2024 11:21:05 +0100 Subject: [PATCH] Fix S3RepositoryThirdPartyTests.testReadFromPositionLargerThanBlobLength The test should use a random operation purpose that is not "Indices", otherwise S3RetryingInputStream retries up to Integer.MAX_VALUE times which causes the test suite to timeout. Also fixes the progress in the retries log messages. Relates #106457 --- .../s3/S3RepositoryThirdPartyTests.java | 8 ++++++-- .../repositories/s3/S3RetryingInputStream.java | 2 +- .../AbstractThirdPartyRepositoryTestCase.java | 14 +++++++++++++- .../repositories/blobstore/BlobStoreTestUtil.java | 4 ++++ 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3RepositoryThirdPartyTests.java b/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3RepositoryThirdPartyTests.java index 583d1477fbaa9..ce1d7073be938 100644 --- a/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3RepositoryThirdPartyTests.java +++ b/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3RepositoryThirdPartyTests.java @@ -47,6 +47,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; +import static org.elasticsearch.repositories.blobstore.BlobStoreTestUtil.randomNonIndicesPurpose; import static org.elasticsearch.repositories.blobstore.BlobStoreTestUtil.randomPurpose; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.blankOrNullString; @@ -226,7 +227,6 @@ List listMultipartUploads() { } } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/106457") public void testReadFromPositionLargerThanBlobLength() { final var blobName = randomIdentifier(); final var blobBytes = randomBytesReference(randomIntBetween(100, 2_000)); @@ -239,7 +239,11 @@ public void testReadFromPositionLargerThanBlobLength() { long position = randomLongBetween(blobBytes.length(), Long.MAX_VALUE - 1L); long length = randomLongBetween(1L, Long.MAX_VALUE - position); - var exception = expectThrows(AmazonClientException.class, () -> readBlob(repository, blobName, position, length)); + var exception = expectThrows( + AmazonClientException.class, + // non Indices operation purpose to avoid Integer.MAX_VALUE number of retries + () -> readBlob(repository, randomNonIndicesPurpose(), blobName, position, length) + ); assertThat(exception, instanceOf(AmazonS3Exception.class)); assertThat(((AmazonS3Exception) exception).getStatusCode(), equalTo(RestStatus.REQUESTED_RANGE_NOT_SATISFIED.getStatus())); diff --git a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java index 998455a658406..729f3c642bf90 100644 --- a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java +++ b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java @@ -94,8 +94,8 @@ private void openStreamWithRetry() throws IOException { : "requesting beyond end, start = " + start + " offset=" + currentOffset + " end=" + end; getObjectRequest.setRange(Math.addExact(start, currentOffset), end); } - final S3Object s3Object = SocketAccess.doPrivileged(() -> clientReference.client().getObject(getObjectRequest)); this.currentStreamFirstOffset = Math.addExact(start, currentOffset); + final S3Object s3Object = SocketAccess.doPrivileged(() -> clientReference.client().getObject(getObjectRequest)); this.currentStreamLastOffset = Math.addExact(currentStreamFirstOffset, getStreamLength(s3Object)); this.currentStream = s3Object.getObjectContent(); return; diff --git a/test/framework/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java b/test/framework/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java index 7cdeaeedfdeaf..a8c42c406ae31 100644 --- a/test/framework/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java @@ -15,6 +15,7 @@ import org.elasticsearch.common.blobstore.BlobContainer; import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.blobstore.BlobStore; +import org.elasticsearch.common.blobstore.OperationPurpose; import org.elasticsearch.common.blobstore.support.BlobMetadata; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; @@ -39,6 +40,7 @@ import java.util.concurrent.Executor; import static org.elasticsearch.repositories.blobstore.BlobStoreTestUtil.randomNonDataPurpose; +import static org.elasticsearch.repositories.blobstore.BlobStoreTestUtil.randomNonIndicesPurpose; import static org.elasticsearch.repositories.blobstore.BlobStoreTestUtil.randomPurpose; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.hamcrest.Matchers.contains; @@ -291,8 +293,18 @@ protected static T executeOnBlobStore(BlobStoreRepository repository, Checke } protected static BytesReference readBlob(BlobStoreRepository repository, String blobName, long position, long length) { + return readBlob(repository, randomPurpose(), blobName, position, length); + } + + protected static BytesReference readBlob( + BlobStoreRepository repository, + OperationPurpose purpose, + String blobName, + long position, + long length + ) { return executeOnBlobStore(repository, blobContainer -> { - try (var input = blobContainer.readBlob(randomPurpose(), blobName, position, length); var output = new BytesStreamOutput()) { + try (var input = blobContainer.readBlob(purpose, blobName, position, length); var output = new BytesStreamOutput()) { Streams.copy(input, output); return output.bytes(); } diff --git a/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreTestUtil.java b/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreTestUtil.java index d31bd16b07fcc..76be6a15309f7 100644 --- a/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreTestUtil.java +++ b/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreTestUtil.java @@ -458,4 +458,8 @@ public static OperationPurpose randomPurpose() { public static OperationPurpose randomNonDataPurpose() { return randomValueOtherThan(OperationPurpose.SNAPSHOT_DATA, BlobStoreTestUtil::randomPurpose); } + + public static OperationPurpose randomNonIndicesPurpose() { + return randomValueOtherThan(OperationPurpose.INDICES, BlobStoreTestUtil::randomPurpose); + } }