Add retries for building S3 client (#16438)

* Add retries for building S3 client

* Use S3Utils instead of RetryUtils

* Add test
This commit is contained in:
Akshat Jain 2024-05-14 05:02:06 +05:30 committed by GitHub
parent 4bfc186153
commit d1100a6f63
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 40 additions and 5 deletions

View File

@ -92,6 +92,10 @@ public class S3Utils
} 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 {
@ -101,8 +105,8 @@ public class S3Utils
};
/**
* 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
{
@ -110,8 +114,9 @@ public class S3Utils
}
/**
* 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
{

View File

@ -204,7 +204,15 @@ public class ServerSideEncryptingAmazonS3
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());
}
}
}

View File

@ -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;
@ -108,4 +109,25 @@ public class S3UtilsTest
);
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());
}
}