GH-1067: Address HDFS cleanup review feedback

Author: xborder

dataset/pom.xml

@@ -53,6 +53,10 @@ under the License.
       <artifactId>arrow-c-data</artifactId>
       <scope>compile</scope>
     </dependency>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-api</artifactId>
+    </dependency>
     <dependency>
       <groupId>org.immutables</groupId>
       <artifactId>value-annotations</artifactId>

dataset/src/main/java/module-info.java

@@ -26,4 +26,5 @@
   requires org.apache.arrow.c;
   requires org.apache.arrow.memory.core;
   requires org.apache.arrow.vector;
+  requires org.slf4j;
 }

dataset/src/main/java/org/apache/arrow/dataset/file/FileSystemDatasetFactory.java

@@ -18,17 +18,22 @@
 
 import java.lang.reflect.Method;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.LinkedHashSet;
 import java.util.Optional;
 import java.util.Set;
 import org.apache.arrow.dataset.jni.NativeDatasetFactory;
 import org.apache.arrow.dataset.jni.NativeMemoryPool;
 import org.apache.arrow.dataset.scanner.FragmentScanOptions;
 import org.apache.arrow.memory.BufferAllocator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /** Java binding of the C++ FileSystemDatasetFactory. */
 public class FileSystemDatasetFactory extends NativeDatasetFactory {
 
+  private static final Logger LOGGER = LoggerFactory.getLogger(FileSystemDatasetFactory.class);
+
   private final Set<URI> hdfsFileSystems;
 
   public FileSystemDatasetFactory(
@@ -78,11 +83,10 @@ public synchronized void close() {
   }
 
   /**
-   * For each {@code hdfs://} URI, close the cached Hadoop FileSystem.
-   * When Arrow C++ accesses HDFS via libhdfs, the Hadoop Java client creates cached FileSystem
-   * instances with non-daemon threads (IPC connections, lease renewers) that prevent JVM exit.
-   * Closing the FileSystem terminates these connections. Uses reflection to avoid a compile-time
-   * dependency on hadoop-common.
+   * For each {@code hdfs://} URI, close the cached Hadoop FileSystem. When Arrow C++ accesses HDFS
+   * via libhdfs, the Hadoop Java client creates cached FileSystem instances with non-daemon threads
+   * (IPC connections, lease renewers) that prevent JVM exit. Closing the FileSystem terminates
+   * these connections. Uses reflection to avoid a compile-time dependency on hadoop-common.
    */
   static void closeHadoopFileSystemsIfHdfs(String... uris) {
     toHdfsFileSystems(uris).forEach(FileSystemDatasetFactory::closeHadoopFileSystem);
@@ -94,13 +98,16 @@ private static Set<URI> toHdfsFileSystems(String... uris) {
       return hdfsFileSystems;
     }
     for (String uri : uris) {
+      if (uri == null) {
+        continue;
+      }
       try {
         URI parsedUri = new URI(uri);
         if ("hdfs".equalsIgnoreCase(parsedUri.getScheme())) {
           hdfsFileSystems.add(
               new URI(parsedUri.getScheme(), parsedUri.getAuthority(), null, null, null));
         }
-      } catch (Exception e) {
+      } catch (URISyntaxException e) {
         // Ignore here; native factory creation reports invalid user URIs.
       }
     }
@@ -117,7 +124,8 @@ private static void closeHadoopFileSystem(URI hdfsUri) {
       Method closeMethod = fsClass.getMethod("close");
       closeMethod.invoke(fs);
     } catch (Exception e) {
-      // Best-effort cleanup; Hadoop may not be on classpath or FileSystem already closed
+      // Best-effort cleanup; Hadoop may not be on classpath or FileSystem already closed.
+      LOGGER.debug("Failed to close Hadoop FileSystem for {}", hdfsUri, e);
     }
   }
 

dataset/src/test/java/org/apache/arrow/dataset/file/TestHdfsFileSystemCleanup.java

@@ -65,8 +65,7 @@ void testJvmHangsWithoutCleanup() throws Exception {
   @Test
   void testJvmExitsWithCleanup() throws Exception {
     Process child = forkChildProcess(true);
-    assertTrue(
-        waitForExit(child), "JVM should exit when FileSystemDatasetFactory cleanup runs");
+    assertTrue(waitForExit(child), "JVM should exit when FileSystemDatasetFactory cleanup runs");
     assertEquals(0, child.exitValue(), "Child process should exit cleanly (exit code 0)");
   }