[GitHub] [iceberg] ConeyLiu commented on a diff in pull request #6335: Core: Avoid generating a large ManifestFile when committing

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

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-unsubscribe@iceberg.apache.org

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