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 42cd1d67b6f Tolerate concurrent deletes when sizing consumer and
segment directories (#19580)
42cd1d67b6f is described below
commit 42cd1d67b6f081bbdfa75963cf9e50204da2037e
Author: Xiaotian (Jackie) Jiang <[email protected]>
AuthorDate: Wed Sep 16 16:13:23 2026 -0700
Tolerate concurrent deletes when sizing consumer and segment directories
(#19580)
---
.../org/apache/pinot/common/utils/FileUtils.java | 22 ++++++++++++++++++++++
.../segment/store/SegmentLocalFSDirectory.java | 8 ++------
.../server/starter/helix/BaseServerStarter.java | 3 ++-
3 files changed, 26 insertions(+), 7 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/FileUtils.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/FileUtils.java
index d053d6375ec..2a7873d9e1f 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/utils/FileUtils.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/utils/FileUtils.java
@@ -133,4 +133,26 @@ public class FileUtils {
}
}
}
+
+ /// Returns the total size in bytes of the regular files under `dir`, or `0`
if `dir` does not exist. Symbolic links
+ /// are not followed.
+ ///
+ /// Use this for a directory that another thread may be writing to while it
is measured. It walks with the `File`
+ /// API, whose `listFiles()` and `length()` report a concurrently deleted
entry as `null` / `0` instead of throwing,
+ /// so a delete racing with the walk cannot fail the whole computation. The
trade-off is that an unreadable directory
+ /// is also counted as `0`. For a directory nothing else is writing to,
prefer
+ /// `org.apache.commons.io.FileUtils#sizeOfDirectory` so that a genuine I/O
error surfaces.
+ public static long sizeOfDirectory(File dir) {
+ long size = 0;
+ File[] files = dir.listFiles();
+ if (files != null) {
+ for (File file : files) {
+ if (Files.isSymbolicLink(file.toPath())) {
+ continue;
+ }
+ size += file.isDirectory() ? sizeOfDirectory(file) : file.length();
+ }
+ }
+ return size;
+ }
}
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SegmentLocalFSDirectory.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SegmentLocalFSDirectory.java
index b454765d634..2f0338ed36a 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SegmentLocalFSDirectory.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SegmentLocalFSDirectory.java
@@ -182,12 +182,8 @@ public class SegmentLocalFSDirectory extends
SegmentDirectory {
// check that v3 subdirectory exists since the format may not have been
converted
if (_segmentDirectory != null && _segmentDirectory.exists()) {
- try {
- return FileUtils.sizeOfDirectory(_segmentDirectory.toPath().toFile());
- } catch (IllegalArgumentException e) {
- LOGGER.error("Failed to read disk size for directory: {}",
_segmentDirectory.getAbsolutePath());
- return -1;
- }
+ // A concurrent reload may be rewriting this directory in place, so use
the delete-tolerant walk.
+ return
org.apache.pinot.common.utils.FileUtils.sizeOfDirectory(_segmentDirectory);
} else {
if (!SegmentDirectoryPaths.isV3Directory(_segmentDirectory)) {
LOGGER
diff --git
a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java
b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java
index 24cd13f3baa..d43a54b0c64 100644
---
a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java
+++
b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java
@@ -39,7 +39,6 @@ import java.util.function.Supplier;
import javax.annotation.Nullable;
import javax.net.ssl.SSLContext;
import nl.altindag.ssl.SSLFactory;
-import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.helix.HelixAdmin;
import org.apache.helix.HelixDataAccessor;
@@ -68,6 +67,7 @@ import org.apache.pinot.common.metrics.ServerMeter;
import org.apache.pinot.common.metrics.ServerMetrics;
import org.apache.pinot.common.metrics.ServerTimer;
import org.apache.pinot.common.restlet.resources.SystemResourceInfo;
+import org.apache.pinot.common.utils.FileUtils;
import org.apache.pinot.common.utils.PinotAppConfigs;
import org.apache.pinot.common.utils.ServiceStartableUtils;
import org.apache.pinot.common.utils.ServiceStatus;
@@ -940,6 +940,7 @@ public abstract class BaseServerStarter implements
ServiceStartable {
try {
for (File consumerDir : instanceConsumerDirs) {
if (consumerDir.exists()) {
+ // Consuming segments write to this directory continuously, so use
the delete-tolerant walk.
totalSize += FileUtils.sizeOfDirectory(consumerDir);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
