ConeyLiu commented on code in PR #6335:
URL: https://github.com/apache/iceberg/pull/6335#discussion_r1039686191
##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -499,6 +501,40 @@ protected long snapshotId() {
return snapshotId;
}
+ protected static <F extends ContentFile<F>> List<ManifestFile>
writeFilesToManifests(
+ Iterable<F> files,
+ Supplier<ManifestWriter<F>> createWriter,
+ Long newFilesSequenceNumber,
+ long targetSizeBytes)
+ throws IOException {
+ List<ManifestFile> result = Lists.newArrayList();
+ Iterator<F> fileIterator = files.iterator();
+ ManifestWriter<F> writer = null;
Review Comment:
Updated with try-finally, while try-with-resources seems not easy to
implement.
> Could we initialize the writer upfront?
There are some UTs failed if do that. I tried the following:
```java
try {
writer = createWriter.get();
while (fileIterator.hasNext()) {
if (writer.length() >= targetSizeBytes) {
// here could produce an empty file because the intialize size of
the newly writer could be larger than targetSizeBytes
writer.close();
result.add(writer.toManifestFile());
writer = createWriter.get();
}
F file = fileIterator.next();
if (newFilesSequenceNumber == null) {
writer.add(file);
} else {
writer.add(file, newFilesSequenceNumber);
}
}
} finally {
if (writer != null) {
writer.close();
}
}
```
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
