aboutsummaryrefslogtreecommitdiff
path: root/common/network-shuffle
diff options
context:
space:
mode:
authorTejas Patil <tejasp@fb.com>2016-05-18 12:10:32 +0100
committerSean Owen <sowen@cloudera.com>2016-05-18 12:10:32 +0100
commitc1fd9cacba8e602cc55a893f0beb05ea48c3a1f6 (patch)
treed455f0cb8841bf0560ae56d56ce05037b8a780da /common/network-shuffle
parent420b700695fe8bcdda406c34ad48230b9dfc07f1 (diff)
downloadspark-c1fd9cacba8e602cc55a893f0beb05ea48c3a1f6.tar.gz
spark-c1fd9cacba8e602cc55a893f0beb05ea48c3a1f6.tar.bz2
spark-c1fd9cacba8e602cc55a893f0beb05ea48c3a1f6.zip
[SPARK-15263][CORE] Make shuffle service dir cleanup faster by using `rm -rf`
## What changes were proposed in this pull request? Jira: https://issues.apache.org/jira/browse/SPARK-15263 The current logic for directory cleanup is slow because it does directory listing, recurses over child directories, checks for symbolic links, deletes leaf files and finally deletes the dirs when they are empty. There is back-and-forth switching from kernel space to user space while doing this. Since most of the deployment backends would be Unix systems, we could essentially just do `rm -rf` so that entire deletion logic runs in kernel space. The current Java based impl in Spark seems to be similar to what standard libraries like guava and commons IO do (eg. http://svn.apache.org/viewvc/commons/proper/io/trunk/src/main/java/org/apache/commons/io/FileUtils.java?view=markup#l1540). However, guava removed this method in favour of shelling out to an operating system command (like in this PR). See the `Deprecated` note in older javadocs for guava for details : http://google.github.io/guava/releases/10.0.1/api/docs/com/google/common/io/Files.html#deleteRecursively(java.io.File) Ideally, Java should be providing such APIs so that users won't have to do such things to get platform specific code. Also, its not just about speed, but also handling race conditions while doing at FS deletions is tricky. I could find this bug for Java in similar context : http://bugs.java.com/bugdatabase/view_bug.do?bug_id=7148952 ## How was this patch tested? I am relying on existing test cases to test the method. If there are suggestions about testing it, welcome to hear about it. ## Performance gains *Input setup* : Created a nested directory structure of depth 3 and each entry having 50 sub-dirs. The input being cleaned up had total ~125k dirs. Ran both approaches (in isolation) for 6 times to get average numbers: Native Java cleanup | `rm -rf` as a separate process ------------ | ------------- 10.04 sec | 4.11 sec This change made deletion 2.4 times faster for the given test input. Author: Tejas Patil <tejasp@fb.com> Closes #13042 from tejasapatil/delete_recursive.
Diffstat (limited to 'common/network-shuffle')
-rw-r--r--common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/TestShuffleDataContext.java24
1 files changed, 10 insertions, 14 deletions
diff --git a/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/TestShuffleDataContext.java b/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/TestShuffleDataContext.java
index 62a1fb42b0..81e01949e5 100644
--- a/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/TestShuffleDataContext.java
+++ b/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/TestShuffleDataContext.java
@@ -27,12 +27,17 @@ import com.google.common.io.Closeables;
import com.google.common.io.Files;
import org.apache.spark.network.shuffle.protocol.ExecutorShuffleInfo;
+import org.apache.spark.network.util.JavaUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Manages some sort-shuffle data, including the creation
* and cleanup of directories that can be read by the {@link ExternalShuffleBlockResolver}.
*/
public class TestShuffleDataContext {
+ private static final Logger logger = LoggerFactory.getLogger(TestShuffleDataContext.class);
+
public final String[] localDirs;
public final int subDirsPerLocalDir;
@@ -53,7 +58,11 @@ public class TestShuffleDataContext {
public void cleanup() {
for (String localDir : localDirs) {
- deleteRecursively(new File(localDir));
+ try {
+ JavaUtils.deleteRecursively(new File(localDir));
+ } catch (IOException e) {
+ logger.warn("Unable to cleanup localDir = " + localDir, e);
+ }
}
}
@@ -92,17 +101,4 @@ public class TestShuffleDataContext {
public ExecutorShuffleInfo createExecutorInfo(String shuffleManager) {
return new ExecutorShuffleInfo(localDirs, subDirsPerLocalDir, shuffleManager);
}
-
- private static void deleteRecursively(File f) {
- assert f != null;
- if (f.isDirectory()) {
- File[] children = f.listFiles();
- if (children != null) {
- for (File child : children) {
- deleteRecursively(child);
- }
- }
- }
- f.delete();
- }
}