Skip to content

Commit

Permalink
Add retries for building S3 client (#16438)
Browse files Browse the repository at this point in the history
* Add retries for building S3 client

* Use S3Utils instead of RetryUtils

* Add test
  • Loading branch information
Akshat-Jain committed May 13, 2024
1 parent 4bfc186 commit d1100a6
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ public boolean apply(Throwable e)
} else if (e instanceof SdkClientException && e.getMessage().contains("Unable to execute HTTP request")) {
// This is likely due to a temporary DNS issue and can be retried.
return true;
} else if (e instanceof SdkClientException && e.getMessage().contains("Unable to find a region via the region provider chain")) {
// This can happen sometimes when AWS isn't able to obtain the credentials for some service:
// https://github.com/aws/aws-sdk-java/issues/2285
return true;
} else if (e instanceof AmazonClientException) {
return AWSClientUtil.isClientExceptionRecoverable((AmazonClientException) e);
} else {
Expand All @@ -101,17 +105,18 @@ public boolean apply(Throwable e)
};

/**
* Retries S3 operations that fail due to io-related exceptions. Service-level exceptions (access denied, file not
* found, etc) are not retried.
* Retries S3 operations that fail intermittently (due to io-related exceptions, during obtaining credentials, etc).
* Service-level exceptions (access denied, file not found, etc) are not retried.
*/
public static <T> T retryS3Operation(Task<T> f) throws Exception
{
return RetryUtils.retry(f, S3RETRY, RetryUtils.DEFAULT_MAX_TRIES);
}

/**
* Retries S3 operations that fail due to io-related exceptions. Service-level exceptions (access denied, file not
* found, etc) are not retried. Also provide a way to set maxRetries that can be useful, i.e. for testing.
* Retries S3 operations that fail intermittently (due to io-related exceptions, during obtaining credentials, etc).
* Service-level exceptions (access denied, file not found, etc) are not retried.
* Also provide a way to set maxRetries that can be useful, i.e. for testing.
*/
public static <T> T retryS3Operation(Task<T> f, int maxRetries) throws Exception
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,15 @@ public ServerSideEncryptingAmazonS3 build()
throw new ISE("S3StorageConfig cannot be null!");
}

return new ServerSideEncryptingAmazonS3(amazonS3ClientBuilder.build(), s3StorageConfig.getServerSideEncryption());
AmazonS3 amazonS3Client;
try {
amazonS3Client = S3Utils.retryS3Operation(() -> amazonS3ClientBuilder.build());
}
catch (Exception e) {
throw new RuntimeException(e);
}

return new ServerSideEncryptingAmazonS3(amazonS3Client, s3StorageConfig.getServerSideEncryption());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.druid.storage.s3;

import com.amazonaws.SdkClientException;
import com.amazonaws.services.s3.model.AmazonS3Exception;
import org.junit.Assert;
import org.junit.Test;
Expand Down Expand Up @@ -108,4 +109,25 @@ public void testRetryWith5XXErrorsExceedingMaxRetries()
);
Assert.assertEquals(maxRetries, count.get());
}

@Test
public void testRetryWithSdkClientException() throws Exception
{
final int maxRetries = 3;
final AtomicInteger count = new AtomicInteger();
S3Utils.retryS3Operation(
() -> {
if (count.incrementAndGet() >= maxRetries) {
return "hey";
} else {
throw new SdkClientException(
"Unable to find a region via the region provider chain. "
+ "Must provide an explicit region in the builder or setup environment to supply a region."
);
}
},
maxRetries
);
Assert.assertEquals(maxRetries, count.get());
}
}

0 comments on commit d1100a6

Please sign in to comment.