This is an automated email from the ASF dual-hosted git repository.
peter-toth pushed a commit to branch branch-3.5
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-3.5 by this push:
new 094d1b18ebfb Validate executor local directories and block ids in the
external shuffle service follow-up
094d1b18ebfb is described below
commit 094d1b18ebfb56e7c83e22786d075d33ad43d6ab
Author: Peter Toth <[email protected]>
AuthorDate: Thu Jul 2 10:42:57 2026 +0200
Validate executor local directories and block ids in the external shuffle
service follow-up
---
.../shuffle/ExternalShuffleBlockResolver.java | 2 +-
.../spark/network/shuffle/LocalDirValidator.java | 10 ++++-----
.../shuffle/ExternalShuffleBlockResolverSuite.java | 26 ++++++++++++----------
.../network/shuffle/LocalDirValidatorSuite.java | 14 ++++++------
.../spark/deploy/ExternalShuffleService.scala | 3 +--
5 files changed, 27 insertions(+), 28 deletions(-)
diff --git
a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalShuffleBlockResolver.java
b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalShuffleBlockResolver.java
index 4ae6add2cd89..18e32e19dee8 100644
---
a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalShuffleBlockResolver.java
+++
b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalShuffleBlockResolver.java
@@ -367,7 +367,7 @@ public class ExternalShuffleBlockResolver {
validateBlockId(blockId);
} catch (IllegalArgumentException e) {
logger.warn("Skipping block with invalid id: {}",
- MDC.of(LogKeys.BLOCK_ID, blockId));
+ blockId);
continue;
}
File file = new File(
diff --git
a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/LocalDirValidator.java
b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/LocalDirValidator.java
index b7e08386a222..9750c224f2e8 100644
---
a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/LocalDirValidator.java
+++
b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/LocalDirValidator.java
@@ -23,10 +23,8 @@ import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
-import org.apache.spark.internal.SparkLogger;
-import org.apache.spark.internal.SparkLoggerFactory;
-import org.apache.spark.internal.LogKeys;
-import org.apache.spark.internal.MDC;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Validates the local directories an executor reports at registration, before
the external shuffle
@@ -42,7 +40,7 @@ import org.apache.spark.internal.MDC;
* contain the entries against and they are accepted. An empty array is always
allowed.
*/
class LocalDirValidator {
- private static final SparkLogger logger =
SparkLoggerFactory.getLogger(LocalDirValidator.class);
+ private static final Logger logger =
LoggerFactory.getLogger(LocalDirValidator.class);
private final List<Path> allowedLocalDirRoots;
private final boolean requireAppScopedLocalDirs;
@@ -106,7 +104,7 @@ class LocalDirValidator {
roots.add(new File(root).getCanonicalFile().toPath());
} catch (IOException e) {
logger.warn("Ignoring local dir root that could not be
canonicalized: {}",
- MDC.of(LogKeys.PATH, root));
+ root);
}
}
}
diff --git
a/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleBlockResolverSuite.java
b/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleBlockResolverSuite.java
index 42afcdc13757..0c49e8b1eb66 100644
---
a/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleBlockResolverSuite.java
+++
b/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleBlockResolverSuite.java
@@ -144,8 +144,8 @@ public class ExternalShuffleBlockResolverSuite {
File localDir = new File(context.localDirs[0]);
File parentOfLocalDir = localDir.getParentFile();
outsideFile = new File(parentOfLocalDir, "outside-" + UUID.randomUUID()
+ ".txt");
- Files.writeString(outsideFile.toPath(), "should not be deleted",
StandardCharsets.UTF_8);
- assertTrue(outsideFile.exists(), "precondition: file was created");
+ Files.write(outsideFile.toPath(), "should not be
deleted".getBytes(StandardCharsets.UTF_8));
+ assertTrue("precondition: file was created", outsideFile.exists());
ExternalShuffleBlockResolver resolver = new
ExternalShuffleBlockResolver(conf, null);
resolver.registerExecutor("app0", "0",
context.createExecutorInfo(SORT_MANAGER));
@@ -153,10 +153,11 @@ public class ExternalShuffleBlockResolverSuite {
String blockId = ".." + File.separator + ".." + File.separator +
outsideFile.getName();
int removed = resolver.removeBlocks("app0", "0", new String[] { blockId
});
- assertEquals(0, removed, "A block id with parent-dir segments must not
remove a block");
- assertTrue(outsideFile.exists(),
+ assertEquals("A block id with parent-dir segments must not remove a
block", 0, removed);
+ assertTrue(
"A file outside the local directory must not be removed (" +
- outsideFile.getAbsolutePath() + ")");
+ outsideFile.getAbsolutePath() + ")",
+ outsideFile.exists());
} finally {
if (outsideFile != null && outsideFile.exists()) {
assertTrue(outsideFile.delete() || !outsideFile.exists());
@@ -180,7 +181,7 @@ public class ExternalShuffleBlockResolverSuite {
String blockId = "sub" + File.separator + "child";
int removed = resolver.removeBlocks("app0", "0", new String[] { blockId
});
- assertEquals(0, removed, "A block id containing a path separator must be
skipped");
+ assertEquals("A block id containing a path separator must be skipped",
0, removed);
} finally {
context.cleanup();
}
@@ -199,7 +200,7 @@ public class ExternalShuffleBlockResolverSuite {
File localDir = new File(context.localDirs[0]);
File parentOfLocalDir = localDir.getParentFile();
outsideFile = new File(parentOfLocalDir, "outside-" + UUID.randomUUID()
+ ".txt");
- Files.writeString(outsideFile.toPath(), "should not be deleted",
StandardCharsets.UTF_8);
+ Files.write(outsideFile.toPath(), "should not be
deleted".getBytes(StandardCharsets.UTF_8));
// Plant a real, legitimately-named block in the localDir so the valid
removal has something
// to actually remove.
@@ -207,7 +208,7 @@ public class ExternalShuffleBlockResolverSuite {
String validBlockId = "rdd_7_0";
File validFile = new File(ExecutorDiskUtils.getFilePath(
context.localDirs, context.subDirsPerLocalDir, validBlockId));
- assertTrue(validFile.exists(), "precondition: block file was created");
+ assertTrue("precondition: block file was created", validFile.exists());
ExternalShuffleBlockResolver resolver = new
ExternalShuffleBlockResolver(conf, null);
resolver.registerExecutor("app0", "0",
context.createExecutorInfo(SORT_MANAGER));
@@ -217,10 +218,11 @@ public class ExternalShuffleBlockResolverSuite {
int removed = resolver.removeBlocks(
"app0", "0", new String[] { invalidBlockId, validBlockId });
- assertEquals(1, removed, "The valid block id in the batch should still
be removed");
- assertTrue(outsideFile.exists(),
- "The invalid entry must not remove a file outside the local
directory");
- assertFalse(validFile.exists(), "The valid block file should have been
removed by the batch");
+ assertEquals("The valid block id in the batch should still be removed",
1, removed);
+ assertTrue(
+ "The invalid entry must not remove a file outside the local directory",
+ outsideFile.exists());
+ assertFalse("The valid block file should have been removed by the
batch", validFile.exists());
} finally {
if (outsideFile != null && outsideFile.exists()) {
assertTrue(outsideFile.delete() || !outsideFile.exists());
diff --git
a/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/LocalDirValidatorSuite.java
b/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/LocalDirValidatorSuite.java
index cf678bb0d633..006c0572744f 100644
---
a/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/LocalDirValidatorSuite.java
+++
b/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/LocalDirValidatorSuite.java
@@ -21,9 +21,9 @@ import java.io.File;
import java.nio.file.Files;
import org.apache.spark.network.util.JavaUtils;
-import org.junit.jupiter.api.Test;
+import org.junit.Test;
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.Assert.*;
public class LocalDirValidatorSuite {
@@ -68,11 +68,11 @@ public class LocalDirValidatorSuite {
File root = Files.createTempDirectory("ess-nm-local").toFile();
try {
File otherAppDir = new File(root,
"usercache/u/appcache/app1/blockmgr-x");
- assertTrue(otherAppDir.mkdirs(), "precondition: directory created");
+ assertTrue("precondition: directory created", otherAppDir.mkdirs());
LocalDirValidator validator = new LocalDirValidator(new String[] {
root.getPath() }, true);
- assertThrows(IllegalArgumentException.class,
- () -> validator.validate(new String[] { otherAppDir.getPath() },
"app0"),
- "validate must reject a localDir scoped to a different application");
+ assertThrows("validate must reject a localDir scoped to a different
application",
+ IllegalArgumentException.class,
+ () -> validator.validate(new String[] { otherAppDir.getPath() },
"app0"));
} finally {
JavaUtils.deleteRecursively(root);
}
@@ -83,7 +83,7 @@ public class LocalDirValidatorSuite {
File root = Files.createTempDirectory("ess-nm-local").toFile();
try {
File ownDir = new File(root, "usercache/u/appcache/app0/blockmgr-x");
- assertTrue(ownDir.mkdirs(), "precondition: directory created");
+ assertTrue("precondition: directory created", ownDir.mkdirs());
LocalDirValidator validator = new LocalDirValidator(new String[] {
root.getPath() }, true);
// Must not throw.
validator.validate(new String[] { ownDir.getPath() }, "app0");
diff --git
a/core/src/main/scala/org/apache/spark/deploy/ExternalShuffleService.scala
b/core/src/main/scala/org/apache/spark/deploy/ExternalShuffleService.scala
index 3e6e40d5d3f4..b237a0e2696e 100644
--- a/core/src/main/scala/org/apache/spark/deploy/ExternalShuffleService.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/ExternalShuffleService.scala
@@ -87,8 +87,7 @@ class ExternalShuffleService(sparkConf: SparkConf,
securityManager: SecurityMana
logInfo(s"Use ${dbBackend.name()} as the implementation of " +
s"${config.SHUFFLE_SERVICE_DB_BACKEND.key}")
new ExternalBlockHandler(conf,
-
findRegisteredExecutorsDBFile(dbBackend.fileName(registeredExecutorsDB)))
- localDirs, false)
+
findRegisteredExecutorsDBFile(dbBackend.fileName(registeredExecutorsDB)),
localDirs, false)
} else {
new ExternalBlockHandler(conf, null, localDirs, false)
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
