[SPARK-25711][Core] Improve start-history-server.sh: show usage User-Friendly and remove deprecated options #22699

gengliangwang · 2018-10-11T15:36:09Z

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

This is not User-Friendly. For option -h or --help, it should be parsed correctly and show the usage of the class/script.
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

jiangxb1987 · 2018-10-11T15:52:27Z

sbin/start-history-server.sh

+CLASS="org.apache.spark.deploy.history.HistoryServer"
+
+if [[ "$@" = *--help ]] || [[ "$@" = *-h ]]; then
+  echo "Usage: ./sbin/start-history-server.sh [options]"


nit: why not have a separated usage() function?

Well this is short, and I am following what start-master.sh and start-slave.sh did.

Well, I also saw similar code in start-thriftserver.sh, it uses usage(). Both are fine to me, just head up to make sure we've taken that into consideration.

jiangxb1987 · 2018-10-11T15:52:48Z

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

+        propertiesFile = value
+        parse(tail)
+
+      case dir :: Nil =>


IIUC this is not related to the PR description?

No, it is related. For the last single argument, it is treated as the event log directory.
See the deleted code

if (args.length == 1) { setLogDirectory(args.head) }

I prefer to keep the behavior.

sounds good.

I see. I have updated the description.

srowen

Looks OK but update the title. It's not clear that it's about showing a help message on the command line

dongjoon-hyun · 2018-10-11T17:42:37Z

BTW, @srowen , @jiangxb1987 , and @gengliangwang .

Spark 3.0 is a good chance to remove the deprecated options. Shall we remove the following? If we don't do this now, this can happen in Spark 4.0. (This is deprecated since Spark 1.1.0)

  DIR                         Deprecated; set spark.history.fs.logDirectory directly
  --dir DIR (-d DIR)          Deprecated; set spark.history.fs.logDirectory directly

srowen · 2018-10-11T17:47:08Z

Agree that's a good idea

gengliangwang · 2018-10-11T17:54:36Z

Agree. Let me remove them in this PR.

SparkQA · 2018-10-11T19:54:37Z

Test build #97267 has finished for PR 22699 at commit e91f611.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-10-11T21:51:14Z

Test build #97278 has finished for PR 22699 at commit 5e05c60.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-10-12T02:02:08Z

Test build #4373 has finished for PR 22699 at commit 5e05c60.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

jiangxb1987 · 2018-10-12T02:15:45Z

Let's also update the title to include the deprecation changes.

gengliangwang · 2018-10-12T02:58:40Z

retest this please.

SparkQA · 2018-10-12T06:42:22Z

Test build #97292 has finished for PR 22699 at commit 5e05c60.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2018-10-12T06:50:34Z

Retest this please.

SparkQA · 2018-10-12T07:05:02Z

Test build #97295 has finished for PR 22699 at commit 5e05c60.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2018-10-12T07:18:32Z

Retest this please.

SparkQA · 2018-10-12T10:48:48Z

Test build #97296 has finished for PR 22699 at commit 5e05c60.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

gengliangwang · 2018-10-12T15:03:28Z

retest this please.

SparkQA · 2018-10-12T18:18:45Z

Test build #97308 has finished for PR 22699 at commit 5e05c60.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-10-13T03:00:10Z

Test build #4375 has finished for PR 22699 at commit 5e05c60.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-10-13T07:05:02Z

Test build #97333 has finished for PR 22699 at commit 7fada77.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

gengliangwang · 2018-10-13T10:36:33Z

retest this please.

SparkQA · 2018-10-13T15:03:03Z

Test build #97338 has finished for PR 22699 at commit 7fada77.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun

+1, LGTM.

dongjoon-hyun · 2018-10-13T20:33:47Z

Merged to master. Thank you, @gengliangwang , @srowen , @jiangxb1987 .

…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 apache#22699 from gengliangwang/refactorSHSUsage. Authored-by: Gengliang Wang <gengliang.wang@databricks.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

Allow history server to show usage

e91f611

gengliangwang changed the title ~~[SPARK-25711][Core] Allow history server to show usage~~ [SPARK-25711][Core] Allow history server to show usage User-Friendly Oct 11, 2018

jiangxb1987 reviewed Oct 11, 2018

View reviewed changes

srowen approved these changes Oct 11, 2018

View reviewed changes

gengliangwang changed the title ~~[SPARK-25711][Core] Allow history server to show usage User-Friendly~~ [SPARK-25711][Core] Allow start-history-server.sh to show usage User-Friendly Oct 11, 2018

remove deprecated options

5e05c60

gengliangwang changed the title ~~[SPARK-25711][Core] Allow start-history-server.sh to show usage User-Friendly~~ [SPARK-25711][Core] Improve start-history-server.sh: show usage User-Friendly and remove deprecated options Oct 12, 2018

remove test cases

7fada77

srowen approved these changes Oct 13, 2018

View reviewed changes

dongjoon-hyun approved these changes Oct 13, 2018

View reviewed changes

asfgit closed this in 26c1b95 Oct 13, 2018

[SPARK-25711][Core] Improve start-history-server.sh: show usage User-Friendly and remove deprecated options #22699

[SPARK-25711][Core] Improve start-history-server.sh: show usage User-Friendly and remove deprecated options #22699

Uh oh!

Conversation

gengliangwang commented Oct 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

jiangxb1987 Oct 11, 2018

Choose a reason for hiding this comment

Uh oh!

gengliangwang Oct 11, 2018

Choose a reason for hiding this comment

Uh oh!

jiangxb1987 Oct 11, 2018

Choose a reason for hiding this comment

Uh oh!

jiangxb1987 Oct 11, 2018

Choose a reason for hiding this comment

Uh oh!

gengliangwang Oct 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiangxb1987 Oct 11, 2018

Choose a reason for hiding this comment

Uh oh!

gengliangwang Oct 11, 2018

Choose a reason for hiding this comment

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Oct 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srowen commented Oct 11, 2018

Uh oh!

gengliangwang commented Oct 11, 2018

Uh oh!

SparkQA commented Oct 11, 2018

Uh oh!

SparkQA commented Oct 11, 2018

Uh oh!

SparkQA commented Oct 12, 2018

Uh oh!

jiangxb1987 commented Oct 12, 2018

Uh oh!

gengliangwang commented Oct 12, 2018

Uh oh!

SparkQA commented Oct 12, 2018

Uh oh!

dongjoon-hyun commented Oct 12, 2018

Uh oh!

SparkQA commented Oct 12, 2018

Uh oh!

dongjoon-hyun commented Oct 12, 2018

Uh oh!

SparkQA commented Oct 12, 2018

Uh oh!

gengliangwang commented Oct 12, 2018

Uh oh!

SparkQA commented Oct 12, 2018

Uh oh!

SparkQA commented Oct 13, 2018

Uh oh!

SparkQA commented Oct 13, 2018

Uh oh!

gengliangwang commented Oct 13, 2018

Uh oh!

SparkQA commented Oct 13, 2018

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Oct 13, 2018

Uh oh!

Reviewers

Assignees

gengliangwang commented Oct 11, 2018 •

edited

Loading

gengliangwang Oct 11, 2018 •

edited

Loading

dongjoon-hyun commented Oct 11, 2018 •

edited

Loading