stayrascal commented on code in PR #23725:
URL: https://github.com/apache/flink/pull/23725#discussion_r1724293957
##########
flink-filesystems/flink-s3-fs-base/src/main/java/org/apache/flink/fs/s3/common/writer/S3RecoverableFsDataOutputStream.java:
##########
@@ -131,8 +131,7 @@ public void sync() throws IOException {
fileStream.flush();
openNewPartIfNecessary(userDefinedMinPartSize);
Committer committer = upload.snapshotAndGetCommitter();
- committer.commitAfterRecovery();
- closeForCommit();
+ committer.commit();
Review Comment:
Regarding the semantic of FSDataOutputStream which doesn't guarantee that
the `write` method won't been invoked after invoking `sync` method, if writing
any new data after invoking `sync` and invoke `closeForCommit`, it will upload
part failed since the mpu is completed. And also we cannot create a new
fileStream and mpu because it might overwrite the existing file and cause data
lost.
I'm thinking if we can route to `persist` method here, or implement a
`StreamCapabilities` interface like hdfs StreamCapabilities, the downstream
decide whether invoke `persist` or `sync` according the sync capability of
output stream.
--
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]
