You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@drill.apache.org by "Paul Rogers (JIRA)" <ji...@apache.org> on 2016/04/06 00:48:25 UTC

[jira] [Updated] (DRILL-4581) Various inconsistencies 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_DIR    Where log files are stored.  PWD by default.
Code: DRILL_LOG_DIR=/var/log/drill

2) Comment: DRILL_PID_DIR    The 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.

7) The implementation of the above check is a bit awkward. Can just use the -w shell check instead of creating a dummy file with the fallback flag.

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 never sets, 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 those two arguments will leave no command argument. Thus, the no-argument check should be done AFTER consuming --config.

  was:
Noticed the following in drillbit.sh:

1) Comment: DRILL_LOG_DIR    Where log files are stored.  PWD by default.
Code: DRILL_LOG_DIR=/var/log/drill

2) Comment: DRILL_PID_DIR    The 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.

7) The implementation of the above check is a bit awkward. Can just use the -w shell check instead of creating a dummy file with the fallback flag.

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 never sets, 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


> Various inconsistencies 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
>            Priority: Minor
>
> Noticed the following in drillbit.sh:
> 1) Comment: DRILL_LOG_DIR    Where log files are stored.  PWD by default.
> Code: DRILL_LOG_DIR=/var/log/drill
> 2) Comment: DRILL_PID_DIR    The 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.
> 7) The implementation of the above check is a bit awkward. Can just use the -w shell check instead of creating a dummy file with the fallback flag.
> 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 never sets, 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 those two arguments will leave no command argument. Thus, the no-argument check should be done AFTER consuming --config.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)