Skip to content

Commit d8fc877

Browse files
authored
Fix S3RepositoryThirdPartyTests.testReadFromPositionLargerThanBlobLength (#106466)
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. Closes #106457
1 parent 14ca58c commit d8fc877

File tree

2 files changed

+47
-10
lines changed

2 files changed

+47
-10
lines changed

modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3RepositoryThirdPartyTests.java

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
*/
88
package org.elasticsearch.repositories.s3;
99

10-
import com.amazonaws.AmazonClientException;
1110
import com.amazonaws.services.s3.model.AmazonS3Exception;
1211
import com.amazonaws.services.s3.model.GetObjectRequest;
1312
import com.amazonaws.services.s3.model.InitiateMultipartUploadRequest;
@@ -26,6 +25,7 @@
2625
import org.elasticsearch.common.settings.SecureSettings;
2726
import org.elasticsearch.common.settings.Settings;
2827
import org.elasticsearch.common.util.BigArrays;
28+
import org.elasticsearch.common.util.concurrent.UncategorizedExecutionException;
2929
import org.elasticsearch.core.Booleans;
3030
import org.elasticsearch.core.TimeValue;
3131
import org.elasticsearch.indices.recovery.RecoverySettings;
@@ -44,12 +44,14 @@
4444
import java.io.IOException;
4545
import java.util.Collection;
4646
import java.util.List;
47+
import java.util.concurrent.ExecutionException;
4748
import java.util.concurrent.TimeUnit;
4849
import java.util.concurrent.atomic.AtomicLong;
4950

5051
import static org.elasticsearch.repositories.blobstore.BlobStoreTestUtil.randomPurpose;
5152
import static org.hamcrest.Matchers.allOf;
5253
import static org.hamcrest.Matchers.blankOrNullString;
54+
import static org.hamcrest.Matchers.containsString;
5355
import static org.hamcrest.Matchers.equalTo;
5456
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
5557
import static org.hamcrest.Matchers.hasSize;
@@ -226,7 +228,6 @@ List<MultipartUpload> listMultipartUploads() {
226228
}
227229
}
228230

229-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/106457")
230231
public void testReadFromPositionLargerThanBlobLength() {
231232
final var blobName = randomIdentifier();
232233
final var blobBytes = randomBytesReference(randomIntBetween(100, 2_000));
@@ -239,9 +240,26 @@ public void testReadFromPositionLargerThanBlobLength() {
239240

240241
long position = randomLongBetween(blobBytes.length(), Long.MAX_VALUE - 1L);
241242
long length = randomLongBetween(1L, Long.MAX_VALUE - position);
242-
var exception = expectThrows(AmazonClientException.class, () -> readBlob(repository, blobName, position, length));
243243

244-
assertThat(exception, instanceOf(AmazonS3Exception.class));
245-
assertThat(((AmazonS3Exception) exception).getStatusCode(), equalTo(RestStatus.REQUESTED_RANGE_NOT_SATISFIED.getStatus()));
244+
var exception = expectThrows(UncategorizedExecutionException.class, () -> readBlob(repository, blobName, position, length));
245+
assertThat(exception.getCause(), instanceOf(ExecutionException.class));
246+
assertThat(exception.getCause().getCause(), instanceOf(IOException.class));
247+
assertThat(
248+
exception.getCause().getCause().getMessage(),
249+
containsString(
250+
"Requested range [start="
251+
+ position
252+
+ ", end="
253+
+ (position + length - 1L)
254+
+ ", currentOffset=0] cannot be satisfied for blob object ["
255+
+ repository.basePath().buildAsString()
256+
+ blobName
257+
+ ']'
258+
)
259+
);
260+
assertThat(
261+
asInstanceOf(AmazonS3Exception.class, exception.getRootCause()).getStatusCode(),
262+
equalTo(RestStatus.REQUESTED_RANGE_NOT_SATISFIED.getStatus())
263+
);
246264
}
247265
}

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.elasticsearch.common.blobstore.OperationPurpose;
2222
import org.elasticsearch.core.IOUtils;
2323
import org.elasticsearch.repositories.s3.S3BlobStore.Operation;
24+
import org.elasticsearch.rest.RestStatus;
2425

2526
import java.io.IOException;
2627
import java.io.InputStream;
@@ -94,16 +95,34 @@ private void openStreamWithRetry() throws IOException {
9495
: "requesting beyond end, start = " + start + " offset=" + currentOffset + " end=" + end;
9596
getObjectRequest.setRange(Math.addExact(start, currentOffset), end);
9697
}
97-
final S3Object s3Object = SocketAccess.doPrivileged(() -> clientReference.client().getObject(getObjectRequest));
9898
this.currentStreamFirstOffset = Math.addExact(start, currentOffset);
99+
final S3Object s3Object = SocketAccess.doPrivileged(() -> clientReference.client().getObject(getObjectRequest));
99100
this.currentStreamLastOffset = Math.addExact(currentStreamFirstOffset, getStreamLength(s3Object));
100101
this.currentStream = s3Object.getObjectContent();
101102
return;
102103
} catch (AmazonClientException e) {
103-
if (e instanceof AmazonS3Exception amazonS3Exception && 404 == amazonS3Exception.getStatusCode()) {
104-
throw addSuppressedExceptions(
105-
new NoSuchFileException("Blob object [" + blobKey + "] not found: " + amazonS3Exception.getMessage())
106-
);
104+
if (e instanceof AmazonS3Exception amazonS3Exception) {
105+
if (amazonS3Exception.getStatusCode() == RestStatus.NOT_FOUND.getStatus()) {
106+
throw addSuppressedExceptions(
107+
new NoSuchFileException("Blob object [" + blobKey + "] not found: " + amazonS3Exception.getMessage())
108+
);
109+
}
110+
if (amazonS3Exception.getStatusCode() == RestStatus.REQUESTED_RANGE_NOT_SATISFIED.getStatus()) {
111+
throw addSuppressedExceptions(
112+
new IOException(
113+
"Requested range [start="
114+
+ start
115+
+ ", end="
116+
+ end
117+
+ ", currentOffset="
118+
+ currentOffset
119+
+ "] cannot be satisfied for blob object ["
120+
+ blobKey
121+
+ ']',
122+
amazonS3Exception
123+
)
124+
);
125+
}
107126
}
108127

109128
if (attempt == 1) {

0 commit comments

Comments
 (0)