[GitHub] [iceberg] danielcweeks commented on pull request #2159: AWS: Fail writes fast when S3OutputStream encountered error in async …

[GitBox <gi...@apache.org>]

danielcweeks commented on pull request #2159:
URL: https://github.com/apache/iceberg/pull/2159#issuecomment-772734858


   @stevenzwu Could you clarify as part of this why we need to fail fast?  Is this actually causing real-world problems or just an optimization for cases where a failure occurs during write?
   
   I have concerns about introducing an atomic reference check into a critical section (like `write(int)` though that method really shouldn't be used anyway) and unless there's significant problems with catching up in rare cases where a failure occurs, I'd rather not introduce the extra check (this was intentionally avoided in the initial implementation).
   
   Also, for the multipart upload error, I feel like we we should not be catching and logging, but rather let the underlying exception propagate up.  We don't make guarantees around cleaning up multipart uploads (that should be handled by bucket policy because there are numerous failure scenarios we cannot cover).
   
   I'm probably missing context around what prompted this change.  


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org