SPARK-2481: The environment variables SPARK_HISTORY_OPTS is covered in spark-env.sh

[witgo]

No description provided.

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[AmplabJenkins]

Merged build finished.

[AmplabJenkins]

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16460/

[witgo]

Jenkins, retest this please.

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[AmplabJenkins]

Merged build finished.

[AmplabJenkins]

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16463/

[vanzin]

Is there a specific issue you're trying to fix here? sbin/spark-daemon.sh, which is called from the scripts you mention, already load that file.

[witgo]

if [ $# != 0 ]; then
  echo "Using command line arguments for setting the log directory is deprecated. Please "
  echo "set the spark.history.fs.logDirectory configuration option instead."
  export SPARK_HISTORY_OPTS="$SPARK_HISTORY_OPTS -Dspark.history.fs.logDirectory=$1"
fi

exec "$sbin"/spark-daemon.sh start org.apache.spark.deploy.history.HistoryServer 1

Before export SPARK_HISTORY_OPTS="$SPARK_HISTORY_OPTS -Dspark.history.fs.logDirectory=$1" execution should load spark-env.sh

[vanzin]

I'd rather do it differently in that case, if you really care about it. Instead of loading spark-env.sh, do this:

CMD_LINE_ARGS=
if [ $# != 0 ]; then
  # print warning
  CMD_LINE_ARGS="--dir $1"
fi

exec spark-daemon.sh .... $CMD_LINE_ARGS

This restores the previous behaviour while still warning the user.

[vanzin]

Also, as a general comment, it's nice when the PR summary/description explain the problem being solved (not just what the patch is doing).

[witgo]

Use load-spark-env.sh to ensure spark-env.sh loads only once. start-master.sh did the same.

[vanzin]

LGTM.

[tsudukim]

Looks good, but this patch seems to includes some unrelated diffs to SPARK-2481.

• conf/spark-env.sh.template
• docs/spark-standalone.md
• sbin/spark-config.sh

[SparkQA]

QA tests have started for PR 1341. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16778/consoleFull

[SparkQA]

QA results for PR 1341:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16778/consoleFull

[witgo]

@pwendell I think the PR can be merged into 1.1

[SparkQA]

QA tests have started for PR 1341. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17796/consoleFull

[SparkQA]

QA results for PR 1341:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17796/consoleFull

[SparkQA]

QA tests have started for PR 1341. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17798/consoleFull

[SparkQA]

QA results for PR 1341:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17798/consoleFull

[andrewor14]

I believe you don't need to change this file, since we're not using any environment variables here. Also, spark-daemon.sh actually already does this.

[andrewor14]

@witgo pending 1 more comment this LGTM

[witgo]

@andrewor14 done

[andrewor14]

Thanks I merged this into master and 1.1