voonhous commented on code in PR #19935:
URL: https://github.com/apache/hudi/pull/19935#discussion_r4002090417


##########
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:
   Intentionally out of scope. The GCS branch drops plain `FSDataInputStream` 
wrappers; the connector's own FSInputStream, which owns the connection, is what 
gets closed, so nothing leaks there. The only thing that observes an unclosed 
outer wrapper is a stream-tracking filesystem like Spark's test 
DebugFilesystem, which only fronts file:. This PR fixes the branch that had an 
observed failure and a test; I'd leave GCS alone rather than change an I/O path 
no test exercises.



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