This is an automated email from the ASF dual-hosted git repository.
Jackie-Jiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new c27e3a77701 Close Files.walk stream in LocalLogFileServer to prevent
file descriptor leak (#19242)
c27e3a77701 is described below
commit c27e3a777019b885e61ded96fc52d33c43574542
Author: Deepak kumar <[email protected]>
AuthorDate: Sat Aug 15 12:22:30 2026 -0700
Close Files.walk stream in LocalLogFileServer to prevent file descriptor
leak (#19242)
---
.../pinot/common/utils/log/LocalLogFileServer.java | 9 ++++--
.../common/utils/log/LocalLogFileServerTest.java | 33 ++++++++++++++++++++++
2 files changed, 40 insertions(+), 2 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/log/LocalLogFileServer.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/log/LocalLogFileServer.java
index 5874ff6589c..701b67620b9 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/utils/log/LocalLogFileServer.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/utils/log/LocalLogFileServer.java
@@ -26,6 +26,7 @@ import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Set;
import java.util.TreeSet;
+import java.util.stream.Stream;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response;
@@ -47,8 +48,12 @@ public class LocalLogFileServer implements LogFileServer {
public Set<String> getAllLogFilePaths()
throws IOException {
Set<String> allFiles = new TreeSet<>();
- Files.walk(_logRootDirPath).filter(Files::isRegularFile).forEach(
- f ->
allFiles.add(f.toAbsolutePath().toString().replace(_logRootDirPath.toAbsolutePath()
+ "/", "")));
+ // Files.walk holds one or more DirectoryStreams; close it eagerly so this
method does not leak
+ // file descriptors when invoked repeatedly (e.g. once per downloadLogFile
call).
+ try (Stream<Path> paths = Files.walk(_logRootDirPath)) {
+ paths.filter(Files::isRegularFile).forEach(
+ f ->
allFiles.add(f.toAbsolutePath().toString().replace(_logRootDirPath.toAbsolutePath()
+ "/", "")));
+ }
return allFiles;
}
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/utils/log/LocalLogFileServerTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/utils/log/LocalLogFileServerTest.java
index 323379e57e3..19e9678509b 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/utils/log/LocalLogFileServerTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/utils/log/LocalLogFileServerTest.java
@@ -21,6 +21,7 @@ package org.apache.pinot.common.utils.log;
import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
+import java.util.Set;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.Response;
import org.apache.commons.io.FileUtils;
@@ -29,6 +30,7 @@ import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
public class LocalLogFileServerTest {
@@ -76,4 +78,35 @@ public class LocalLogFileServerTest {
FileUtils.deleteQuietly(logRootDir);
}
}
+
+ /// Verifies that {@link LocalLogFileServer#getAllLogFilePaths()} enumerates
files inside nested
+ /// subdirectories and returns paths that are relative to the log root. This
is a regression test
+ /// for the {@code Files.walk} refactor that wraps the stream in a
try-with-resources block; the
+ /// recursion behavior must be preserved so that downloads under nested
directories continue to
+ /// work.
+ @Test
+ public void testGetAllLogFilePathsEnumeratesNestedDirectories()
+ throws IOException {
+ File logRootDir = new File(FileUtils.getTempDirectory(),
+ "testGetAllLogFilePathsEnumeratesNestedDirectories-" +
System.currentTimeMillis());
+ try {
+ assertTrue(logRootDir.mkdirs());
+ File nested = new File(logRootDir, "sub/dir");
+ assertTrue(nested.mkdirs());
+ FileUtils.writeStringToFile(new File(logRootDir, "top.log"), "top",
Charset.defaultCharset());
+ FileUtils.writeStringToFile(new File(nested, "nested.log"), "nested",
Charset.defaultCharset());
+
+ LogFileServer logFileServer = new
LocalLogFileServer(logRootDir.getAbsolutePath());
+ Set<String> paths = logFileServer.getAllLogFilePaths();
+
+ assertEquals(paths.size(), 2, "expected two enumerated files, got: " +
paths);
+ assertTrue(paths.contains("top.log"), "missing top.log in " + paths);
+ assertTrue(paths.contains("sub/dir/nested.log"), "missing
sub/dir/nested.log in " + paths);
+ // Both files must be downloadable via the relative paths returned above.
+ assertNotNull(logFileServer.downloadLogFile("top.log"));
+ assertNotNull(logFileServer.downloadLogFile("sub/dir/nested.log"));
+ } finally {
+ FileUtils.deleteQuietly(logRootDir);
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
