Tensorstore's S3 retry implementation does not conform to S3 specs, resulting in checkpointing failing when it should not.

[apghml]

Tensorstore's S3 implemenation appears to use the same isRetriable() implemenation as GCS. However, S3 and GCS have very different conditions for when a request should be retried. E.g., an S3 http 400 RequestTimeout error should be retried, but doesn't seem to currently be retried by tensorstore. However, not all 400 errors should be retried. It requires checking the response body for the error name.

This results in checkpointing sometimes failing when it should not.

The ideal solution to this would probably be to switch to using the AWS SDK like tf.io does. This would also have other benefits like supporting multipart uploads, which also can help with reliability, and which is currently not supported in tensorstore.

[laramiel]

See also:

#114
#149

I was looking at that pull request more, and it appears that we need the following libraries imported

"https://github.com/awslabs/aws-c-http",
"https://github.com/awslabs/aws-c-io",
"https://github.com/awslabs/aws-c-auth",
"https://github.com/awslabs/aws-c-mqtt",
"https://github.com/awslabs/aws-c-cal",
"https://github.com/awslabs/aws-c-compression",
"https://github.com/awslabs/aws-checksums",
"https://github.com/awslabs/aws-c-sdkutils",
"https://github.com/awslabs/aws-c-common",
"https://github.com/awslabs/aws-c-event-stream",
"https://github.com/awslabs/aws-c-s3",
"https://github.com/awslabs/aws-crt-cpp",
"https://github.com/aws/s2n-tls",

And then the C++ sdk has a bunch of sub-libraries.

"https://github.com/aws/aws-sdk-cpp",

"aws-cpp-sdk-core",
"aws-cpp-sdk-s3",

In the meantime, if you suggest which codes are retriable then we can change the retry implementation.

[apghml]

I'm not very knowledgable about AWS, but I took a look at the AWS SDK repo, and it looks like these two files have lists of retriable and non-retriable errors. Based on a blog post (https://guihao-liang.github.io/2020/04/12/aws-s3-retry#summary) it sounds like the first file takes precedence.

• https://github.com/aws/aws-sdk-cpp/blob/67f1aae9c821f8ed061a89423741a16a4723d018/generated/src/aws-cpp-sdk-s3/source/S3Errors.cpp#L26
• https://github.com/aws/aws-sdk-cpp/blob/master/aws-cpp-sdk-core/source/client/CoreErrors.cpp

[laramiel]

Sure. I'm poking at this. The documentation is here:

• https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingRESTError.html
• https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html

Much of the error logic appears to be here:

• https://github.com/aws/aws-sdk-cpp/blob/67f1aae9c821f8ed061a89423741a16a4723d018/src/aws-cpp-sdk-core/source/client/AWSErrorMarshaller.cpp#L22

And the retryable errors are mainly listed here:

• https://github.com/aws/aws-sdk-cpp/blob/67f1aae9c821f8ed061a89423741a16a4723d018/src/aws-cpp-sdk-core/source/client/CoreErrors.cpp#L36

[apghml]

Thanks a lot!