From 1340e2b40fe51473c62276277c76e370217c706e Mon Sep 17 00:00:00 2001 From: Akanksha Kedia Date: Mon, 11 May 2026 11:05:12 +0530 Subject: [PATCH 1/2] Fix null-safety and contract violations in HadoopPinotFS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - visitFileStatus(): guard against null return value from FileSystem.listStatus() before iterating, avoiding NPE on empty or inaccessible directories - isDirectory() / lastModified(): remove RuntimeException wrapping of IOException — both methods are declared in the PinotFS interface to throw IOException, so the checked exception should propagate directly to callers rather than being hidden in a RuntimeException - touch(): use try-with-resources for FSDataOutputStream so the stream is closed even if fos.close() throws, preventing a resource leak - close(): null-check _hadoopFS before calling close() to guard against the case where init() was never called or threw --- .../plugin/filesystem/HadoopPinotFS.java | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/pinot-plugins/pinot-file-system/pinot-hdfs/src/main/java/org/apache/pinot/plugin/filesystem/HadoopPinotFS.java b/pinot-plugins/pinot-file-system/pinot-hdfs/src/main/java/org/apache/pinot/plugin/filesystem/HadoopPinotFS.java index 37ff660e0c00..9384b3ca0ec9 100644 --- a/pinot-plugins/pinot-file-system/pinot-hdfs/src/main/java/org/apache/pinot/plugin/filesystem/HadoopPinotFS.java +++ b/pinot-plugins/pinot-file-system/pinot-hdfs/src/main/java/org/apache/pinot/plugin/filesystem/HadoopPinotFS.java @@ -183,6 +183,9 @@ private void visitFileStatus(Path path, boolean recursive, Consumer throws IOException { // _hadoopFS.listFiles(path, false) will not return directories as files, thus use listStatus(path) here. FileStatus[] files = _hadoopFS.listStatus(path); + if (files == null) { + return; + } for (FileStatus file : files) { visitor.accept(file); if (file.isDirectory() && recursive) { @@ -234,23 +237,15 @@ public void copyFromLocalDir(File srcFile, URI dstUri) } @Override - public boolean isDirectory(URI uri) { - try { - return _hadoopFS.getFileStatus(new Path(uri)).isDirectory(); - } catch (IOException e) { - LOGGER.error("Could not get file status for {}", uri, e); - throw new RuntimeException(e); - } + public boolean isDirectory(URI uri) + throws IOException { + return _hadoopFS.getFileStatus(new Path(uri)).isDirectory(); } @Override - public long lastModified(URI uri) { - try { - return _hadoopFS.getFileStatus(new Path(uri)).getModificationTime(); - } catch (IOException e) { - LOGGER.error("Could not get file status for {}", uri, e); - throw new RuntimeException(e); - } + public long lastModified(URI uri) + throws IOException { + return _hadoopFS.getFileStatus(new Path(uri)).getModificationTime(); } @Override @@ -258,8 +253,9 @@ public boolean touch(URI uri) throws IOException { Path path = new Path(uri); if (!exists(uri)) { - FSDataOutputStream fos = _hadoopFS.create(path); - fos.close(); + try (FSDataOutputStream fos = _hadoopFS.create(path)) { + // create an empty file; stream closed by try-with-resources + } } else { _hadoopFS.setTimes(path, System.currentTimeMillis(), -1); } @@ -307,7 +303,9 @@ private org.apache.hadoop.conf.Configuration getConf(String hadoopConfPath) { @Override public void close() throws IOException { - _hadoopFS.close(); + if (_hadoopFS != null) { + _hadoopFS.close(); + } super.close(); } } From c62dcb40039c76ef35abe25c6d57dfb38a4821d6 Mon Sep 17 00:00:00 2001 From: Akanksha Kedia Date: Mon, 11 May 2026 19:03:57 +0530 Subject: [PATCH 2/2] Fix visitFileStatus to throw IOException instead of silently returning on null listStatus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Silently returning when listStatus() returns null causes delete(uri, false) to see an empty directory listing, skip the non-empty guard, and fall through to _hadoopFS.delete(path, true) — recursively deleting a non-empty directory that should have been preserved. Throw IOException instead so callers fail fast. --- .../java/org/apache/pinot/plugin/filesystem/HadoopPinotFS.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pinot-plugins/pinot-file-system/pinot-hdfs/src/main/java/org/apache/pinot/plugin/filesystem/HadoopPinotFS.java b/pinot-plugins/pinot-file-system/pinot-hdfs/src/main/java/org/apache/pinot/plugin/filesystem/HadoopPinotFS.java index 9384b3ca0ec9..be2aa28ec618 100644 --- a/pinot-plugins/pinot-file-system/pinot-hdfs/src/main/java/org/apache/pinot/plugin/filesystem/HadoopPinotFS.java +++ b/pinot-plugins/pinot-file-system/pinot-hdfs/src/main/java/org/apache/pinot/plugin/filesystem/HadoopPinotFS.java @@ -184,7 +184,7 @@ private void visitFileStatus(Path path, boolean recursive, Consumer // _hadoopFS.listFiles(path, false) will not return directories as files, thus use listStatus(path) here. FileStatus[] files = _hadoopFS.listStatus(path); if (files == null) { - return; + throw new IOException("FileSystem.listStatus() returned null for path: " + path); } for (FileStatus file : files) { visitor.accept(file);