Github user nchammas commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3916#discussion_r26081219
  
    --- Diff: sbin/spark-daemon.sh ---
    @@ -121,45 +121,63 @@ if [ "$SPARK_NICENESS" = "" ]; then
         export SPARK_NICENESS=0
     fi
     
    +run_command() {
    +  mode=$1
    +  shift
     
    -case $option in
    -
    -  (start|spark-submit)
    -
    -    mkdir -p "$SPARK_PID_DIR"
    +  mkdir -p "$SPARK_PID_DIR"
     
    -    if [ -f $pid ]; then
    -      TARGET_ID="$(cat "$pid")"
    -      if [[ $(ps -p "$TARGET_ID" -o args=) =~ $command ]]; then
    -        echo "$command running as process $TARGET_ID.  Stop it first."
    -        exit 1
    -      fi
    +  if [ -f $pid ]; then
    +    TARGET_ID="$(cat "$pid")"
    +    if [[ $(ps -p "$TARGET_ID" -o args=) =~ $command ]]; then
    +      echo "$command running as process $TARGET_ID.  Stop it first."
    +      exit 1
         fi
    +  fi
     
    -    if [ "$SPARK_MASTER" != "" ]; then
    -      echo rsync from "$SPARK_MASTER"
    -      rsync -a -e ssh --delete --exclude=.svn --exclude='logs/*' 
--exclude='contrib/hod/logs/*' $SPARK_MASTER/ "$SPARK_HOME"
    -    fi
    +  if [ "$SPARK_MASTER" != "" ]; then
    +    echo rsync from "$SPARK_MASTER"
    +    rsync -a -e ssh --delete --exclude=.svn --exclude='logs/*' 
--exclude='contrib/hod/logs/*' $SPARK_MASTER/ "$SPARK_HOME"
    +  fi
     
    -    spark_rotate_log "$log"
    -    echo "starting $command, logging to $log"
    -    if [ $option == spark-submit ]; then
    -      source "$SPARK_HOME"/bin/utils.sh
    -      gatherSparkSubmitOpts "$@"
    -      nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-submit 
--class $command \
    -        "${SUBMISSION_OPTS[@]}" spark-internal "${APPLICATION_OPTS[@]}" >> 
"$log" 2>&1 < /dev/null &
    -    else
    +  spark_rotate_log "$log"
    +  echo "starting $command, logging to $log"
    +
    +  case $mode in
    +    (class)
           nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-class 
$command "$@" >> "$log" 2>&1 < /dev/null &
    -    fi
    -    newpid=$!
    -    echo $newpid > $pid
    -    sleep 2
    -    # Check if the process has died; in that case we'll tail the log so 
the user can see
    -    if [[ ! $(ps -p "$newpid" -o args=) =~ $command ]]; then
    -      echo "failed to launch $command:"
    -      tail -2 "$log" | sed 's/^/  /'
    -      echo "full log in $log"
    -    fi
    +      newpid=$!
    +      ;;
    +
    +    (submit)
    +      nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-submit 
--class $command "$@" >> "$log" 2>&1 < /dev/null &
    --- End diff --
    
    For consistency, and out of paranoia, I suggest for this block:
    
    ```
    "$SPARK_NICENESS"
    "$!"
    "$newpid"
    "$pid"
    ```
    
    And so forth.
    
    I know most of these variables are never supposed to contain a space, but 
this counts as good Bash hygiene, as annoying as it is.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to