Skip to content

Commit 26c1b95

Browse files
gengliangwangdongjoon-hyun
authored andcommitted
[SPARK-25711][CORE] Improve start-history-server.sh: show usage User-Friendly and remove deprecated options
## What changes were proposed in this pull request? Currently, if we try run ``` ./start-history-server.sh -h ``` We will get such error ``` java.io.FileNotFoundException: File -h does not exist ``` 1. This is not User-Friendly. For option `-h` or `--help`, it should be parsed correctly and show the usage of the class/script. 2. We can remove deprecated options for setting event log directory through command line options. After fix, we can get following output: ``` Usage: ./sbin/start-history-server.sh [options] Options: --properties-file FILE Path to a custom Spark properties file. Default is conf/spark-defaults.conf. Configuration options can be set by setting the corresponding JVM system property. History Server options are always available; additional options depend on the provider. History Server options: spark.history.ui.port Port where server will listen for connections (default 18080) spark.history.acls.enable Whether to enable view acls for all applications (default false) spark.history.provider Name of history provider class (defaults to file system-based provider) spark.history.retainedApplications Max number of application UIs to keep loaded in memory (default 50) FsHistoryProvider options: spark.history.fs.logDirectory Directory where app logs are stored (default: file:/tmp/spark-events) spark.history.fs.updateInterval How often to reload log data from storage (in seconds, default: 10) ``` ## How was this patch tested? Manual test Closes #22699 from gengliangwang/refactorSHSUsage. Authored-by: Gengliang Wang <gengliang.wang@databricks.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
1 parent 2eaf058 commit 26c1b95

3 files changed

Lines changed: 25 additions & 38 deletions

File tree

core/src/main/scala/org/apache/spark/deploy/history/HistoryServerArguments.scala

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -34,35 +34,21 @@ private[history] class HistoryServerArguments(conf: SparkConf, args: Array[Strin
3434

3535
@tailrec
3636
private def parse(args: List[String]): Unit = {
37-
if (args.length == 1) {
38-
setLogDirectory(args.head)
39-
} else {
40-
args match {
41-
case ("--dir" | "-d") :: value :: tail =>
42-
setLogDirectory(value)
43-
parse(tail)
37+
args match {
38+
case ("--help" | "-h") :: tail =>
39+
printUsageAndExit(0)
4440

45-
case ("--help" | "-h") :: tail =>
46-
printUsageAndExit(0)
41+
case ("--properties-file") :: value :: tail =>
42+
propertiesFile = value
43+
parse(tail)
4744

48-
case ("--properties-file") :: value :: tail =>
49-
propertiesFile = value
50-
parse(tail)
45+
case Nil =>
5146

52-
case Nil =>
53-
54-
case _ =>
55-
printUsageAndExit(1)
56-
}
47+
case _ =>
48+
printUsageAndExit(1)
5749
}
5850
}
5951

60-
private def setLogDirectory(value: String): Unit = {
61-
logWarning("Setting log directory through the command line is deprecated as of " +
62-
"Spark 1.1.0. Please set this through spark.history.fs.logDirectory instead.")
63-
conf.set("spark.history.fs.logDirectory", value)
64-
}
65-
6652
// This mutates the SparkConf, so all accesses to it must be made after this line
6753
Utils.loadDefaultSparkProperties(conf, propertiesFile)
6854

@@ -73,8 +59,6 @@ private[history] class HistoryServerArguments(conf: SparkConf, args: Array[Strin
7359
|Usage: HistoryServer [options]
7460
|
7561
|Options:
76-
| DIR Deprecated; set spark.history.fs.logDirectory directly
77-
| --dir DIR (-d DIR) Deprecated; set spark.history.fs.logDirectory directly
7862
| --properties-file FILE Path to a custom Spark properties file.
7963
| Default is conf/spark-defaults.conf.
8064
|

core/src/test/scala/org/apache/spark/deploy/history/HistoryServerArgumentsSuite.scala

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,6 @@ class HistoryServerArgumentsSuite extends SparkFunSuite {
4040
assert(conf.get("spark.testing") === "true")
4141
}
4242

43-
test("Directory Arguments Parsing --dir or -d") {
44-
val argStrings = Array("--dir", "src/test/resources/spark-events1")
45-
val hsa = new HistoryServerArguments(conf, argStrings)
46-
assert(conf.get("spark.history.fs.logDirectory") === "src/test/resources/spark-events1")
47-
}
48-
49-
test("Directory Param can also be set directly") {
50-
val argStrings = Array("src/test/resources/spark-events2")
51-
val hsa = new HistoryServerArguments(conf, argStrings)
52-
assert(conf.get("spark.history.fs.logDirectory") === "src/test/resources/spark-events2")
53-
}
54-
5543
test("Properties File Arguments Parsing --properties-file") {
5644
val tmpDir = Utils.createTempDir()
5745
val outFile = File.createTempFile("test-load-spark-properties", "test", tmpDir)

sbin/start-history-server.sh

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,22 @@ if [ -z "${SPARK_HOME}" ]; then
2828
export SPARK_HOME="$(cd "`dirname "$0"`"/..; pwd)"
2929
fi
3030

31+
# NOTE: This exact class name is matched downstream by SparkSubmit.
32+
# Any changes need to be reflected there.
33+
CLASS="org.apache.spark.deploy.history.HistoryServer"
34+
35+
if [[ "$@" = *--help ]] || [[ "$@" = *-h ]]; then
36+
echo "Usage: ./sbin/start-history-server.sh [options]"
37+
pattern="Usage:"
38+
pattern+="\|Using Spark's default log4j profile:"
39+
pattern+="\|Started daemon with process name"
40+
pattern+="\|Registered signal handler for"
41+
42+
"${SPARK_HOME}"/bin/spark-class $CLASS --help 2>&1 | grep -v "$pattern" 1>&2
43+
exit 1
44+
fi
45+
3146
. "${SPARK_HOME}/sbin/spark-config.sh"
3247
. "${SPARK_HOME}/bin/load-spark-env.sh"
3348

34-
exec "${SPARK_HOME}/sbin"/spark-daemon.sh start org.apache.spark.deploy.history.HistoryServer 1 "$@"
49+
exec "${SPARK_HOME}/sbin"/spark-daemon.sh start $CLASS 1 "$@"

0 commit comments

Comments
 (0)