hudi-agent commented on code in PR #19935:
URL: https://github.com/apache/hudi/pull/19935#discussion_r3999277020


##########
hudi-hadoop-common/src/main/java/org/apache/hudi/hadoop/fs/HadoopFSUtils.java:
##########
@@ -231,8 +231,21 @@ public static FSDataInputStream 
getFSDataInputStream(FileSystem fs,
     }
 
     if (fsDataInputStream.getWrappedStream() instanceof FSInputStream) {
+      // The buffered stream wraps the inner FSInputStream, not the 
FSDataInputStream the filesystem
+      // returned. Filesystems that track the streams they hand out (Spark's 
DebugFilesystem in tests)
+      // see a leak unless that outer object is closed too; the inner stream's 
close is idempotent.
+      final FSDataInputStream original = fsDataInputStream;
       return new TimedFSDataInputStream(convertToHadoopPath(filePath), new 
FSDataInputStream(
-          new BufferedFSInputStream((FSInputStream) 
fsDataInputStream.getWrappedStream(), bufferSize)));
+          new BufferedFSInputStream((FSInputStream) 
original.getWrappedStream(), bufferSize)) {
+        @Override
+        public void close() throws IOException {
+          try {
+            super.close();
+          } finally {
+            original.close();

Review Comment:
   🤖 The GCS branch in `getFSDataInputStreamForGCS` (a few lines below) has the 
same shape — it pulls the inner `FSInputStream` out of two levels of 
`FSDataInputStream` and drops both outer objects. Not reachable by 
`DebugFilesystem`, but would it make sense to apply the same close-through 
there for consistency, or is that intentionally out of scope?
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



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

Reply via email to