From e8ea16a5699843c266eb117f9d3861d0091a2723 Mon Sep 17 00:00:00 2001 From: chengpan Date: Mon, 29 Jun 2020 03:31:10 +0800 Subject: [PATCH 01/10] [SPARK-32121][SHUFFLE][TEST] Fix ExternalShuffleBlockResolverSuite failed on Windows --- .../spark/network/shuffle/ExecutorDiskUtils.java | 7 ++++--- .../shuffle/ExternalShuffleBlockResolverSuite.java | 14 +++++++------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java index 13f6046dd856..2851d395a321 100644 --- a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java +++ b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java @@ -27,7 +27,7 @@ public class ExecutorDiskUtils { - private static final Pattern MULTIPLE_SEPARATORS = Pattern.compile(File.separator + "{2,}"); + private static final Pattern MULTIPLE_SEPARATORS = Pattern.compile("[/\\\\]+"); /** * Hashes a filename into the corresponding local directory, in a manner consistent with @@ -50,14 +50,15 @@ public static File getFile(String[] localDirs, int subDirsPerLocalDir, String fi * the internal code in java.io.File would normalize it later, creating a new "foo/bar" * String copy. Unfortunately, we cannot just reuse the normalization code that java.io.File * uses, since it is in the package-private class java.io.FileSystem. + * On Windows, separator / should be \. */ @VisibleForTesting static String createNormalizedInternedPathname(String dir1, String dir2, String fname) { String pathname = dir1 + File.separator + dir2 + File.separator + fname; Matcher m = MULTIPLE_SEPARATORS.matcher(pathname); - pathname = m.replaceAll("/"); + pathname = m.replaceAll(Matcher.quoteReplacement(File.separator)); // A single trailing slash needs to be taken care of separately - if (pathname.length() > 1 && pathname.endsWith("/")) { + if (pathname.length() > 1 && pathname.endsWith(File.separator)) { pathname = pathname.substring(0, pathname.length() - 1); } return pathname.intern(); 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 09b31430b1eb..fb3f08d978e6 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 @@ -146,12 +146,12 @@ public void jsonSerializationOfExecutorRegistration() throws IOException { @Test public void testNormalizeAndInternPathname() { - assertPathsMatch("/foo", "bar", "baz", "/foo/bar/baz"); - assertPathsMatch("//foo/", "bar/", "//baz", "/foo/bar/baz"); - assertPathsMatch("foo", "bar", "baz///", "foo/bar/baz"); - assertPathsMatch("/foo/", "/bar//", "/baz", "/foo/bar/baz"); - assertPathsMatch("/", "", "", "/"); - assertPathsMatch("/", "/", "/", "/"); + assertPathsMatch("/foo", "bar", "baz", File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); + assertPathsMatch("//foo/", "bar/", "//baz", File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); + assertPathsMatch("foo", "bar", "baz///", "foo" + File.separator + "bar" + File.separator + "baz"); + assertPathsMatch("/foo/", "/bar//", "/baz", File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); + assertPathsMatch("/", "", "", File.separator); + assertPathsMatch("/", "/", "/", File.separator); } private void assertPathsMatch(String p1, String p2, String p3, String expectedPathname) { @@ -160,6 +160,6 @@ private void assertPathsMatch(String p1, String p2, String p3, String expectedPa assertEquals(expectedPathname, normPathname); File file = new File(normPathname); String returnedPath = file.getPath(); - assertTrue(normPathname == returnedPath); + assertEquals(normPathname, returnedPath); } } From 187e8133522ee15a608a6473aa089086e224a88f Mon Sep 17 00:00:00 2001 From: chengpan Date: Mon, 29 Jun 2020 04:11:40 +0800 Subject: [PATCH 02/10] [FOLLOWUP][SPARK-32121][SHUFFLE][TEST] fix checkstyle --- .../ExternalShuffleBlockResolverSuite.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) 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 fb3f08d978e6..a6c3e466a016 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 @@ -146,12 +146,18 @@ public void jsonSerializationOfExecutorRegistration() throws IOException { @Test public void testNormalizeAndInternPathname() { - assertPathsMatch("/foo", "bar", "baz", File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); - assertPathsMatch("//foo/", "bar/", "//baz", File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); - assertPathsMatch("foo", "bar", "baz///", "foo" + File.separator + "bar" + File.separator + "baz"); - assertPathsMatch("/foo/", "/bar//", "/baz", File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); - assertPathsMatch("/", "", "", File.separator); - assertPathsMatch("/", "/", "/", File.separator); + assertPathsMatch("/foo", "bar", "baz", + File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); + assertPathsMatch("//foo/", "bar/", "//baz", + File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); + assertPathsMatch("foo", "bar", "baz///", + "foo" + File.separator + "bar" + File.separator + "baz"); + assertPathsMatch("/foo/", "/bar//", "/baz", + File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); + assertPathsMatch("/", "", "", + File.separator); + assertPathsMatch("/", "/", "/", + File.separator); } private void assertPathsMatch(String p1, String p2, String p3, String expectedPathname) { From 9b24a147ec90a33983b957cc2bea651140adcf02 Mon Sep 17 00:00:00 2001 From: pancheng <379377944@qq.com> Date: Mon, 29 Jun 2020 11:54:10 +0800 Subject: [PATCH 03/10] [SPARK-32121][SHUFFLE][TEST] use different MULTIPLE_SEPARATORS in Windows and Unix-like OS --- .../network/shuffle/ExecutorDiskUtils.java | 20 +++++++++++++++++-- .../ExternalShuffleBlockResolverSuite.java | 7 +++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java index 2851d395a321..3e78eb8ef2e2 100644 --- a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java +++ b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java @@ -27,7 +27,13 @@ public class ExecutorDiskUtils { - private static final Pattern MULTIPLE_SEPARATORS = Pattern.compile("[/\\\\]+"); + private static final Pattern MULTIPLE_SEPARATORS; + static { + if (isWindows()) + MULTIPLE_SEPARATORS= Pattern.compile("[/\\\\]+"); + else + MULTIPLE_SEPARATORS = Pattern.compile("/{2,}"); + } /** * Hashes a filename into the corresponding local directory, in a manner consistent with @@ -41,6 +47,13 @@ public static File getFile(String[] localDirs, int subDirsPerLocalDir, String fi localDir, String.format("%02x", subDirId), filename)); } + /** Returns whether the OS is Windows. */ + @VisibleForTesting + static boolean isWindows() { + String os = System.getProperty("os.name"); + return os.startsWith("Windows"); + } + /** * This method is needed to avoid the situation when multiple File instances for the * same pathname "foo/bar" are created, each with a separate copy of the "foo/bar" String. @@ -50,7 +63,10 @@ public static File getFile(String[] localDirs, int subDirsPerLocalDir, String fi * the internal code in java.io.File would normalize it later, creating a new "foo/bar" * String copy. Unfortunately, we cannot just reuse the normalization code that java.io.File * uses, since it is in the package-private class java.io.FileSystem. - * On Windows, separator / should be \. + * + * On Windows, separator "/" should be "\". + * + * "\\" is legal character in path name on Unix like OS, but illegal on Windows. */ @VisibleForTesting static String createNormalizedInternedPathname(String dir1, String dir2, String fname) { 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 a6c3e466a016..17d8d6b22480 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 @@ -158,6 +158,13 @@ public void testNormalizeAndInternPathname() { File.separator); assertPathsMatch("/", "/", "/", File.separator); + if (ExecutorDiskUtils.isWindows()) { + assertPathsMatch("/foo\\/", "bar", "baz", + File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); + } else { + assertPathsMatch("/foo\\/", "bar", "baz", + File.separator + "foo\\" + File.separator + "bar" + File.separator + "baz"); + } } private void assertPathsMatch(String p1, String p2, String p3, String expectedPathname) { From 8cd8bd1708fd492db7583bfe3f0fcfe05fb8bb75 Mon Sep 17 00:00:00 2001 From: pancheng <379377944@qq.com> Date: Mon, 29 Jun 2020 12:41:25 +0800 Subject: [PATCH 04/10] [FOLLOWUP][SPARK-32121][SHUFFLE][TEST] fix checkstyle --- .../apache/spark/network/shuffle/ExecutorDiskUtils.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java index 3e78eb8ef2e2..7f9b14a2a441 100644 --- a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java +++ b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java @@ -29,10 +29,11 @@ public class ExecutorDiskUtils { private static final Pattern MULTIPLE_SEPARATORS; static { - if (isWindows()) - MULTIPLE_SEPARATORS= Pattern.compile("[/\\\\]+"); - else + if (isWindows()) { + MULTIPLE_SEPARATORS = Pattern.compile("[/\\\\]+"); + } else { MULTIPLE_SEPARATORS = Pattern.compile("/{2,}"); + } } /** From 06024f4018e77549d6147d63c5fc75b276cc33e8 Mon Sep 17 00:00:00 2001 From: pancheng <379377944@qq.com> Date: Mon, 29 Jun 2020 20:09:25 +0800 Subject: [PATCH 05/10] [FOLLOWUP][SPARK-32121][SHUFFLE][TEST] fix checkstyle --- .../ExternalShuffleBlockResolverSuite.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) 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 17d8d6b22480..c5a44a808907 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 @@ -147,23 +147,21 @@ public void jsonSerializationOfExecutorRegistration() throws IOException { @Test public void testNormalizeAndInternPathname() { assertPathsMatch("/foo", "bar", "baz", - File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); + File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); assertPathsMatch("//foo/", "bar/", "//baz", - File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); + File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); assertPathsMatch("foo", "bar", "baz///", - "foo" + File.separator + "bar" + File.separator + "baz"); + "foo" + File.separator + "bar" + File.separator + "baz"); assertPathsMatch("/foo/", "/bar//", "/baz", - File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); - assertPathsMatch("/", "", "", - File.separator); - assertPathsMatch("/", "/", "/", - File.separator); + File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); + assertPathsMatch("/", "", "", File.separator); + assertPathsMatch("/", "/", "/", File.separator); if (ExecutorDiskUtils.isWindows()) { assertPathsMatch("/foo\\/", "bar", "baz", - File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); + File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); } else { assertPathsMatch("/foo\\/", "bar", "baz", - File.separator + "foo\\" + File.separator + "bar" + File.separator + "baz"); + File.separator + "foo\\" + File.separator + "bar" + File.separator + "baz"); } } From 6a201a578f1321a094a590f72027b838dee0a8fc Mon Sep 17 00:00:00 2001 From: pancheng <379377944@qq.com> Date: Mon, 29 Jun 2020 20:58:24 +0800 Subject: [PATCH 06/10] [FOLLOWUP][SPARK-32121][SHUFFLE][TEST] simplify code by commons-lang3 --- .../spark/network/shuffle/ExecutorDiskUtils.java | 10 ++-------- .../shuffle/ExternalShuffleBlockResolverSuite.java | 3 ++- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java index 7f9b14a2a441..0a3d7d1207c5 100644 --- a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java +++ b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java @@ -23,13 +23,14 @@ import com.google.common.annotations.VisibleForTesting; +import org.apache.commons.lang3.SystemUtils; import org.apache.spark.network.util.JavaUtils; public class ExecutorDiskUtils { private static final Pattern MULTIPLE_SEPARATORS; static { - if (isWindows()) { + if (SystemUtils.IS_OS_WINDOWS) { MULTIPLE_SEPARATORS = Pattern.compile("[/\\\\]+"); } else { MULTIPLE_SEPARATORS = Pattern.compile("/{2,}"); @@ -48,13 +49,6 @@ public static File getFile(String[] localDirs, int subDirsPerLocalDir, String fi localDir, String.format("%02x", subDirId), filename)); } - /** Returns whether the OS is Windows. */ - @VisibleForTesting - static boolean isWindows() { - String os = System.getProperty("os.name"); - return os.startsWith("Windows"); - } - /** * This method is needed to avoid the situation when multiple File instances for the * same pathname "foo/bar" are created, each with a separate copy of the "foo/bar" String. 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 c5a44a808907..e62637e72347 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 @@ -25,6 +25,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.io.CharStreams; +import org.apache.commons.lang3.SystemUtils; import org.apache.spark.network.shuffle.protocol.ExecutorShuffleInfo; import org.apache.spark.network.util.MapConfigProvider; import org.apache.spark.network.util.TransportConf; @@ -156,7 +157,7 @@ public void testNormalizeAndInternPathname() { File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); assertPathsMatch("/", "", "", File.separator); assertPathsMatch("/", "/", "/", File.separator); - if (ExecutorDiskUtils.isWindows()) { + if (SystemUtils.IS_OS_WINDOWS) { assertPathsMatch("/foo\\/", "bar", "baz", File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); } else { From 50a742da1c48bcfb6f69b319c9a0142801c959c6 Mon Sep 17 00:00:00 2001 From: pancheng <379377944@qq.com> Date: Mon, 29 Jun 2020 21:01:24 +0800 Subject: [PATCH 07/10] [FOLLOWUP][SPARK-32121][SHUFFLE][TEST] improve comments --- .../org/apache/spark/network/shuffle/ExecutorDiskUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java index 0a3d7d1207c5..a1e494517127 100644 --- a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java +++ b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java @@ -59,7 +59,7 @@ public static File getFile(String[] localDirs, int subDirsPerLocalDir, String fi * String copy. Unfortunately, we cannot just reuse the normalization code that java.io.File * uses, since it is in the package-private class java.io.FileSystem. * - * On Windows, separator "/" should be "\". + * On Windows, separator "\" is used instead of "/". * * "\\" is legal character in path name on Unix like OS, but illegal on Windows. */ From 56f3be5775ec6c5801a249d9d5929f14b4e22a96 Mon Sep 17 00:00:00 2001 From: pancheng <379377944@qq.com> Date: Tue, 30 Jun 2020 21:08:51 +0800 Subject: [PATCH 08/10] [FOLLOWUP][SPARK-32121][SHUFFLE][TEST] minor refactor --- .../network/shuffle/ExecutorDiskUtils.java | 2 +- .../ExternalShuffleBlockResolverSuite.java | 26 +++++++++---------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java index a1e494517127..c546a5437bf1 100644 --- a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java +++ b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java @@ -69,7 +69,7 @@ static String createNormalizedInternedPathname(String dir1, String dir2, String Matcher m = MULTIPLE_SEPARATORS.matcher(pathname); pathname = m.replaceAll(Matcher.quoteReplacement(File.separator)); // A single trailing slash needs to be taken care of separately - if (pathname.length() > 1 && pathname.endsWith(File.separator)) { + if (pathname.length() > 1 && pathname.charAt(pathname.length() - 1) == File.separatorChar) { pathname = pathname.substring(0, pathname.length() - 1); } return pathname.intern(); 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 e62637e72347..2a0b9c24be9a 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 @@ -147,22 +147,20 @@ public void jsonSerializationOfExecutorRegistration() throws IOException { @Test public void testNormalizeAndInternPathname() { - assertPathsMatch("/foo", "bar", "baz", - File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); - assertPathsMatch("//foo/", "bar/", "//baz", - File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); - assertPathsMatch("foo", "bar", "baz///", - "foo" + File.separator + "bar" + File.separator + "baz"); - assertPathsMatch("/foo/", "/bar//", "/baz", - File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); - assertPathsMatch("/", "", "", File.separator); - assertPathsMatch("/", "/", "/", File.separator); + String sep = File.separator; + String expectedPathname1 = sep + "foo" + sep + "bar" + sep + "baz"; + String expectedPathname2 = "foo" + sep + "bar" + sep + "baz"; + String expectedPathname3 = sep + "foo\\" + sep + "bar" + sep + "baz"; + assertPathsMatch("/foo", "bar", "baz", expectedPathname1); + assertPathsMatch("//foo/", "bar/", "//baz", expectedPathname1); + assertPathsMatch("foo", "bar", "baz///", expectedPathname2); + assertPathsMatch("/foo/", "/bar//", "/baz", expectedPathname1); + assertPathsMatch("/", "", "", sep); + assertPathsMatch("/", "/", "/", sep); if (SystemUtils.IS_OS_WINDOWS) { - assertPathsMatch("/foo\\/", "bar", "baz", - File.separator + "foo" + File.separator + "bar" + File.separator + "baz"); + assertPathsMatch("/foo\\/", "bar", "baz", expectedPathname1); } else { - assertPathsMatch("/foo\\/", "bar", "baz", - File.separator + "foo\\" + File.separator + "bar" + File.separator + "baz"); + assertPathsMatch("/foo\\/", "bar", "baz", expectedPathname3); } } From aac01ca11c3c024e8a75753e43a217cadb1d8c46 Mon Sep 17 00:00:00 2001 From: chengpan Date: Tue, 30 Jun 2020 22:26:32 +0800 Subject: [PATCH 09/10] [FOLLOWUP][SPARK-32121][SHUFFLE][TEST] code style --- .../ExternalShuffleBlockResolverSuite.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) 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 2a0b9c24be9a..6515b6ca035f 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 @@ -148,19 +148,17 @@ public void jsonSerializationOfExecutorRegistration() throws IOException { @Test public void testNormalizeAndInternPathname() { String sep = File.separator; - String expectedPathname1 = sep + "foo" + sep + "bar" + sep + "baz"; - String expectedPathname2 = "foo" + sep + "bar" + sep + "baz"; - String expectedPathname3 = sep + "foo\\" + sep + "bar" + sep + "baz"; - assertPathsMatch("/foo", "bar", "baz", expectedPathname1); - assertPathsMatch("//foo/", "bar/", "//baz", expectedPathname1); - assertPathsMatch("foo", "bar", "baz///", expectedPathname2); - assertPathsMatch("/foo/", "/bar//", "/baz", expectedPathname1); + String expectedPathname = sep + "foo" + sep + "bar" + sep + "baz"; + assertPathsMatch("/foo", "bar", "baz", expectedPathname); + assertPathsMatch("//foo/", "bar/", "//baz", expectedPathname); + assertPathsMatch("/foo/", "/bar//", "/baz", expectedPathname); + assertPathsMatch("foo", "bar", "baz///", "foo" + sep + "bar" + sep + "baz"); assertPathsMatch("/", "", "", sep); assertPathsMatch("/", "/", "/", sep); if (SystemUtils.IS_OS_WINDOWS) { - assertPathsMatch("/foo\\/", "bar", "baz", expectedPathname1); + assertPathsMatch("/foo\\/", "bar", "baz", expectedPathname); } else { - assertPathsMatch("/foo\\/", "bar", "baz", expectedPathname3); + assertPathsMatch("/foo\\/", "bar", "baz", sep + "foo\\" + sep + "bar" + sep + "baz"); } } From a80bd6c8a5d93187cf06f941c2a9d296a7b6ca61 Mon Sep 17 00:00:00 2001 From: pancheng <379377944@qq.com> Date: Wed, 1 Jul 2020 12:46:32 +0800 Subject: [PATCH 10/10] [SPARK-32121][SHUFFLE] improve comments --- .../org/apache/spark/network/shuffle/ExecutorDiskUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java index c546a5437bf1..6549cac011fe 100644 --- a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java +++ b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExecutorDiskUtils.java @@ -61,7 +61,7 @@ public static File getFile(String[] localDirs, int subDirsPerLocalDir, String fi * * On Windows, separator "\" is used instead of "/". * - * "\\" is legal character in path name on Unix like OS, but illegal on Windows. + * "\\" is a legal character in path name on Unix-like OS, but illegal on Windows. */ @VisibleForTesting static String createNormalizedInternedPathname(String dir1, String dir2, String fname) {