[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1350: [format] Supports estimation of data size in memory for better control of file size.

["JingsongLi (via GitHub)" <gi...@apache.org>]

JingsongLi commented on code in PR #1350:
URL: https://github.com/apache/incubator-paimon/pull/1350#discussion_r1226107330


##########
paimon-format/src/main/java/org/apache/paimon/format/orc/writer/OrcBulkWriter.java:
##########
@@ -67,4 +89,43 @@ public void finish() throws IOException {
         flush();
         writer.close();
     }
+
+    @Override
+    public long length() throws IOException {
+        long estimateMemory = treeWriter.estimateMemory();
+
+        long dataLength = 0;
+        List<StripeInformation> stripes =
+                Collections.unmodifiableList(OrcUtils.convertProtoStripesToStripes(this.stripes));
+        if (!stripes.isEmpty()) {
+            StripeInformation stripeInformation = stripes.get(stripes.size() - 1);
+            dataLength =
+                    stripeInformation != null
+                            ? stripeInformation.getOffset() + stripeInformation.getLength()
+                            : 0;
+        }
+
+        // This value is estimated, not actual.
+        return (long)
+                Math.ceil(
+                        dataLength
+                                + (estimateMemory + (long) rowBatch.size * avgRowByteSize) * 0.2);

Review Comment:
   By default, rowBatch is 1000, and the rolling is required every 1000 records, my question is: can we check rolling during flushBatch?
   
   So the `avgRowByteSize` can be removed.
   
   



##########
paimon-format/src/main/java/org/apache/paimon/format/orc/writer/OrcBulkWriter.java:
##########
@@ -67,4 +89,43 @@ public void finish() throws IOException {
         flush();
         writer.close();
     }
+
+    @Override
+    public long length() throws IOException {
+        long estimateMemory = treeWriter.estimateMemory();
+
+        long dataLength = 0;

Review Comment:
   Do we really need to get this from stripes? Can we just get pos from output stream?
   All stripes should have be flushed? right?



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

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