[jira] [Updated] (DRILL-4581) Various problems in the Drill startup scripts
[ https://issues.apache.org/jira/browse/DRILL-4581?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Suresh Ollala updated DRILL-4581: - Reviewer: Abhishek Girish (was: Sudheesh Katkam) [~agirish]assigning for verification > Various problems in the Drill startup scripts > - > > Key: DRILL-4581 > URL: https://issues.apache.org/jira/browse/DRILL-4581 > Project: Apache Drill > Issue Type: Sub-task > Components: Server >Affects Versions: 1.6.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.8.0 > > > Noticed the following in drillbit.sh: > 1) Comment: DRILL_LOG_DIRWhere log files are stored. PWD by default. > Code: DRILL_LOG_DIR=/var/log/drill or, if it does not exist, $DRILL_HOME/log > 2) Comment: DRILL_PID_DIRThe pid files are stored. /tmp by default. > Code: DRILL_PID_DIR=$DRILL_HOME > 3) Redundant checking of JAVA_HOME. drillbit.sh sources drill-config.sh which > checks JAVA_HOME. Later, drillbit.sh checks it again. The second check is > both unnecessary and prints a less informative message than the > drill-config.sh check. Suggestion: Remove the JAVA_HOME check in drillbit.sh. > 4) Though drill-config.sh carefully checks JAVA_HOME, it does not export the > JAVA_HOME variable. Perhaps this is why drillbit.sh repeats the check? > Recommended: export JAVA_HOME from drill-config.sh. > 5) Both drillbit.sh and the sourced drill-config.sh check DRILL_LOG_DIR and > set the default value. Drill-config.sh defaults to /var/log/drill, or if that > fails, to $DRILL_HOME/log. Drillbit.sh just sets /var/log/drill and does not > handle the case where that directory is not writable. Suggested: remove the > check in drillbit.sh. > 6) Drill-config.sh checks the writability of the DRILL_LOG_DIR by touching > sqlline.log, but does not delete that file, leaving a bogus, empty client log > file on the drillbit server. Recommendation: use bash commands instead. > 7) The implementation of the above check is a bit awkward. It has a fallback > case with somewhat awkward logic. Clean this up. > 8) drillbit.sh, but not drill-config.sh, attempts to create /var/log/drill if > it does not exist. Recommended: decide on a single choice, implement it in > drill-config.sh. > 9) drill-config.sh checks if $DRILL_CONF_DIR is a directory. If not, defaults > it to $DRILL_HOME/conf. This can lead to subtle errors. If I use > drillbit.sh --config /misspelled/path > where I mistype the path, I won't get an error, I get the default config, > which may not at all be what I want to run. Recommendation: if the value of > DRILL_CONF_DRILL is passed into the script (as a variable or via --config), > then that directory must exist. Else, use the default. > 10) drill-config.sh exports, but may not set, HADOOP_HOME. This may be left > over from the original Hadoop script that the Drill script was based upon. > Recomendation: export only in the case that HADOOP_HOME is set for cygwin. > 11) Drill-config.sh checks JAVA_HOME and prints a big, bold error message to > stderr if JAVA_HOME is not set. Then, it checks the Java version and prints a > different message (to stdout) if the version is wrong. Recommendation: use > the same format (and stderr) for both. > 12) Similarly, other Java checks later in the script produce messages to > stdout, not stderr. > 13) Drill-config.sh searches $JAVA_HOME to find java/java.exe and verifies > that it is executable. The script then throws away what we just found. Then, > drill-bit.sh tries to recreate this information as: > JAVA=$JAVA_HOME/bin/java > This is wrong in two ways: 1) it ignores the actual java location and assumes > it, and 2) it does not handle the java.exe case that drill-config.sh > carefully worked out. > Recommendation: export JAVA from drill-config.sh and remove the above line > from drillbit.sh. > 14) drillbit.sh presumably takes extra arguments like this: > drillbit.sh -Dvar0=value0 --config /my/conf/dir start -Dvar1=value1 > -Dvar2=value2 -Dvar3=value3 > The -D bit allows the user to override config variables at the command line. > But, the scripts don't use the values. > A) drill-config.sh consumes --config /my/conf/dir after consuming the leading > arguments: > while [ $# -gt 1 ]; do > if [ "--config" = "$1" ]; then > shift > confdir=$1 > shift > DRILL_CONF_DIR=$confdir > else > # Presume we are at end of options and break > break > fi > done > B) drill-bit.sh will discard the var1: > startStopStatus=$1 <-- grabs "start" > shift > command=drillbit > shift <-- Consumes -Dvar1=value1 > C) Remaining values passed back into drillbit.sh: > args=$@ > nohup $thiscmd internal_start $command $args > D) Second invocation discards -Dvar2=value2 as described above. > E) Remaining values
[jira] [Updated] (DRILL-4581) Various problems in the Drill startup scripts
[ https://issues.apache.org/jira/browse/DRILL-4581?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Rogers updated DRILL-4581: --- Issue Type: Sub-task (was: Bug) Parent: DRILL-1170 > Various problems in the Drill startup scripts > - > > Key: DRILL-4581 > URL: https://issues.apache.org/jira/browse/DRILL-4581 > Project: Apache Drill > Issue Type: Sub-task > Components: Server >Affects Versions: 1.6.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > > Noticed the following in drillbit.sh: > 1) Comment: DRILL_LOG_DIRWhere log files are stored. PWD by default. > Code: DRILL_LOG_DIR=/var/log/drill or, if it does not exist, $DRILL_HOME/log > 2) Comment: DRILL_PID_DIRThe pid files are stored. /tmp by default. > Code: DRILL_PID_DIR=$DRILL_HOME > 3) Redundant checking of JAVA_HOME. drillbit.sh sources drill-config.sh which > checks JAVA_HOME. Later, drillbit.sh checks it again. The second check is > both unnecessary and prints a less informative message than the > drill-config.sh check. Suggestion: Remove the JAVA_HOME check in drillbit.sh. > 4) Though drill-config.sh carefully checks JAVA_HOME, it does not export the > JAVA_HOME variable. Perhaps this is why drillbit.sh repeats the check? > Recommended: export JAVA_HOME from drill-config.sh. > 5) Both drillbit.sh and the sourced drill-config.sh check DRILL_LOG_DIR and > set the default value. Drill-config.sh defaults to /var/log/drill, or if that > fails, to $DRILL_HOME/log. Drillbit.sh just sets /var/log/drill and does not > handle the case where that directory is not writable. Suggested: remove the > check in drillbit.sh. > 6) Drill-config.sh checks the writability of the DRILL_LOG_DIR by touching > sqlline.log, but does not delete that file, leaving a bogus, empty client log > file on the drillbit server. Recommendation: use bash commands instead. > 7) The implementation of the above check is a bit awkward. It has a fallback > case with somewhat awkward logic. Clean this up. > 8) drillbit.sh, but not drill-config.sh, attempts to create /var/log/drill if > it does not exist. Recommended: decide on a single choice, implement it in > drill-config.sh. > 9) drill-config.sh checks if $DRILL_CONF_DIR is a directory. If not, defaults > it to $DRILL_HOME/conf. This can lead to subtle errors. If I use > drillbit.sh --config /misspelled/path > where I mistype the path, I won't get an error, I get the default config, > which may not at all be what I want to run. Recommendation: if the value of > DRILL_CONF_DRILL is passed into the script (as a variable or via --config), > then that directory must exist. Else, use the default. > 10) drill-config.sh exports, but may not set, HADOOP_HOME. This may be left > over from the original Hadoop script that the Drill script was based upon. > Recomendation: export only in the case that HADOOP_HOME is set for cygwin. > 11) Drill-config.sh checks JAVA_HOME and prints a big, bold error message to > stderr if JAVA_HOME is not set. Then, it checks the Java version and prints a > different message (to stdout) if the version is wrong. Recommendation: use > the same format (and stderr) for both. > 12) Similarly, other Java checks later in the script produce messages to > stdout, not stderr. > 13) Drill-config.sh searches $JAVA_HOME to find java/java.exe and verifies > that it is executable. The script then throws away what we just found. Then, > drill-bit.sh tries to recreate this information as: > JAVA=$JAVA_HOME/bin/java > This is wrong in two ways: 1) it ignores the actual java location and assumes > it, and 2) it does not handle the java.exe case that drill-config.sh > carefully worked out. > Recommendation: export JAVA from drill-config.sh and remove the above line > from drillbit.sh. > 14) drillbit.sh presumably takes extra arguments like this: > drillbit.sh -Dvar0=value0 --config /my/conf/dir start -Dvar1=value1 > -Dvar2=value2 -Dvar3=value3 > The -D bit allows the user to override config variables at the command line. > But, the scripts don't use the values. > A) drill-config.sh consumes --config /my/conf/dir after consuming the leading > arguments: > while [ $# -gt 1 ]; do > if [ "--config" = "$1" ]; then > shift > confdir=$1 > shift > DRILL_CONF_DIR=$confdir > else > # Presume we are at end of options and break > break > fi > done > B) drill-bit.sh will discard the var1: > startStopStatus=$1 <-- grabs "start" > shift > command=drillbit > shift <-- Consumes -Dvar1=value1 > C) Remaining values passed back into drillbit.sh: > args=$@ > nohup $thiscmd internal_start $command $args > D) Second invocation discards -Dvar2=value2 as described above. > E) Remaining values are passed to runbit: > "$DRILL_HOME"/bin/runbit $command "$@"
[jira] [Updated] (DRILL-4581) Various problems in the Drill startup scripts
[ https://issues.apache.org/jira/browse/DRILL-4581?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Rogers updated DRILL-4581: --- Description: Noticed the following in drillbit.sh: 1) Comment: DRILL_LOG_DIRWhere log files are stored. PWD by default. Code: DRILL_LOG_DIR=/var/log/drill or, if it does not exist, $DRILL_HOME/log 2) Comment: DRILL_PID_DIRThe pid files are stored. /tmp by default. Code: DRILL_PID_DIR=$DRILL_HOME 3) Redundant checking of JAVA_HOME. drillbit.sh sources drill-config.sh which checks JAVA_HOME. Later, drillbit.sh checks it again. The second check is both unnecessary and prints a less informative message than the drill-config.sh check. Suggestion: Remove the JAVA_HOME check in drillbit.sh. 4) Though drill-config.sh carefully checks JAVA_HOME, it does not export the JAVA_HOME variable. Perhaps this is why drillbit.sh repeats the check? Recommended: export JAVA_HOME from drill-config.sh. 5) Both drillbit.sh and the sourced drill-config.sh check DRILL_LOG_DIR and set the default value. Drill-config.sh defaults to /var/log/drill, or if that fails, to $DRILL_HOME/log. Drillbit.sh just sets /var/log/drill and does not handle the case where that directory is not writable. Suggested: remove the check in drillbit.sh. 6) Drill-config.sh checks the writability of the DRILL_LOG_DIR by touching sqlline.log, but does not delete that file, leaving a bogus, empty client log file on the drillbit server. Recommendation: use bash commands instead. 7) The implementation of the above check is a bit awkward. It has a fallback case with somewhat awkward logic. Clean this up. 8) drillbit.sh, but not drill-config.sh, attempts to create /var/log/drill if it does not exist. Recommended: decide on a single choice, implement it in drill-config.sh. 9) drill-config.sh checks if $DRILL_CONF_DIR is a directory. If not, defaults it to $DRILL_HOME/conf. This can lead to subtle errors. If I use drillbit.sh --config /misspelled/path where I mistype the path, I won't get an error, I get the default config, which may not at all be what I want to run. Recommendation: if the value of DRILL_CONF_DRILL is passed into the script (as a variable or via --config), then that directory must exist. Else, use the default. 10) drill-config.sh exports, but may not set, HADOOP_HOME. This may be left over from the original Hadoop script that the Drill script was based upon. Recomendation: export only in the case that HADOOP_HOME is set for cygwin. 11) Drill-config.sh checks JAVA_HOME and prints a big, bold error message to stderr if JAVA_HOME is not set. Then, it checks the Java version and prints a different message (to stdout) if the version is wrong. Recommendation: use the same format (and stderr) for both. 12) Similarly, other Java checks later in the script produce messages to stdout, not stderr. 13) Drill-config.sh searches $JAVA_HOME to find java/java.exe and verifies that it is executable. The script then throws away what we just found. Then, drill-bit.sh tries to recreate this information as: JAVA=$JAVA_HOME/bin/java This is wrong in two ways: 1) it ignores the actual java location and assumes it, and 2) it does not handle the java.exe case that drill-config.sh carefully worked out. Recommendation: export JAVA from drill-config.sh and remove the above line from drillbit.sh. 14) drillbit.sh presumably takes extra arguments like this: drillbit.sh -Dvar0=value0 --config /my/conf/dir start -Dvar1=value1 -Dvar2=value2 -Dvar3=value3 The -D bit allows the user to override config variables at the command line. But, the scripts don't use the values. A) drill-config.sh consumes --config /my/conf/dir after consuming the leading arguments: while [ $# -gt 1 ]; do if [ "--config" = "$1" ]; then shift confdir=$1 shift DRILL_CONF_DIR=$confdir else # Presume we are at end of options and break break fi done B) drill-bit.sh will discard the var1: startStopStatus=$1 <-- grabs "start" shift command=drillbit shift <-- Consumes -Dvar1=value1 C) Remaining values passed back into drillbit.sh: args=$@ nohup $thiscmd internal_start $command $args D) Second invocation discards -Dvar2=value2 as described above. E) Remaining values are passed to runbit: "$DRILL_HOME"/bin/runbit $command "$@" start F) Where they again pass though drill-config. (Allowing us to do: drillbit.sh --config /first/conf --config /second/conf which is asking for trouble) G) And, the remaining arguments are simply not used: exec $JAVA -Dlog.path=$DRILLBIT_LOG_PATH -Dlog.query.path=$DRILLBIT_QUERY_LOG_PATH $DRILL_ALL_JAVA_OPTS -cp $CP org.apache.drill.exec.server.Drillbit 15) The checking of command-line args in drillbit.sh is wrong: # if no args specified, show usage if [ $# -lt 1 ]; then echo $usage exit 1 fi ... . "$bin"/drill-config.sh But, note, that drill-config.sh handles: drillbit.sh --config /conf/dir Consuming
[jira] [Updated] (DRILL-4581) Various problems in the Drill startup scripts
[ https://issues.apache.org/jira/browse/DRILL-4581?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Rogers updated DRILL-4581: --- Description: Noticed the following in drillbit.sh: 1) Comment: DRILL_LOG_DIRWhere log files are stored. PWD by default. Code: DRILL_LOG_DIR=/var/log/drill or, if it does not exist, $DRILL_HOME/log 2) Comment: DRILL_PID_DIRThe pid files are stored. /tmp by default. Code: DRILL_PID_DIR=$DRILL_HOME 3) Redundant checking of JAVA_HOME. drillbit.sh sources drill-config.sh which checks JAVA_HOME. Later, drillbit.sh checks it again. The second check is both unnecessary and prints a less informative message than the drill-config.sh check. Suggestion: Remove the JAVA_HOME check in drillbit.sh. 4) Though drill-config.sh carefully checks JAVA_HOME, it does not export the JAVA_HOME variable. Perhaps this is why drillbit.sh repeats the check? Recommended: export JAVA_HOME from drill-config.sh. 5) Both drillbit.sh and the sourced drill-config.sh check DRILL_LOG_DIR and set the default value. Drill-config.sh defaults to /var/log/drill, or if that fails, to $DRILL_HOME/log. Drillbit.sh just sets /var/log/drill and does not handle the case where that directory is not writable. Suggested: remove the check in drillbit.sh. 6) Drill-config.sh checks the writability of the DRILL_LOG_DIR by touching sqlline.log, but does not delete that file, leaving a bogus, empty client log file on the drillbit server. Recommendation: use bash commands instead. 7) The implementation of the above check is a bit awkward. It has a fallback case with somewhat awkward logic. Clean this up. 8) drillbit.sh, but not drill-config.sh, attempts to create /var/log/drill if it does not exist. Recommended: decide on a single choice, implement it in drill-config.sh. 9) drill-config.sh checks if $DRILL_CONF_DIR is a directory. If not, defaults it to $DRILL_HOME/conf. This can lead to subtle errors. If I use drillbit.sh --config /misspelled/path where I mistype the path, I won't get an error, I get the default config, which may not at all be what I want to run. Recommendation: if the value of DRILL_CONF_DRILL is passed into the script (as a variable or via --config), then that directory must exist. Else, use the default. 10) drill-config.sh exports, but may not set, HADOOP_HOME. This may be left over from the original Hadoop script that the Drill script was based upon. Recomendation: export only in the case that HADOOP_HOME is set for cygwin. 11) Drill-config.sh checks JAVA_HOME and prints a big, bold error message to stderr if JAVA_HOME is not set. Then, it checks the Java version and prints a different message (to stdout) if the version is wrong. Recommendation: use the same format (and stderr) for both. 12) Similarly, other Java checks later in the script produce messages to stdout, not stderr. 13) Drill-config.sh searches $JAVA_HOME to find java/java.exe and verifies that it is executable. The script then throws away what we just found. Then, drill-bit.sh tries to recreate this information as: JAVA=$JAVA_HOME/bin/java This is wrong in two ways: 1) it ignores the actual java location and assumes it, and 2) it does not handle the java.exe case that drill-config.sh carefully worked out. Recommendation: export JAVA from drill-config.sh and remove the above line from drillbit.sh. 14) drillbit.sh presumably takes extra arguments like this: drillbit.sh -Dvar0=value0 --config /my/conf/dir start -Dvar1=value1 -Dvar2=value2 -Dvar3=value3 The -D bit allows the user to override config variables at the command line. But, the scripts don't use the values. A) drill-config.sh consumes --config /my/conf/dir after consuming the leading arguments: while [ $# -gt 1 ]; do if [ "--config" = "$1" ]; then shift confdir=$1 shift DRILL_CONF_DIR=$confdir else # Presume we are at end of options and break break fi done B) drill-bit.sh will discard the var1: startStopStatus=$1 <-- grabs "start" shift command=drillbit shift <-- Consumes -Dvar1=value1 C) Remaining values passed back into drillbit.sh: args=$@ nohup $thiscmd internal_start $command $args D) Second invocation discards -Dvar2=value2 as described above. E) Remaining values are passed to runbit: "$DRILL_HOME"/bin/runbit $command "$@" start F) Where they again pass though drill-config. (Allowing us to do: drillbit.sh --config /first/conf --config /second/conf which is asking for trouble) G) And, the remaining arguments are simply not used: exec $JAVA -Dlog.path=$DRILLBIT_LOG_PATH -Dlog.query.path=$DRILLBIT_QUERY_LOG_PATH $DRILL_ALL_JAVA_OPTS -cp $CP org.apache.drill.exec.server.Drillbit 15) The checking of command-line args in drillbit.sh is wrong: # if no args specified, show usage if [ $# -lt 1 ]; then echo $usage exit 1 fi ... . "$bin"/drill-config.sh But, note, that drill-config.sh handles: drillbit.sh --config /conf/dir Consuming
[jira] [Updated] (DRILL-4581) Various problems in the Drill startup scripts
[ https://issues.apache.org/jira/browse/DRILL-4581?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Rogers updated DRILL-4581: --- Description: Noticed the following in drillbit.sh: 1) Comment: DRILL_LOG_DIRWhere log files are stored. PWD by default. Code: DRILL_LOG_DIR=/var/log/drill or, if it does not exist, $DRILL_HOME/log 2) Comment: DRILL_PID_DIRThe pid files are stored. /tmp by default. Code: DRILL_PID_DIR=$DRILL_HOME 3) Redundant checking of JAVA_HOME. drillbit.sh sources drill-config.sh which checks JAVA_HOME. Later, drillbit.sh checks it again. The second check is both unnecessary and prints a less informative message than the drill-config.sh check. Suggestion: Remove the JAVA_HOME check in drillbit.sh. 4) Though drill-config.sh carefully checks JAVA_HOME, it does not export the JAVA_HOME variable. Perhaps this is why drillbit.sh repeats the check? Recommended: export JAVA_HOME from drill-config.sh. 5) Both drillbit.sh and the sourced drill-config.sh check DRILL_LOG_DIR and set the default value. Drill-config.sh defaults to /var/log/drill, or if that fails, to $DRILL_HOME/log. Drillbit.sh just sets /var/log/drill and does not handle the case where that directory is not writable. Suggested: remove the check in drillbit.sh. 6) Drill-config.sh checks the writability of the DRILL_LOG_DIR by touching sqlline.log, but does not delete that file, leaving a bogus, empty client log file on the drillbit server. Recommendation: use bash commands instead. 7) The implementation of the above check is a bit awkward. It has a fallback case with somewhat awkward logic. Clean this up. 8) drillbit.sh, but not drill-config.sh, attempts to create /var/log/drill if it does not exist. Recommended: decide on a single choice, implement it in drill-config.sh. 9) drill-config.sh checks if $DRILL_CONF_DIR is a directory. If not, defaults it to $DRILL_HOME/conf. This can lead to subtle errors. If I use drillbit.sh --config /misspelled/path where I mistype the path, I won't get an error, I get the default config, which may not at all be what I want to run. Recommendation: if the value of DRILL_CONF_DRILL is passed into the script (as a variable or via --config), then that directory must exist. Else, use the default. 10) drill-config.sh exports, but may not set, HADOOP_HOME. This may be left over from the original Hadoop script that the Drill script was based upon. Recomendation: export only in the case that HADOOP_HOME is set for cygwin. 11) Drill-config.sh checks JAVA_HOME and prints a big, bold error message to stderr if JAVA_HOME is not set. Then, it checks the Java version and prints a different message (to stdout) if the version is wrong. Recommendation: use the same format (and stderr) for both. 12) Similarly, other Java checks later in the script produce messages to stdout, not stderr. 13) Drill-config.sh searches $JAVA_HOME to find java/java.exe and verifies that it is executable. The script then throws away what we just found. Then, drill-bit.sh tries to recreate this information as: JAVA=$JAVA_HOME/bin/java This is wrong in two ways: 1) it ignores the actual java location and assumes it, and 2) it does not handle the java.exe case that drill-config.sh carefully worked out. Recommendation: export JAVA from drill-config.sh and remove the above line from drillbit.sh. 14) drillbit.sh presumably takes extra arguments like this: drillbit.sh -Dvar0=value0 --config /my/conf/dir start -Dvar1=value1 -Dvar2=value2 -Dvar3=value3 The -D bit allows the user to override config variables at the command line. But, the scripts don't use the values. A) drill-config.sh consumes --config /my/conf/dir after consuming the leading arguments: while [ $# -gt 1 ]; do if [ "--config" = "$1" ]; then shift confdir=$1 shift DRILL_CONF_DIR=$confdir else # Presume we are at end of options and break break fi done B) drill-bit.sh will discard the var1: startStopStatus=$1 <-- grabs "start" shift command=drillbit shift <-- Consumes -Dvar1=value1 C) Remaining values passed back into drillbit.sh: args=$@ nohup $thiscmd internal_start $command $args D) Second invocation discards -Dvar2=value2 as described above. E) Remaining values are passed to runbit: "$DRILL_HOME"/bin/runbit $command "$@" start F) Where they again pass though drill-config. (Allowing us to do: drillbit.sh --config /first/conf --config /second/conf which is asking for trouble) G) And, the remaining arguments are simply not used: exec $JAVA -Dlog.path=$DRILLBIT_LOG_PATH -Dlog.query.path=$DRILLBIT_QUERY_LOG_PATH $DRILL_ALL_JAVA_OPTS -cp $CP org.apache.drill.exec.server.Drillbit 15) The checking of command-line args in drillbit.sh is wrong: # if no args specified, show usage if [ $# -lt 1 ]; then echo $usage exit 1 fi ... . "$bin"/drill-config.sh But, note, that drill-config.sh handles: drillbit.sh --config /conf/dir Consuming
[jira] [Updated] (DRILL-4581) Various problems in the Drill startup scripts
[ https://issues.apache.org/jira/browse/DRILL-4581?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Rogers updated DRILL-4581: --- Description: Noticed the following in drillbit.sh: 1) Comment: DRILL_LOG_DIRWhere log files are stored. PWD by default. Code: DRILL_LOG_DIR=/var/log/drill or, if it does not exist, $DRILL_HOME/log 2) Comment: DRILL_PID_DIRThe pid files are stored. /tmp by default. Code: DRILL_PID_DIR=$DRILL_HOME 3) Redundant checking of JAVA_HOME. drillbit.sh sources drill-config.sh which checks JAVA_HOME. Later, drillbit.sh checks it again. The second check is both unnecessary and prints a less informative message than the drill-config.sh check. Suggestion: Remove the JAVA_HOME check in drillbit.sh. 4) Though drill-config.sh carefully checks JAVA_HOME, it does not export the JAVA_HOME variable. Perhaps this is why drillbit.sh repeats the check? Recommended: export JAVA_HOME from drill-config.sh. 5) Both drillbit.sh and the sourced drill-config.sh check DRILL_LOG_DIR and set the default value. Drill-config.sh defaults to /var/log/drill, or if that fails, to $DRILL_HOME/log. Drillbit.sh just sets /var/log/drill and does not handle the case where that directory is not writable. Suggested: remove the check in drillbit.sh. 6) Drill-config.sh checks the writability of the DRILL_LOG_DIR by touching sqlline.log, but does not delete that file, leaving a bogus, empty client log file on the drillbit server. Recommendation: use bash commands instead. 7) The implementation of the above check is a bit awkward. It has a fallback case with somewhat awkward logic. Clean this up. 8) drillbit.sh, but not drill-config.sh, attempts to create /var/log/drill if it does not exist. Recommended: decide on a single choice, implement it in drill-config.sh. 9) drill-config.sh checks if $DRILL_CONF_DIR is a directory. If not, defaults it to $DRILL_HOME/conf. This can lead to subtle errors. If I use drillbit.sh --config /misspelled/path where I mistype the path, I won't get an error, I get the default config, which may not at all be what I want to run. Recommendation: if the value of DRILL_CONF_DRILL is passed into the script (as a variable or via --config), then that directory must exist. Else, use the default. 10) drill-config.sh exports, but may not set, HADOOP_HOME. This may be left over from the original Hadoop script that the Drill script was based upon. Recomendation: export only in the case that HADOOP_HOME is set for cygwin. 11) Drill-config.sh checks JAVA_HOME and prints a big, bold error message to stderr if JAVA_HOME is not set. Then, it checks the Java version and prints a different message (to stdout) if the version is wrong. Recommendation: use the same format (and stderr) for both. 12) Similarly, other Java checks later in the script produce messages to stdout, not stderr. 13) Drill-config.sh searches $JAVA_HOME to find java/java.exe and verifies that it is executable. The script then throws away what we just found. Then, drill-bit.sh tries to recreate this information as: JAVA=$JAVA_HOME/bin/java This is wrong in two ways: 1) it ignores the actual java location and assumes it, and 2) it does not handle the java.exe case that drill-config.sh carefully worked out. Recommendation: export JAVA from drill-config.sh and remove the above line from drillbit.sh. 14) drillbit.sh presumably takes extra arguments like this: drillbit.sh -Dvar0=value0 --config /my/conf/dir start -Dvar1=value1 -Dvar2=value2 -Dvar3=value3 The -D bit allows the user to override config variables at the command line. But, the scripts don't use the values. A) drill-config.sh consumes --config /my/conf/dir after consuming the leading arguments: while [ $# -gt 1 ]; do if [ "--config" = "$1" ]; then shift confdir=$1 shift DRILL_CONF_DIR=$confdir else # Presume we are at end of options and break break fi done B) drill-bit.sh will discard the var1: startStopStatus=$1 <-- grabs "start" shift command=drillbit shift <-- Consumes -Dvar1=value1 C) Remaining values passed back into drillbit.sh: args=$@ nohup $thiscmd internal_start $command $args D) Second invocation discards -Dvar2=value2 as described above. E) Remaining values are passed to runbit: "$DRILL_HOME"/bin/runbit $command "$@" start F) Where they again pass though drill-config. (Allowing us to do: drillbit.sh --config /first/conf --config /second/conf which is asking for trouble) G) And, the remaining arguments are simply not used: exec $JAVA -Dlog.path=$DRILLBIT_LOG_PATH -Dlog.query.path=$DRILLBIT_QUERY_LOG_PATH $DRILL_ALL_JAVA_OPTS -cp $CP org.apache.drill.exec.server.Drillbit 15) The checking of command-line args in drillbit.sh is wrong: # if no args specified, show usage if [ $# -lt 1 ]; then echo $usage exit 1 fi ... . "$bin"/drill-config.sh But, note, that drill-config.sh handles: drillbit.sh --config /conf/dir Consuming
[jira] [Updated] (DRILL-4581) Various problems in the Drill startup scripts
[ https://issues.apache.org/jira/browse/DRILL-4581?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Rogers updated DRILL-4581: --- Description: Noticed the following in drillbit.sh: 1) Comment: DRILL_LOG_DIRWhere log files are stored. PWD by default. Code: DRILL_LOG_DIR=/var/log/drill or, if it does not exist, $DRILL_HOME/log 2) Comment: DRILL_PID_DIRThe pid files are stored. /tmp by default. Code: DRILL_PID_DIR=$DRILL_HOME 3) Redundant checking of JAVA_HOME. drillbit.sh sources drill-config.sh which checks JAVA_HOME. Later, drillbit.sh checks it again. The second check is both unnecessary and prints a less informative message than the drill-config.sh check. Suggestion: Remove the JAVA_HOME check in drillbit.sh. 4) Though drill-config.sh carefully checks JAVA_HOME, it does not export the JAVA_HOME variable. Perhaps this is why drillbit.sh repeats the check? Recommended: export JAVA_HOME from drill-config.sh. 5) Both drillbit.sh and the sourced drill-config.sh check DRILL_LOG_DIR and set the default value. Drill-config.sh defaults to /var/log/drill, or if that fails, to $DRILL_HOME/log. Drillbit.sh just sets /var/log/drill and does not handle the case where that directory is not writable. Suggested: remove the check in drillbit.sh. 6) Drill-config.sh checks the writability of the DRILL_LOG_DIR by touching sqlline.log, but does not delete that file, leaving a bogus, empty client log file on the drillbit server. Recommendation: use bash commands instead. 7) The implementation of the above check is a bit awkward. It has a fallback case with somewhat awkward logic. Clean this up. 8) drillbit.sh, but not drill-config.sh, attempts to create /var/log/drill if it does not exist. Recommended: decide on a single choice, implement it in drill-config.sh. 9) drill-config.sh checks if $DRILL_CONF_DIR is a directory. If not, defaults it to $DRILL_HOME/conf. This can lead to subtle errors. If I use drillbit.sh --config /misspelled/path where I mistype the path, I won't get an error, I get the default config, which may not at all be what I want to run. Recommendation: if the value of DRILL_CONF_DRILL is passed into the script (as a variable or via --config), then that directory must exist. Else, use the default. 10) drill-config.sh exports, but may not set, HADOOP_HOME. This may be left over from the original Hadoop script that the Drill script was based upon. Recomendation: export only in the case that HADOOP_HOME is set for cygwin. 11) Drill-config.sh checks JAVA_HOME and prints a big, bold error message to stderr if JAVA_HOME is not set. Then, it checks the Java version and prints a different message (to stdout) if the version is wrong. Recommendation: use the same format (and stderr) for both. 12) Similarly, other Java checks later in the script produce messages to stdout, not stderr. 13) Drill-config.sh searches $JAVA_HOME to find java/java.exe and verifies that it is executable. The script then throws away what we just found. Then, drill-bit.sh tries to recreate this information as: JAVA=$JAVA_HOME/bin/java This is wrong in two ways: 1) it ignores the actual java location and assumes it, and 2) it does not handle the java.exe case that drill-config.sh carefully worked out. Recommendation: export JAVA from drill-config.sh and remove the above line from drillbit.sh. 14) drillbit.sh presumably takes extra arguments like this: drillbit.sh -Dvar0=value0 --config /my/conf/dir start -Dvar1=value1 -Dvar2=value2 -Dvar3=value3 The -D bit allows the user to override config variables at the command line. But, the scripts don't use the values. A) drill-config.sh consumes --config /my/conf/dir after consuming the leading arguments: while [ $# -gt 1 ]; do if [ "--config" = "$1" ]; then shift confdir=$1 shift DRILL_CONF_DIR=$confdir else # Presume we are at end of options and break break fi done B) drill-bit.sh will discard the var1: startStopStatus=$1 <-- grabs "start" shift command=drillbit shift <-- Consumes -Dvar1=value1 C) Remaining values passed back into drillbit.sh: args=$@ nohup $thiscmd internal_start $command $args D) Second invocation discards -Dvar2=value2 as described above. E) Remaining values are passed to runbit: "$DRILL_HOME"/bin/runbit $command "$@" start F) Where they again pass though drill-config. (Allowing us to do: drillbit.sh --config /first/conf --config /second/conf which is asking for trouble) G) And, the remaining arguments are simply not used: exec $JAVA -Dlog.path=$DRILLBIT_LOG_PATH -Dlog.query.path=$DRILLBIT_QUERY_LOG_PATH $DRILL_ALL_JAVA_OPTS -cp $CP org.apache.drill.exec.server.Drillbit 15) The checking of command-line args in drillbit.sh is wrong: # if no args specified, show usage if [ $# -lt 1 ]; then echo $usage exit 1 fi ... . "$bin"/drill-config.sh But, note, that drill-config.sh handles: drillbit.sh --config /conf/dir Consuming
[jira] [Updated] (DRILL-4581) Various problems in the Drill startup scripts
[ https://issues.apache.org/jira/browse/DRILL-4581?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Rogers updated DRILL-4581: --- Description: Noticed the following in drillbit.sh: 1) Comment: DRILL_LOG_DIRWhere log files are stored. PWD by default. Code: DRILL_LOG_DIR=/var/log/drill or, if it does not exist, $DRILL_HOME/log 2) Comment: DRILL_PID_DIRThe pid files are stored. /tmp by default. Code: DRILL_PID_DIR=$DRILL_HOME 3) Redundant checking of JAVA_HOME. drillbit.sh sources drill-config.sh which checks JAVA_HOME. Later, drillbit.sh checks it again. The second check is both unnecessary and prints a less informative message than the drill-config.sh check. Suggestion: Remove the JAVA_HOME check in drillbit.sh. 4) Though drill-config.sh carefully checks JAVA_HOME, it does not export the JAVA_HOME variable. Perhaps this is why drillbit.sh repeats the check? Recommended: export JAVA_HOME from drill-config.sh. 5) Both drillbit.sh and the sourced drill-config.sh check DRILL_LOG_DIR and set the default value. Drill-config.sh defaults to /var/log/drill, or if that fails, to $DRILL_HOME/log. Drillbit.sh just sets /var/log/drill and does not handle the case where that directory is not writable. Suggested: remove the check in drillbit.sh. 6) Drill-config.sh checks the writability of the DRILL_LOG_DIR by touching sqlline.log, but does not delete that file, leaving a bogus, empty client log file on the drillbit server. Recommendation: use bash commands instead. 7) The implementation of the above check is a bit awkward. It has a fallback case with somewhat awkward logic. Clean this up. 8) drillbit.sh, but not drill-config.sh, attempts to create /var/log/drill if it does not exist. Recommended: decide on a single choice, implement it in drill-config.sh. 9) drill-config.sh checks if $DRILL_CONF_DIR is a directory. If not, defaults it to $DRILL_HOME/conf. This can lead to subtle errors. If I use drillbit.sh --config /misspelled/path where I mistype the path, I won't get an error, I get the default config, which may not at all be what I want to run. Recommendation: if the value of DRILL_CONF_DRILL is passed into the script (as a variable or via --config), then that directory must exist. Else, use the default. 10) drill-config.sh exports, but may not set, HADOOP_HOME. This may be left over from the original Hadoop script that the Drill script was based upon. Recomendation: export only in the case that HADOOP_HOME is set for cygwin. 11) Drill-config.sh checks JAVA_HOME and prints a big, bold error message to stderr if JAVA_HOME is not set. Then, it checks the Java version and prints a different message (to stdout) if the version is wrong. Recommendation: use the same format (and stderr) for both. 12) Similarly, other Java checks later in the script produce messages to stdout, not stderr. 13) Drill-config.sh searches $JAVA_HOME to find java/java.exe and verifies that it is executable. The script then throws away what we just found. Then, drill-bit.sh tries to recreate this information as: JAVA=$JAVA_HOME/bin/java This is wrong in two ways: 1) it ignores the actual java location and assumes it, and 2) it does not handle the java.exe case that drill-config.sh carefully worked out. Recommendation: export JAVA from drill-config.sh and remove the above line from drillbit.sh. 14) drillbit.sh presumably takes extra arguments like this: drillbit.sh -Dvar0=value0 --config /my/conf/dir start -Dvar1=value1 -Dvar2=value2 -Dvar3=value3 The -D bit allows the user to override config variables at the command line. But, the scripts don't use the values. A) drill-config.sh consumes --config /my/conf/dir after consuming the leading arguments: while [ $# -gt 1 ]; do if [ "--config" = "$1" ]; then shift confdir=$1 shift DRILL_CONF_DIR=$confdir else # Presume we are at end of options and break break fi done B) drill-bit.sh will discard the var1: startStopStatus=$1 <-- grabs "start" shift command=drillbit shift <-- Consumes -Dvar1=value1 C) Remaining values passed back into drillbit.sh: args=$@ nohup $thiscmd internal_start $command $args D) Second invocation discards -Dvar2=value2 as described above. E) Remaining values are passed to runbit: "$DRILL_HOME"/bin/runbit $command "$@" start F) Where they again pass though drill-config. (Allowing us to do: drillbit.sh --config /first/conf --config /second/conf which is asking for trouble) G) And, the remaining arguments are simply not used: exec $JAVA -Dlog.path=$DRILLBIT_LOG_PATH -Dlog.query.path=$DRILLBIT_QUERY_LOG_PATH $DRILL_ALL_JAVA_OPTS -cp $CP org.apache.drill.exec.server.Drillbit 15) The checking of command-line args in drillbit.sh is wrong: # if no args specified, show usage if [ $# -lt 1 ]; then echo $usage exit 1 fi ... . "$bin"/drill-config.sh But, note, that drill-config.sh handles: drillbit.sh --config /conf/dir Consuming
[jira] [Updated] (DRILL-4581) Various problems in the Drill startup scripts
[ https://issues.apache.org/jira/browse/DRILL-4581?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Rogers updated DRILL-4581: --- Summary: Various problems in the Drill startup scripts (was: Various inconsistencies in the Drill startup scripts) > Various problems in the Drill startup scripts > - > > Key: DRILL-4581 > URL: https://issues.apache.org/jira/browse/DRILL-4581 > Project: Apache Drill > Issue Type: Bug > Components: Server >Affects Versions: 1.6.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > > Noticed the following in drillbit.sh: > 1) Comment: DRILL_LOG_DIRWhere log files are stored. PWD by default. > Code: DRILL_LOG_DIR=/var/log/drill or, if it does not exist, $DRILL_HOME/log > 2) Comment: DRILL_PID_DIRThe pid files are stored. /tmp by default. > Code: DRILL_PID_DIR=$DRILL_HOME > 3) Redundant checking of JAVA_HOME. drillbit.sh sources drill-config.sh which > checks JAVA_HOME. Later, drillbit.sh checks it again. The second check is > both unnecessary and prints a less informative message than the > drill-config.sh check. Suggestion: Remove the JAVA_HOME check in drillbit.sh. > 4) Though drill-config.sh carefully checks JAVA_HOME, it does not export the > JAVA_HOME variable. Perhaps this is why drillbit.sh repeats the check? > Recommended: export JAVA_HOME from drill-config.sh. > 5) Both drillbit.sh and the sourced drill-config.sh check DRILL_LOG_DIR and > set the default value. Drill-config.sh defaults to /var/log/drill, or if that > fails, to $DRILL_HOME/log. Drillbit.sh just sets /var/log/drill and does not > handle the case where that directory is not writable. Suggested: remove the > check in drillbit.sh. > 6) Drill-config.sh checks the writability of the DRILL_LOG_DIR by touching > sqlline.log, but does not delete that file, leaving a bogus, empty client log > file on the drillbit server. Recommendation: use bash commands instead. > 7) The implementation of the above check is a bit awkward. It has a fallback > case with somewhat awkward logic. Clean this up. > 8) drillbit.sh, but not drill-config.sh, attempts to create /var/log/drill if > it does not exist. Recommended: decide on a single choice, implement it in > drill-config.sh. > 9) drill-config.sh checks if $DRILL_CONF_DIR is a directory. If not, defaults > it to $DRILL_HOME/conf. This can lead to subtle errors. If I use > drillbit.sh --config /misspelled/path > where I mistype the path, I won't get an error, I get the default config, > which may not at all be what I want to run. Recommendation: if the value of > DRILL_CONF_DRILL is passed into the script (as a variable or via --config), > then that directory must exist. Else, use the default. > 10) drill-config.sh exports, but may not set, HADOOP_HOME. This may be left > over from the original Hadoop script that the Drill script was based upon. > Recomendation: export only in the case that HADOOP_HOME is set for cygwin. > 11) Drill-config.sh checks JAVA_HOME and prints a big, bold error message to > stderr if JAVA_HOME is not set. Then, it checks the Java version and prints a > different message (to stdout) if the version is wrong. Recommendation: use > the same format (and stderr) for both. > 12) Similarly, other Java checks later in the script produce messages to > stdout, not stderr. > 13) Drill-config.sh searches $JAVA_HOME to find java/java.exe and verifies > that it is executable. The script then throws away what we just found. Then, > drill-bit.sh tries to recreate this information as: > JAVA=$JAVA_HOME/bin/java > This is wrong in two ways: 1) it ignores the actual java location and assumes > it, and 2) it does not handle the java.exe case that drill-config.sh > carefully worked out. > Recommendation: export JAVA from drill-config.sh and remove the above line > from drillbit.sh. > 14) drillbit.sh presumably takes extra arguments like this: > drillbit.sh -Dvar0=value0 --config /my/conf/dir start -Dvar1=value1 > -Dvar2=value2 -Dvar3=value3 > The -D bit allows the user to override config variables at the command line. > But, the scripts don't use the values. > A) drill-config.sh consumes --config /my/conf/dir after consuming the leading > arguments: > while [ $# -gt 1 ]; do > if [ "--config" = "$1" ]; then > shift > confdir=$1 > shift > DRILL_CONF_DIR=$confdir > else > # Presume we are at end of options and break > break > fi > done > B) drill-bit.sh will discard the var1: > startStopStatus=$1 <-- grabs "start" > shift > command=drillbit > shift <-- Consumes -Dvar1=value1 > C) Remaining values passed back into drillbit.sh: > args=$@ > nohup $thiscmd internal_start $command $args > D) Second invocation discards -Dvar2=value2 as described above. > E) Remaining values are passed to
