This is an automated email from the ASF dual-hosted git repository. parthc pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/drill.git
commit f48894c7294b70d17ebd92d4c92cd5e60821beeb Author: Sorabh Hamirwasia <[email protected]> AuthorDate: Thu May 24 11:58:01 2018 -0700 DRILL-6445: Fix existing test cases in TestScripts.java and add new test case for DRILLBIT_CONTEXT variable This closes #1289 --- .../org/apache/drill/yarn/scripts/ScriptUtils.java | 98 +++++++++++-- .../org/apache/drill/yarn/scripts/TestScripts.java | 161 ++++++++++++++++----- 2 files changed, 208 insertions(+), 51 deletions(-) diff --git a/drill-yarn/src/test/java/org/apache/drill/yarn/scripts/ScriptUtils.java b/drill-yarn/src/test/java/org/apache/drill/yarn/scripts/ScriptUtils.java index 3517cf8..8a909a5 100644 --- a/drill-yarn/src/test/java/org/apache/drill/yarn/scripts/ScriptUtils.java +++ b/drill-yarn/src/test/java/org/apache/drill/yarn/scripts/ScriptUtils.java @@ -38,6 +38,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.regex.Pattern; @@ -66,7 +67,6 @@ public class ScriptUtils { String args[] = { "-Dlog.path=/.*/drill/log/sqlline\\.log", "-Dlog.query.path=/.*/drill/log/sqlline_queries\\.json", - "-XX:MaxPermSize=512M", "sqlline\\.SqlLine", "-d", "org\\.apache\\.drill\\.jdbc\\.Driver", @@ -123,11 +123,10 @@ public class ScriptUtils { "-Xms4G", "-Xmx4G", "-XX:MaxDirectMemorySize=8G", - "-XX:MaxPermSize=512M", "-XX:ReservedCodeCacheSize=1G", - // Removed in Drill 1.8 -// "-Ddrill\\.exec\\.enable-epoll=true", - "-XX:\\+CMSClassUnloadingEnabled", + "-Ddrill\\.exec\\.enable-epoll=false", + // Removed in Drill 1.14 + //"-XX:\\+CMSClassUnloadingEnabled", "-XX:\\+UseG1GC", "org\\.apache\\.drill\\.exec\\.server\\.Drillbit", "-Dlog\\.path=/.*/script-test/drill/log/drillbit\\.log", @@ -197,7 +196,8 @@ public class ScriptUtils { "sqlline", //sqlline.bat //submit_plan - "yarn-drillbit.sh" + "yarn-drillbit.sh", + "auto-setup.sh" }; /** @@ -286,24 +286,89 @@ public class ScriptUtils { } } + public void writeEnvFile(PrintWriter out, String key, String value, boolean overrideValue) { + out.print("export "); + out.print(key); + out.print("="); + + if (!overrideValue) { + out.print("${"); + out.print(key); + out.print(":-"); + } + out.print("\""); + out.print(value); + out.print("\""); + + if (!overrideValue) { + out.print("}"); + } + out.println(); + } + /** * Create a drill-env.sh or distrib-env.sh file with the given environment in * the recommended format. + * different formats based on overrideValue flag + * + * @param file - File instance to set environment variables in + * @param env - Environment to be placed inside File + * @param overrideValue - true - Set environment value such that it overrides previously set value + * - false - Set environment value in recommended format. */ - - public void createEnvFile(File file, Map<String, String> env) + public void createEnvFile(File file, Map<String, String> env, boolean overrideValue) throws IOException { try (PrintWriter out = new PrintWriter(new FileWriter(file))) { out.println("#!/usr/bin/env bash"); for (String key : env.keySet()) { String value = env.get(key); - out.print("export "); - out.print(key); - out.print("=${"); - out.print(key); - out.print(":-\""); - out.print(value); - out.println("\"}"); + writeEnvFile(out, key, value, overrideValue); + } + } + } + + /** + * Creates a drill-env.sh or distrib-env.sh file with the given environment under + * a given condition. If size of env map is smaller than condition map then last + * env entry is repeated for rest of conditions. + * + * @param file - File instance to set environment and condition in + * @param condition - Conditions to guard environment variable + * @param env - Environment to be placed inside File + * @param overrideValue - true - Set environment value such that it overrides previously set value + * - false - Set environment value in recommended format. + * + */ + public void createEnvFileWithCondition(File file, Map<String, String> condition, + Map<String, String> env, boolean overrideValue) throws IOException { + if (env.size() == 0 || condition.size() == 0) { + return; + } + + final Iterator envIterator = env.entrySet().iterator(); + Map.Entry currentEnv = (Map.Entry) envIterator.next(); + + try (PrintWriter out = new PrintWriter(new FileWriter(file))) { + out.println("#!/usr/bin/env bash"); + + for (String condKey : condition.keySet()) { + String condValue = condition.get(condKey); + out.print("if [ \"$"); + out.print(condKey); + out.print("\" = \""); + out.print(condValue); + out.println("\" ]; then"); + + final String envKey = currentEnv.getKey().toString(); + final String envValue = currentEnv.getValue().toString(); + writeEnvFile(out, envKey, envValue, overrideValue); + + out.println("fi"); + out.println(); + + if (envIterator.hasNext()) { + currentEnv = (Map.Entry) envIterator.next(); + } } } } @@ -342,7 +407,8 @@ public class ScriptUtils { * Consume the input from a stream, specifically the stderr or stdout stream * from a process. * - * @see http://stackoverflow.com/questions/14165517/processbuilder-forwarding-stdout-and-stderr-of-started-processes-without-blocki + * @link http://stackoverflow.com/questions/14165517/processbuilder-forwarding-stdout-and-stderr-of-started-processes + * -without-blocki */ private static class StreamGobbler extends Thread { diff --git a/drill-yarn/src/test/java/org/apache/drill/yarn/scripts/TestScripts.java b/drill-yarn/src/test/java/org/apache/drill/yarn/scripts/TestScripts.java index 5f6b5bb..38279f8 100644 --- a/drill-yarn/src/test/java/org/apache/drill/yarn/scripts/TestScripts.java +++ b/drill-yarn/src/test/java/org/apache/drill/yarn/scripts/TestScripts.java @@ -159,14 +159,6 @@ public class TestScripts { result.validateArgRegex("-Xloggc:.*/" + logTail); } - // Max Perm Size - - { - RunResult result = new DrillbitRun(DrillbitRun.DRILLBIT_RUN) - .addEnv("DRILLBIT_MAX_PERM", "600M").run(); - result.validateArg("-XX:MaxPermSize=600M"); - } - // Code cache size { @@ -346,9 +338,8 @@ public class TestScripts { drillEnv.put("DRILL_HEAP", "5G"); drillEnv.put("DRILL_MAX_DIRECT_MEMORY", "7G"); drillEnv.put("SERVER_LOG_GC", "1"); - drillEnv.put("DRILLBIT_MAX_PERM", "600M"); drillEnv.put("DRILLBIT_CODE_CACHE_SIZE", "2G"); - context.createEnvFile(new File(siteDir, fileName), drillEnv); + context.createEnvFile(new File(siteDir, fileName), drillEnv, false); { RunResult result = new DrillbitRun(DrillbitRun.DRILLBIT_RUN).run(); @@ -358,8 +349,7 @@ public class TestScripts { propArg, "-Xms5G", "-Xmx5G", "-XX:MaxDirectMemorySize=7G", - "-XX:ReservedCodeCacheSize=2G", - "-XX:MaxPermSize=600M" + "-XX:ReservedCodeCacheSize=2G" }; result.validateArgs(expectedArgs); @@ -378,12 +368,50 @@ public class TestScripts { .run(); assertEquals(0, result.returnCode); result.validateArg("-XX:MaxDirectMemorySize=9G"); - result.validateArg("-XX:MaxPermSize=600M"); String logTail = context.testDrillHome.getName() + "/log/drillbit.gc"; assertFalse(result.containsArgRegex("-Xloggc:.*/" + logTail)); } } + @Test + public void testDistribEnvWithNegativeCond() throws IOException { + // Construct condition map + final Map<String, String> conditions = new HashMap<>(); + conditions.put("DRILLBIT_CONTEXT", "0"); + final String expectedArgs[] = {"-XX:ReservedCodeCacheSize=1G"}; + doEnvFileWithConditionTest("distrib-env.sh", conditions, expectedArgs); + } + + @Test + public void testDistribEnvWithPositiveCond() throws IOException { + // Construct condition map + final Map<String, String> conditions = new HashMap<>(); + conditions.put("DRILLBIT_CONTEXT", "1"); + final String expectedArgs[] = {"-XX:ReservedCodeCacheSize=2G"}; + doEnvFileWithConditionTest("distrib-env.sh", conditions, expectedArgs); + } + + /** + * Implementation of the drill-env.sh or distrib-env.sh tests with conditions + * guarding environment variables. + */ + private void doEnvFileWithConditionTest(String fileName, Map<String, String> conditions, + String[] expectedArgs) throws IOException { + context.createMockDistrib(); + File siteDir = new File(context.testDrillHome, "conf"); + context.createMockConf(siteDir); + + // Set a property in the env file. + Map<String, String> drillEnv = new HashMap<>(); + drillEnv.put("DRILLBIT_CODE_CACHE_SIZE", "2G"); + context.createEnvFileWithCondition(new File(siteDir, fileName), conditions, drillEnv, false); + { + RunResult result = new DrillbitRun(DrillbitRun.DRILLBIT_RUN).run(); + assertEquals(0, result.returnCode); + result.validateArgs(expectedArgs); + } + } + /** * Test that drill-env.sh overrides distrib-env.sh, and that the environment * overrides both. Assumes the basics were tested above. @@ -400,13 +428,12 @@ public class TestScripts { Map<String, String> distribEnv = new HashMap<>(); distribEnv.put("DRILL_HEAP", "5G"); distribEnv.put("DRILL_MAX_DIRECT_MEMORY", "7G"); - distribEnv.put("DRILLBIT_MAX_PERM", "600M"); - context.createEnvFile(new File(siteDir, "distrib-env.sh"), distribEnv); + context.createEnvFile(new File(siteDir, "distrib-env.sh"), distribEnv, false); Map<String, String> drillEnv = new HashMap<>(); drillEnv.put("DRILL_HEAP", "6G"); drillEnv.put("DRILL_MAX_DIRECT_MEMORY", "9G"); - context.createEnvFile(new File(siteDir, "drill-env.sh"), drillEnv); + context.createEnvFile(new File(siteDir, "drill-env.sh"), drillEnv, false); { RunResult result = new DrillbitRun(DrillbitRun.DRILLBIT_RUN).run(); @@ -414,8 +441,7 @@ public class TestScripts { String expectedArgs[] = { "-Xms6G", "-Xmx6G", "-XX:MaxDirectMemorySize=9G", - "-XX:MaxPermSize=600M", - "-XX:ReservedCodeCacheSize=1G" // Default + "-XX:ReservedCodeCacheSize=1024m" // Default }; result.validateArgs(expectedArgs); @@ -428,8 +454,7 @@ public class TestScripts { String expectedArgs[] = { "-Xms6G", "-Xmx6G", "-XX:MaxDirectMemorySize=5G", - "-XX:MaxPermSize=600M", - "-XX:ReservedCodeCacheSize=1G" // Default + "-XX:ReservedCodeCacheSize=1024m" // Default }; result.validateArgs(expectedArgs); @@ -498,19 +523,17 @@ public class TestScripts { Map<String, String> distribEnv = new HashMap<>(); distribEnv.put("DRILL_HEAP", "5G"); distribEnv.put("DRILL_MAX_DIRECT_MEMORY", "7G"); - distribEnv.put("DRILLBIT_MAX_PERM", "600M"); - context.createEnvFile(new File(confDir, "distrib-env.sh"), distribEnv); + context.createEnvFile(new File(confDir, "distrib-env.sh"), distribEnv, false); Map<String, String> drillEnv = new HashMap<>(); drillEnv.put("DRILL_HEAP", "6G"); drillEnv.put("DRILL_MAX_DIRECT_MEMORY", "9G"); - context.createEnvFile(new File(siteDir, "drill-env.sh"), drillEnv); + context.createEnvFile(new File(siteDir, "drill-env.sh"), drillEnv, false); String expectedArgs[] = { "-Xms6G", "-Xmx6G", "-XX:MaxDirectMemorySize=9G", - "-XX:MaxPermSize=600M", - "-XX:ReservedCodeCacheSize=1G" // Default + "-XX:ReservedCodeCacheSize=1024m" // Default }; // Site set using argument @@ -611,8 +634,7 @@ public class TestScripts { String prefix = "-Djava.library.path="; { - RunResult result = new DrillbitRun(DrillbitRun.DRILLBIT_RUN) - .run(); + RunResult result = new DrillbitRun(DrillbitRun.DRILLBIT_RUN).run(); assertFalse(result.containsArgRegex(prefix + ".*")); assertNull(result.libPath); } @@ -874,7 +896,7 @@ public class TestScripts { File pidDir = context.createDir(new File(context.testDir, "pid")); Map<String, String> drillEnv = new HashMap<>(); drillEnv.put("DRILL_PID_DIR", pidDir.getAbsolutePath()); - context.createEnvFile(new File(siteDir, "drill-env.sh"), drillEnv); + context.createEnvFile(new File(siteDir, "drill-env.sh"), drillEnv, false); { RunResult result = new DrillbitRun(DrillbitRun.DRILLBIT_START) @@ -905,7 +927,7 @@ public class TestScripts { Map<String, String> drillEnv = new HashMap<>(); drillEnv.put("DRILL_MAX_DIRECT_MEMORY", "9G"); - context.createEnvFile(new File(siteDir, "drill-env.sh"), drillEnv); + context.createEnvFile(new File(siteDir, "drill-env.sh"), drillEnv, false); // Use the -site (--config) option. @@ -948,7 +970,7 @@ public class TestScripts { context.removeDir(new File(context.testDrillHome, "log")); Map<String, String> drillEnv = new HashMap<>(); drillEnv.put("DRILL_LOG_DIR", logsDir.getAbsolutePath()); - context.createEnvFile(new File(siteDir, "drill-env.sh"), drillEnv); + context.createEnvFile(new File(siteDir, "drill-env.sh"), drillEnv, false); { DrillbitRun runner = new DrillbitRun(DrillbitRun.DRILLBIT_START); @@ -1122,7 +1144,6 @@ public class TestScripts { drillEnv.put("DRILL_HEAP", "5G"); drillEnv.put("DRILL_MAX_DIRECT_MEMORY", "7G"); drillEnv.put("SERVER_LOG_GC", "1"); - drillEnv.put("DRILLBIT_MAX_PERM", "600M"); drillEnv.put("DRILLBIT_CODE_CACHE_SIZE", "2G"); RunResult result = new ScriptRunner("sqlline") .withEnvironment(drillEnv) @@ -1138,11 +1159,9 @@ public class TestScripts { Map<String, String> shellEnv = new HashMap<>(); shellEnv.put("CLIENT_GC_OPTS", "-XX:+UseG1GC"); - shellEnv.put("SQLLINE_JAVA_OPTS", "-XX:MaxPermSize=256M"); RunResult result = new ScriptRunner("sqlline") .withEnvironment(shellEnv) .run(); - assertTrue(result.containsArg("-XX:MaxPermSize=256M")); assertTrue(result.containsArg("-XX:+UseG1GC")); } { @@ -1156,7 +1175,6 @@ public class TestScripts { drillEnv.put("DRILL_HEAP", "5G"); drillEnv.put("DRILL_MAX_DIRECT_MEMORY", "7G"); drillEnv.put("SERVER_LOG_GC", "1"); - drillEnv.put("DRILLBIT_MAX_PERM", "600M"); drillEnv.put("DRILLBIT_CODE_CACHE_SIZE", "2G"); drillEnv.put("DRILL_EMBEDDED", "1"); RunResult result = new ScriptRunner("sqlline") @@ -1168,7 +1186,6 @@ public class TestScripts { "-Xms5G", "-Xmx5G", "-XX:MaxDirectMemorySize=7G", "-XX:ReservedCodeCacheSize=2G", - "-XX:MaxPermSize=600M" }; result.validateArgs(expectedArgs); @@ -1177,6 +1194,80 @@ public class TestScripts { } /** + * Test to verify no effect of DRILLBIT_CONTEXT for Sqlline. + * @throws IOException + */ + @Test + public void testSqllineWithDrillbitContextEnv() throws IOException { + context.createMockDistrib(); + File siteDir = new File(context.testDrillHome, "conf"); + context.createMockConf(siteDir); + + // Test when SQLLINE_JAVA_OPTS is overriden inside a condition for + // DRILLBIT_CONTEXT = 0, then there is no effect + { + // Create a condition variable to be placed in distrib-env.sh + Map<String, String> conditions = new HashMap<>(); + conditions.put("DRILLBIT_CONTEXT", "0"); + + // Create environment variable to be placed inside a condition in distrib-env.sh + Map<String, String> drillEnv = new HashMap<>(); + drillEnv.put("SQLLINE_JAVA_OPTS", "-XX:MaxPermSize=256M"); + + // Create the environment variable file overriding SQLLINE_JAVA_OPTS + context.createEnvFileWithCondition(new File(siteDir, "distrib-env.sh"), conditions, drillEnv, true); + + // Expected value of the property + String expectedArgs[] = {"-XX:MaxPermSize=256M"}; + + // Run the test and match the output with expectedArgs + RunResult result = new ScriptRunner("sqlline").run(); + assertEquals(0, result.returnCode); + result.validateJava(); + result.validateClassPath(ScriptUtils.stdCp); + // Since by default MaxPermSize is not set anymore for Sqlline. It's removed in 1.13 + assertFalse(result.containsArgsRegex(expectedArgs)); + } + + // Test when SQLLINE_JAVA_OPTS is overriden inside a condition for + // DRILLBIT_CONTEXT = 1, then there is no effect + { + Map<String, String> conditions = new HashMap<>(); + conditions.put("DRILLBIT_CONTEXT", "1"); + + Map<String, String> drillEnv = new HashMap<>(); + drillEnv.put("SQLLINE_JAVA_OPTS", "-XX:MaxPermSize=256M"); + String expectedArgs[] = {"-XX:MaxPermSize=256M"}; + + // Create the environment variable file overriding SQLLINE_JAVA_OPTS + context.createEnvFileWithCondition(new File(siteDir, "distrib-env.sh"), conditions, drillEnv, true); + RunResult result = new ScriptRunner("sqlline").run(); + assertEquals(0, result.returnCode); + result.validateJava(); + result.validateClassPath(ScriptUtils.stdCp); + // Since by default MaxPermSize is not set anymore for Sqlline. It's removed in 1.13 + assertFalse(result.containsArgsRegex(expectedArgs)); + } + + // Test when SQLLINE_JAVA_OPTS is overriden without condition for + // DRILLBIT_CONTEXT then the environment variable is updated + { + Map<String, String> drillEnv = new HashMap<>(); + drillEnv.put("SQLLINE_JAVA_OPTS", "-XX:MaxPermSize=256M"); + + // Create the environment variable file overriding SQLLINE_JAVA_OPTS without any condition + // around it. + String expectedArgs[] = {"-XX:MaxPermSize=256M"}; + context.createEnvFile(new File(siteDir, "distrib-env.sh"), drillEnv, true); + RunResult result = new ScriptRunner("sqlline").run(); + assertEquals(0, result.returnCode); + result.validateJava(); + result.validateClassPath(ScriptUtils.stdCp); + assertTrue(result.containsArgsRegex(expectedArgs)); + } + } + + /** * Verify that the sqlline client works with the --site option by customizing * items in the site directory. * -- To stop receiving notification emails like this one, please contact [email protected].
