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 elastic#106457
tlrx · Mar 19, 2024 · e6ace8a · e6ace8a
1 parent 3af0e93
commit e6ace8a
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 4 deletions.
diff --git a/...ternalClusterTest/java/org/elasticsearch/repositories/s3/S3RepositoryThirdPartyTests.java b/...ternalClusterTest/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<MultipartUpload> 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/.../repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java b/.../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/...rk/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java b/...rk/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> 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
@@ -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);
+    }
 }