Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions bin/spark-sql
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
set -o posix

CLASS="org.apache.spark.sql.hive.thriftserver.SparkSQLCLIDriver"
CLASS_NOT_FOUND_EXIT_STATUS=101

# Figure out where Spark is installed
FWDIR="$(cd "`dirname "$0"`"/..; pwd)"
Expand Down Expand Up @@ -53,13 +52,4 @@ source "$FWDIR"/bin/utils.sh
SUBMIT_USAGE_FUNCTION=usage
gatherSparkSubmitOpts "$@"

"$FWDIR"/bin/spark-submit --class $CLASS "${SUBMISSION_OPTS[@]}" spark-internal "${APPLICATION_OPTS[@]}"
exit_status=$?

if [[ exit_status -eq CLASS_NOT_FOUND_EXIT_STATUS ]]; then
echo
echo "Failed to load Spark SQL CLI main class $CLASS."
echo "You need to build Spark with -Phive."
fi

exit $exit_status
exec "$FWDIR"/bin/spark-submit --class $CLASS "${SUBMISSION_OPTS[@]}" spark-internal "${APPLICATION_OPTS[@]}"
4 changes: 4 additions & 0 deletions core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,10 @@ object SparkSubmit {
} catch {
case e: ClassNotFoundException =>
e.printStackTrace(printStream)
if (childMainClass.contains("thriftserver")) {
println(s"Failed to load main class $childMainClass.")
println("You need to build Spark with -Phive.")
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel complicated to add this logic here... SparkSubmit should not be coupled with Thrift server, but it seems that this is the only legitimate place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some thoughts, I think it's OK to add this checking here. Mainly because it avoids using the exit code checking trick, which may cause potential exit code conflict.

System.exit(CLASS_NOT_FOUND_EXIT_STATUS)
}

Expand Down
16 changes: 11 additions & 5 deletions sbin/spark-daemon.sh
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ then
shift
fi

startStop=$1
option=$1
shift
command=$1
shift
Expand Down Expand Up @@ -122,9 +122,9 @@ if [ "$SPARK_NICENESS" = "" ]; then
fi


case $startStop in
case $option in

(start)
(start|spark-submit)

mkdir -p "$SPARK_PID_DIR"

Expand All @@ -142,8 +142,14 @@ case $startStop in

spark_rotate_log "$log"
echo starting $command, logging to $log
cd "$SPARK_PREFIX"
nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-class $command "$@" >> "$log" 2>&1 < /dev/null &
if [ $option == spark-submit ]; then
source "$SPARK_HOME"/bin/utils.sh
gatherSparkSubmitOpts "$@"
nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-submit --class $command \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to call gatherSparkSubmitOpts "$@" before this line to generate SUBMISSION_OPTS and APPLICATION_OPTS. I don't think we should source spark-daemon.sh in start-thriftserver.sh because

  1. gatherSparkSubmitOpts is tightly coupled with spark-submit. In fact we should always call gatherSparkSubmitOpts before calling spark-submit. Put them together is more preferable.
  2. When adding another daemon component that uses spark-submit in the future, we don't want to duplicate the gatherSparkSubmitOpts call in the new start script.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you are right, but if we put gatherSparkSubmitOpts before spark-submit there comes a trouble that as gatherSparkSubmitOpts takes an argument SUBMIT_USAGE_FUNCTION which is different in those scripts who invoke gatherSparkSubmitOpts(now we have three: start-thriftserver.sh, spark-sql and pyspark).
Instead of passing different SUBMIT_USAGE_FUNCTION and sourcing bin/utils.sh inside of spark-daemon.sh, maybe it's better to assign values to SUBMISSION_OPTS and APPLICATION_OPTS outside of it.
There exists another option that we would not passing SUBMIT_USAGE_FUNCTION to gatherSparkSubmitOpts which leads to less user-friendliness.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, that's fair. I'd use export in sbin/start-thriftserver.sh to fix this issue (exported environment variables are accessible in bash subprocesses):

export SUBMIT_USAGE_FUNCTION=usage
exec "$FWDIR"/sbin/spark-daemon.sh spark-submit $CLASS 1

"${SUBMISSION_OPTS[@]}" spark-internal "${APPLICATION_OPTS[@]}" >> "$log" 2>&1 < /dev/null &
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think SUBMISSION_OPTS and APPLICATION_OPTS are properly defined here. Function gatherSparkSubmitOpts should be called here to define these two variables. You may refer to sbin/start-thriftserver.sh for details.

else
nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-class $command "$@" >> "$log" 2>&1 < /dev/null &
fi
newpid=$!
echo $newpid > $pid
sleep 2
Expand Down
16 changes: 2 additions & 14 deletions sbin/start-thriftserver.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ set -o posix
FWDIR="$(cd "`dirname "$0"`"/..; pwd)"

CLASS="org.apache.spark.sql.hive.thriftserver.HiveThriftServer2"
CLASS_NOT_FOUND_EXIT_STATUS=101

function usage {
echo "Usage: ./sbin/start-thriftserver [options] [thrift server options]"
Expand All @@ -49,17 +48,6 @@ if [[ "$@" = *--help ]] || [[ "$@" = *-h ]]; then
exit 0
fi

source "$FWDIR"/bin/utils.sh
SUBMIT_USAGE_FUNCTION=usage
gatherSparkSubmitOpts "$@"
export SUBMIT_USAGE_FUNCTION=usage

"$FWDIR"/bin/spark-submit --class $CLASS "${SUBMISSION_OPTS[@]}" spark-internal "${APPLICATION_OPTS[@]}"
exit_status=$?

if [[ exit_status -eq CLASS_NOT_FOUND_EXIT_STATUS ]]; then
echo
echo "Failed to load Hive Thrift server main class $CLASS."
echo "You need to build Spark with -Phive."
fi

exit $exit_status
exec "$FWDIR"/sbin/spark-daemon.sh spark-submit $CLASS 1 "$@"
25 changes: 25 additions & 0 deletions sbin/stop-thriftserver.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#!/usr/bin/env bash
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should be executable, please chmod +x.


#
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

# Stops the thrift server on the machine this script is executed on.

sbin="`dirname "$0"`"
sbin="`cd "$sbin"; pwd`"

"$sbin"/spark-daemon.sh stop org.apache.spark.sql.hive.thriftserver.HiveThriftServer2 1