danielcweeks commented on code in PR #10433:
URL: https://github.com/apache/iceberg/pull/10433#discussion_r1624953864
##########
aws/src/main/java/org/apache/iceberg/aws/s3/S3InputStream.java:
##########
@@ -139,7 +140,11 @@ private InputStream readRange(String range) {
S3RequestUtil.configureEncryption(s3FileIOProperties, requestBuilder);
- return s3.getObject(requestBuilder.build(),
ResponseTransformer.toInputStream());
+ stream =
+ RetryableInputStream.builderFor(
+ () -> s3.getObject(requestBuilder.build(),
ResponseTransformer.toInputStream()))
Review Comment:
I feel like there's an issue here. When the stream gets recreated, the
stream will reset back to the original position and we continue from there as
if we're at the right place in the stream.
The `pos` won't reflect the new position of the stream, if I'm reading this
correctly. I would think the following retry would need to start from `next`
to reflect where the next read should start. There's a small problem with the
single byte read method because we increment the positions prior to read, so
that would likely need to be adjusted to be after the read like in the range
read method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
