Fokko commented on code in PR #3571:
URL: https://github.com/apache/parquet-java/pull/3571#discussion_r3291323322
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ColumnChunkPageWriteStore.java:
##########
@@ -692,6 +692,11 @@ public void flushToFileWriter(ParquetFileWriter writer)
throws IOException {
for (ColumnDescriptor path : schema.getColumns()) {
ColumnChunkPageWriter pageWriter = writers.get(path);
Review Comment:
Should we use the try-with-resource pattern here? (Fanboy of the pattern
talking here)
##########
parquet-common/src/main/java/org/apache/parquet/bytes/ConcatenatingByteBufferCollector.java:
##########
@@ -78,6 +83,30 @@ public void writeAllTo(OutputStream out) throws IOException {
}
}
+ /**
+ * Writes all collected slabs to the given output stream, releasing each
slab's
+ * {@link ByteBuffer} back to the allocator immediately after it has been
written.
+ * This progressively frees memory during the write rather than holding all
slabs
+ * until {@link #close()} is called.
+ *
+ * <p>After this method returns, the collector is empty and {@link #size()}
returns 0.
+ * Calling {@link #close()} afterwards is safe but has no additional effect.
+ *
+ * @param out the output stream to write to
+ * @throws IOException if an I/O error occurs
+ */
+ public void writeAllToAndRelease(OutputStream out) throws IOException {
Review Comment:
Wait, are we just using this in tests?
##########
parquet-common/src/main/java/org/apache/parquet/bytes/ConcatenatingByteBufferCollector.java:
##########
@@ -78,6 +83,30 @@ public void writeAllTo(OutputStream out) throws IOException {
}
}
+ /**
+ * Writes all collected slabs to the given output stream, releasing each
slab's
+ * {@link ByteBuffer} back to the allocator immediately after it has been
written.
+ * This progressively frees memory during the write rather than holding all
slabs
+ * until {@link #close()} is called.
+ *
+ * <p>After this method returns, the collector is empty and {@link #size()}
returns 0.
+ * Calling {@link #close()} afterwards is safe but has no additional effect.
+ *
+ * @param out the output stream to write to
+ * @throws IOException if an I/O error occurs
+ */
+ public void writeAllToAndRelease(OutputStream out) throws IOException {
+ WritableByteChannel channel = Channels.newChannel(out);
Review Comment:
Should we wrap a try-with-resource around this channel?
--
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]
