[GitHub] [iceberg] rdblue commented on a diff in pull request #6353: Make sure S3 stream opened by ReadConf ctor is closed

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

rdblue commented on code in PR #6353:
URL: https://github.com/apache/iceberg/pull/6353#discussion_r1051688387


##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetReader.java:
##########
@@ -79,9 +83,11 @@ private ReadConf<T> init() {
               nameMapping,
               reuseContainers,
               caseSensitive,
-              null);
-      this.conf = readConf.copy();
-      return readConf;
+              null)) {
+        this.conf = readConf.copy();

Review Comment:
   The reason why `ReadConf` is not closed here was because we wanted to reuse the reader rather than opening a new one. This change will cause normal file reads to add an additional GET request round trip to open the file again.
   
   What is the case where this wasn't being closed? I think that the originally logic should be safe because the `init` method is only called if a `FileIterator` is created, and that calls `ReadConf.reader()`. So either a new reader is opened, or the existing reader is reused and will be closed by the iterator.



-- 
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