Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
29 changes: 7 additions & 22 deletions bin/beeline
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,14 @@
# limitations under the License.
#

# Figure out where Spark is installed
FWDIR="$(cd `dirname $0`/..; pwd)"
#
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes seem unrelated. Is there a bug you can mention? Otherwise, could you call them out explicitly in the PR description?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, you're right. I should update the PR description.

While working on #1620, the beeline script had once been implemented with spark-submit to avoid duplicated java check and computing classpath, but then reverted because of the issue this PR is trying to fix (beeline --help shows spark-submit usage message).

And while working on this PR, I realized that beeline is only a JDBC client, and unrelated to Spark, I can just start it with spark-class. That's the reason why this change appear here.

# Shell script for starting BeeLine

# Find the java binary
if [ -n "${JAVA_HOME}" ]; then
RUNNER="${JAVA_HOME}/bin/java"
else
if [ `command -v java` ]; then
RUNNER="java"
else
echo "JAVA_HOME is not set" >&2
exit 1
fi
fi
# Enter posix mode for bash
set -o posix

# Compute classpath using external script
classpath_output=$($FWDIR/bin/compute-classpath.sh)
if [[ "$?" != "0" ]]; then
echo "$classpath_output"
exit 1
else
CLASSPATH=$classpath_output
fi
# Figure out where Spark is installed
FWDIR="$(cd `dirname $0`/..; pwd)"

CLASS="org.apache.hive.beeline.BeeLine"
exec "$RUNNER" -cp "$CLASSPATH" $CLASS "$@"
exec "$FWDIR/bin/spark-class" $CLASS "$@"
13 changes: 9 additions & 4 deletions bin/spark-sql
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,16 @@ set -o posix
# Figure out where Spark is installed
FWDIR="$(cd `dirname $0`/..; pwd)"

if [[ "$@" = *--help ]] || [[ "$@" = *-h ]]; then
echo "Usage: ./sbin/spark-sql [options]"
$FWDIR/bin/spark-submit --help 2>&1 | grep -v Usage 1>&2
CLASS="org.apache.spark.sql.hive.thriftserver.SparkSQLCLIDriver"

if [[ "$@" = --help ]] || [[ "$@" = -h ]]; then
echo "Usage: ./sbin/spark-sql [options] [--] [CLI options]"
exec "$FWDIR"/bin/spark-submit --help 2>&1 | grep -v Usage 1>&2
Copy link
Contributor

Choose a reason for hiding this comment

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

exec here means anything past this line won't execute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually if redirection is used, exec does return and the following code gets executed (reference). And during my local test, it works as expected.

But indeed, I didn't notice this issue while adding this change. Should use exec more carefully. Thanks!

echo
echo "CLI options:"
exec "$FWDIR"/bin/spark-submit spark-internal --class $CLASS -- -H 2>&1 | tail -n +3
echo
exit 0
fi

CLASS="org.apache.spark.sql.hive.thriftserver.SparkSQLCLIDriver"
exec "$FWDIR"/bin/spark-submit --class $CLASS spark-internal $@
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,15 @@ private[spark] class SparkSubmitArguments(args: Seq[String]) {
verbose = true
parse(tail)

case "--" :: tail =>
if (inSparkOpts) {
SparkSubmit.printErrorAndExit(
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't you just omit this and just delegate to the existing check in checkRequiredArguments()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair, thanks.

"Application option separator \"--\" must be after the primary resource " +
"(i.e., application jar file or Python file).")
} else {
childArgs ++= tail.filter(_.nonEmpty)
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't filter. If a user has gone through the trouble of quoting things to explicitly include an empty argument, it should probably stay there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This behavior is actually inherited from the current master code. But I agree with you, since -- is a newly introduced argument passing mode, we should allow empty argument.

}

case value :: tail =>
if (inSparkOpts) {
value match {
Expand Down Expand Up @@ -377,8 +386,11 @@ private[spark] class SparkSubmitArguments(args: Seq[String]) {
|
| --executor-memory MEM Memory per executor (e.g. 1000M, 2G) (Default: 1G).
|
| --help, -h Show this help message and exit
| --verbose, -v Print additional debug output
| --help, -h Show this help message and exit.
| --verbose, -v Print additional debug output.
|
| -- A "--" signals the end of spark-submit options, all command
| line arguments after "--" are passed to the application.
|
| Spark standalone with cluster deploy mode only:
| --driver-cores NUM Cores for driver (Default: 1).
Expand Down
16 changes: 16 additions & 0 deletions core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,22 @@ class SparkSubmitSuite extends FunSuite with Matchers {
appArgs.childArgs should be (Seq("some", "--weird", "args"))
}

test("handles arguments with \"--\"") {
val clArgs =
"""--name myApp
|--class Foo
|userjar.jar
|--master local
|some
|--
|--weird args
""".stripMargin.split("\\s+").toSeq
val appArgs = new SparkSubmitArguments(clArgs)
appArgs.master should be ("local")
appArgs.mainClass should be ("Foo")
appArgs.childArgs should be (Seq("some", "--weird", "args"))
}

test("handles YARN cluster mode") {
val clArgs = Seq(
"--deploy-mode", "cluster",
Expand Down
13 changes: 9 additions & 4 deletions sbin/start-thriftserver.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,16 @@ set -o posix
# Figure out where Spark is installed
FWDIR="$(cd `dirname $0`/..; pwd)"

if [[ "$@" = *--help ]] || [[ "$@" = *-h ]]; then
echo "Usage: ./sbin/start-thriftserver [options]"
$FWDIR/bin/spark-submit --help 2>&1 | grep -v Usage 1>&2
CLASS="org.apache.spark.sql.hive.thriftserver.HiveThriftServer2"

if [[ "$@" = --help ]] || [[ "$@" = -h ]]; then
echo "Usage: ./sbin/start-thriftserver.sh [options] [--] [thrift server options]"
exec "$FWDIR"/bin/spark-submit --help 2>&1 | grep -v Usage 1>&2
Copy link
Contributor

Choose a reason for hiding this comment

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

Same deal with exec here.

echo
echo "Thrift server options:"
exec "$FWDIR"/bin/spark-submit spark-internal --class $CLASS -- -H 2>&1 | tail -n +3
echo
exit 0
fi

CLASS="org.apache.spark.sql.hive.thriftserver.HiveThriftServer2"
exec "$FWDIR"/bin/spark-submit --class $CLASS spark-internal $@
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ private[hive] object HiveThriftServer2 extends Logging {
val optionsProcessor = new ServerOptionsProcessor("HiveThriftServer2")

if (!optionsProcessor.process(args)) {
logger.warn("Error starting HiveThriftServer2 with given arguments")
System.exit(-1)
}

Expand Down