From 3a8f4d9d615bc525b41ed13089dd806f56c0c57c Mon Sep 17 00:00:00 2001 From: Tamas Domok Date: Fri, 30 Jul 2021 12:50:33 +0200 Subject: [PATCH 1/4] YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies. Change-Id: If26d4aa33492903aaf24100fea4183a18e7d22ff --- .../launcher/ContainerLaunch.java | 41 +++++++------------ 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java index e864c14ad7092..0b11be36fe1f8 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java @@ -46,6 +46,8 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FSDataInputStream; @@ -1449,6 +1451,9 @@ public Set getEnvDependencies(final String envVal) { private static final class WindowsShellScriptBuilder extends ShellScriptBuilder { + private static final Pattern variablePattern = Pattern.compile("%(.*?)%"); + private static final Pattern splitPattern = Pattern.compile(":"); + private void errorCheck() { line("@if %errorlevel% neq 0 exit /b %errorlevel%"); } @@ -1539,34 +1544,18 @@ public Set getEnvDependencies(final String envVal) { if (envVal == null || envVal.isEmpty()) { return Collections.emptySet(); } + + // Example inputs: %var%, %%, %a:b% + Matcher matcher = variablePattern.matcher(envVal); final Set deps = new HashSet<>(); - final int len = envVal.length(); - int i = 0; - while (i < len) { - i = envVal.indexOf('%', i); // find beginning of variable - if (i < 0 || i == (len - 1)) { - break; - } - i++; - // 3 cases: %var%, %var:...% or %% - final int j = envVal.indexOf('%', i); // find end of variable - if (j == i) { - // %% case, just skip it - i++; - continue; - } - if (j < 0) { - break; // even %var:...% syntax ends with a %, so j cannot be negative - } - final int k = envVal.indexOf(':', i); - if (k >= 0 && k < j) { - // %var:...% syntax - deps.add(envVal.substring(i, k)); - } else { - // %var% syntax - deps.add(envVal.substring(i, j)); + while (matcher.find()) { + String match = matcher.group(1); + if (match.length() > 0) { + // Either store the variable name before the : string manipulation + // character or the whole match. (%var% -> var, %a:b% -> a) + String[] split = splitPattern.split(match, 2); + deps.add(split[0]); } - i = j + 1; } return deps; } From fb3590e651cdd790e02e056bcaaee3b82bb3aa59 Mon Sep 17 00:00:00 2001 From: Tamas Domok Date: Wed, 4 Aug 2021 08:54:41 +0200 Subject: [PATCH 2/4] fixup! YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies. Change-Id: If2ec5d61d64b311d9c7f0214e6aad59217095bfe --- .../containermanager/launcher/ContainerLaunch.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java index 0b11be36fe1f8..2c8b4f304040d 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java @@ -1451,8 +1451,8 @@ public Set getEnvDependencies(final String envVal) { private static final class WindowsShellScriptBuilder extends ShellScriptBuilder { - private static final Pattern variablePattern = Pattern.compile("%(.*?)%"); - private static final Pattern splitPattern = Pattern.compile(":"); + private static final Pattern VARIABLE_PATTERN = Pattern.compile("%(.*?)%"); + private static final Pattern SPLIT_PATTERN = Pattern.compile(":"); private void errorCheck() { line("@if %errorlevel% neq 0 exit /b %errorlevel%"); @@ -1546,14 +1546,14 @@ public Set getEnvDependencies(final String envVal) { } // Example inputs: %var%, %%, %a:b% - Matcher matcher = variablePattern.matcher(envVal); + Matcher matcher = VARIABLE_PATTERN.matcher(envVal); final Set deps = new HashSet<>(); while (matcher.find()) { String match = matcher.group(1); if (match.length() > 0) { // Either store the variable name before the : string manipulation // character or the whole match. (%var% -> var, %a:b% -> a) - String[] split = splitPattern.split(match, 2); + String[] split = SPLIT_PATTERN.split(match, 2); deps.add(split[0]); } } From 822f72ca16f846f2bdfe4c07f31c03880ea86e93 Mon Sep 17 00:00:00 2001 From: Tamas Domok Date: Wed, 4 Aug 2021 09:09:53 +0200 Subject: [PATCH 3/4] fixup! YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies. Change-Id: If7e812975a108cc28c1fe6e1784a9840e60c09ce --- .../launcher/ContainerLaunch.java | 17 ++++++++++++----- .../TestContainerLaunchParameterized.java | 2 ++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java index 2c8b4f304040d..3673cee9e83b3 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java @@ -1550,11 +1550,18 @@ public Set getEnvDependencies(final String envVal) { final Set deps = new HashSet<>(); while (matcher.find()) { String match = matcher.group(1); - if (match.length() > 0) { - // Either store the variable name before the : string manipulation - // character or the whole match. (%var% -> var, %a:b% -> a) - String[] split = SPLIT_PATTERN.split(match, 2); - deps.add(split[0]); + if (!match.isEmpty()) { + if (match.equals(":")) { + // Special case, variable name can be a single : character + deps.add(match); + } else { + // Either store the variable name before the : string manipulation + // character or the whole match. (%var% -> var, %a:b% -> a) + String[] split = SPLIT_PATTERN.split(match, 2); + if (!split[0].isEmpty()) { + deps.add(split[0]); + } + } } } return deps; diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunchParameterized.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunchParameterized.java index 17a5825289aad..a36d5efc2c3bb 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunchParameterized.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunchParameterized.java @@ -118,6 +118,8 @@ private static Stream inputForGetEnvDependenciesWin() { Arguments.of("%A", asSet()), Arguments.of("%A:", asSet()), Arguments.of("%A%", asSet("A")), + Arguments.of("%:%", asSet(":")), + Arguments.of("%:A%", asSet()), Arguments.of("%%%A%", asSet("A")), Arguments.of("%%C%A%", asSet("A")), Arguments.of("%A:~-1%", asSet("A")), From d8334704611b30af3400e8978a3c0506dec46c3c Mon Sep 17 00:00:00 2001 From: Tamas Domok Date: Wed, 4 Aug 2021 10:51:04 +0200 Subject: [PATCH 4/4] Trigger the jenkins job Change-Id: I545c55bc1fa1eb2329f3d46590b793d5e237d587