RussellSpitzer commented on code in PR #5311:
URL: https://github.com/apache/iceberg/pull/5311#discussion_r925962896
##########
aws/src/main/java/org/apache/iceberg/aws/s3/S3OutputStream.java:
##########
@@ -240,6 +242,23 @@ private void newStream() throws IOException {
@Override
public void close() throws IOException {
+
+ /**
+ * As calls to close() are used to mark a stream as completed and add the
file
+ * to the completed data files in BaseTaskWriter we cannot just return if
this
+ * was already closed with failure to upload.
+ *
+ * As close 'completes' the upload and staging files are cleanup, any
failure to
+ * upload should be persisted and thrown back to the caller to make sure
+ * the underlying file represented by this stream is not used in the
completedDataFiles
+ * of a manifest.
+ * see: https://github.com/apache/iceberg/issues/5310
+ */
+ if(closeFailureState != null){
+ throw new IllegalStateException("close called on a closed stream which
failed to close completely earlier at: "+closeFailureState.getFailureAt()
Review Comment:
Few style errors on this line, Checkstyle will mark them as errors though.
(Missing space between + and string, "comma after a newline"). Instructions on
running our current checkstyle rules are in the dev docs
--
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]
