[
https://issues.apache.org/jira/browse/DRILL-6445?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16498869#comment-16498869
]
ASF GitHub Bot commented on DRILL-6445:
---------------------------------------
parthchandra closed pull request #1289: DRILL-6445: Fix existing test cases in
TestScripts.java and add new t…
URL: https://github.com/apache/drill/pull/1289
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 3517cf8ee7..8a909a5d21 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.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 @@
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 ScriptUtils fromDistrib(String distrib) {
"-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 ScriptUtils fromDistrib(String distrib) {
"sqlline",
//sqlline.bat
//submit_plan
- "yarn-drillbit.sh"
+ "yarn-drillbit.sh",
+ "auto-setup.sh"
};
/**
@@ -286,24 +286,89 @@ public void writeFile(File file, String contents) throws
IOException {
}
}
+ 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 @@ private void buildFromDistrib() {
* 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 5f6b5bb005..38279f8a4a 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 void testStockCombined() throws IOException {
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 @@ private void doEnvFileTest(String fileName) throws
IOException {
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 @@ private void doEnvFileTest(String fileName) throws
IOException {
propArg,
"-Xms5G", "-Xmx5G",
"-XX:MaxDirectMemorySize=7G",
- "-XX:ReservedCodeCacheSize=2G",
- "-XX:MaxPermSize=600M"
+ "-XX:ReservedCodeCacheSize=2G"
};
result.validateArgs(expectedArgs);
@@ -378,12 +368,50 @@ private void doEnvFileTest(String fileName) throws
IOException {
.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 void testDrillAndDistribEnv() throws IOException {
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 void testDrillAndDistribEnv() throws IOException {
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 void testDrillAndDistribEnv() throws IOException {
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 void testSiteDir() throws IOException {
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 void testJavaLibDir() throws IOException {
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 void testPidDir() throws IOException {
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 void testSiteDirWithDaemon() throws IOException {
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 void testLogDirWithDaemon() throws IOException {
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 void testSqlline() throws IOException {
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 void testSqlline() throws IOException {
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 void testSqlline() throws IOException {
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 void testSqlline() throws IOException {
"-Xms5G", "-Xmx5G",
"-XX:MaxDirectMemorySize=7G",
"-XX:ReservedCodeCacheSize=2G",
- "-XX:MaxPermSize=600M"
};
result.validateArgs(expectedArgs);
@@ -1176,6 +1193,80 @@ public void testSqlline() throws IOException {
}
}
+ /**
+ * 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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Fix existing test cases in TestScripts.java and add new test case for
> DRILLBIT_CONTEXT variable
> -----------------------------------------------------------------------------------------------
>
> Key: DRILL-6445
> URL: https://issues.apache.org/jira/browse/DRILL-6445
> Project: Apache Drill
> Issue Type: Task
> Reporter: Sorabh Hamirwasia
> Assignee: Sorabh Hamirwasia
> Priority: Major
> Labels: ready-to-commit
> Fix For: 1.14.0
>
>
> Under drill-yarn module there is [TestScripts.java
> file|https://github.com/apache/drill/blob/master/drill-yarn/src/test/java/org/apache/drill/yarn/scripts/TestScripts.java]
> created for testing the scripts provided by Drill to setup the environment.
> Currently those tests are failing. This Jira is to make sure all the tests
> are passing and few new tests are added for DRILLBIT_CONTEXT variable inside
> the script.
> Also currently these tests are ignored and meant to be run in Developer
> environment. May be we can investigate in future to provide support for it to
> be run as regular tests by using DirTestWatcher
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
