[SPARK-12463][SPARK-12464][SPARK-12465][SPARK-10647][MESOS] Fix zookeeper dir with mesos conf and add docs. #10057

tnachen · 2015-12-01T04:49:55Z

Fix zookeeper dir configuration used in cluster mode, and also add documentation around these settings.

tnachen · 2015-12-01T04:50:09Z

@andrewor14 @dragos @JoshRosen PTAL

SparkQA · 2015-12-01T06:40:00Z

Test build #46940 has finished for PR 10057 at commit 01cb559.

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

srowen · 2015-12-01T08:48:06Z

.../src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterPersistenceEngine.scala

@@ -94,7 +94,7 @@ private[spark] class ZookeeperMesosClusterPersistenceEngine(
    conf: SparkConf)
  extends MesosClusterPersistenceEngine with Logging {
  private val WORKING_DIR =
-    conf.get("spark.deploy.zookeeper.dir", "/spark_mesos_dispatcher") + "/" + baseDir
+    conf.get("spark.mesos.deploy.zookeeper.dir", "/spark_mesos_dispatcher") + "/" + baseDir


Up to you, but does it make sense to fall back to the original property as a default to preserve compatibility? Does that make sense for all these Mesos-specific override properties?

was this documented?

dragos · 2015-12-01T09:43:17Z

This looks fine as code changes, but any fixes in cluster mode are untestable for me. #9752 might fix it.

andrewor14 · 2015-12-01T21:18:26Z

Why not just reuse the existing spark.deploy.zookeeper.* configs?

JoshRosen · 2015-12-01T21:22:58Z

@andrewor14, one of the other configurations is already spark.mesos.deploy.zookeeper.url, so we'd have to change that one. I don't have super strong feelings either way.

tnachen · 2015-12-01T22:14:01Z

I think actually just using spark.deploy.* seems like a better choice, as in any case we don't really expect users to have different zookeepers deployed and cluster mode Mesos isn't used in conjunction with standalone mode too. I can update the code and the docs so it will be part of configuring Mesos doc as still.

andrewor14 · 2015-12-01T23:16:36Z

As long as we don't break backward compatibility anywhere then I think just reusing spark.deploy.zookeeper.* is simpler. We don't have to document it twice, and the user won't accidentally use the wrong one.

srowen · 2015-12-02T09:34:45Z

core/src/main/scala/org/apache/spark/deploy/mesos/MesosClusterDispatcher.scala

@@ -50,7 +50,7 @@ private[mesos] class MesosClusterDispatcher(
  extends Logging {

  private val publicAddress = Option(conf.getenv("SPARK_PUBLIC_DNS")).getOrElse(args.host)
-  private val recoveryMode = conf.get("spark.mesos.deploy.recoveryMode", "NONE").toUpperCase()
+  private val recoveryMode = conf.get("spark.deploy.recoveryMode", "NONE").toUpperCase()


Is the theory here that we can change the config param because it wasn't documented before? OK by me. Otherwise seems like the old value would have to be supported.

I'm still wondering about this as well. I think having some backward compatible makes sense at least for a version. Let me add a warning message when it's set and use the value if it's set as well.

SparkQA · 2015-12-10T02:21:01Z

Test build #47464 has finished for PR 10057 at commit 2ff38af.

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

SparkQA · 2015-12-10T17:08:48Z

Test build #47497 has finished for PR 10057 at commit b8fc74c.

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

andrewor14 · 2015-12-14T22:55:36Z

core/src/main/scala/org/apache/spark/deploy/mesos/MesosClusterDispatcher.scala

@@ -50,7 +50,11 @@ private[mesos] class MesosClusterDispatcher(
  extends Logging {

  private val publicAddress = Option(conf.getenv("SPARK_PUBLIC_DNS")).getOrElse(args.host)
-  private val recoveryMode = conf.get("spark.mesos.deploy.recoveryMode", "NONE").toUpperCase()
+  private val recoveryMode = conf.getOption("spark.mesos.deploy.recoverMode").map { mode =>
+    logWarning("spark.mesos.deploy.recoverMode is deprecated. Please configure " +


this was never documented so we don't need to add deprecation warning here.

by the way if you want to change the config name in this PR you should file a separate issue and add it to the title of this PR

I see, should we still honor the setting as a deprecation cycle? I'm going to just remove the warning for now but add a TODO.

I don't think it's even worth logging a warning. It wasn't documented so any user who was using the old config must have somehow found out about it from the code, knowing that it's not officially supported. I'd rather keep the code simpler than add a warning that I doubt will be useful for many.

andrewor14 · 2015-12-14T23:05:51Z

@tnachen can you help @dragos set up cluster mode so someone else other than you can test this patch?

tnachen · 2015-12-21T18:06:58Z

@andrewor14 the cluster mode issue is fixed now, @dragos @mgummelt we can use this patch through our tests

SparkQA · 2015-12-21T20:45:45Z

Test build #48121 has finished for PR 10057 at commit 2c29939.

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

andrewor14 · 2015-12-21T21:04:44Z

docs/spark-standalone.md

-  </tr>
-</table>
+In order to enable this recovery mode, you can set SPARK_DAEMON_JAVA_OPTS in spark-env by configuring `spark.deploy.recoveryMode` and related spark.deploy.zookeeper.* configurations.
+For more information about these configurations please refer to the configurations (doc)[configurations.html#deploy]


we should also briefly mention this in the running-on-mesos.md docs right?

SparkQA · 2015-12-22T05:37:41Z

Test build #48157 has finished for PR 10057 at commit 32a33ae.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * public final class LZ4BlockInputStream extends FilterInputStream\n * case class Range(\n * case class Range(\n

tnachen · 2015-12-22T07:04:20Z

Jenkins, retest this please

SparkQA · 2015-12-22T08:55:06Z

Test build #48170 has finished for PR 10057 at commit 4a56f6c.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * public final class LZ4BlockInputStream extends FilterInputStream\n * case class Range(\n * case class Range(\n

tnachen · 2016-01-04T19:38:27Z

@andrewor14 Can you take a look at this PR sometime this week?

andrewor14 · 2016-01-28T18:57:44Z

retest this please

SparkQA · 2016-01-28T20:30:04Z

Test build #50289 has finished for PR 10057 at commit 4a56f6c.

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

tnachen · 2016-01-29T02:16:41Z

retest this please

SparkQA · 2016-01-29T04:11:42Z

Test build #50333 has finished for PR 10057 at commit 5ebaf04.

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

tnachen · 2016-01-29T06:23:07Z

retest this please

SparkQA · 2016-01-29T08:29:45Z

Test build #50352 has finished for PR 10057 at commit 5ebaf04.

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

andrewor14 · 2016-02-01T20:44:40Z

Merged into master. By the way I might have mentioned this before but you probably don't need 3 different issues to rename 3 configs.

srowen reviewed Dec 1, 2015
View reviewed changes

tnachen force-pushed the fix_mesos_dir branch 2 times, most recently from 3cb36e8 to 2363ae8 Compare December 1, 2015 23:20

srowen reviewed Dec 2, 2015
View reviewed changes

tnachen force-pushed the fix_mesos_dir branch from 2363ae8 to 2ff38af Compare December 10, 2015 00:51

tnachen force-pushed the fix_mesos_dir branch from 2ff38af to b8fc74c Compare December 10, 2015 09:00

andrewor14 reviewed Dec 14, 2015
View reviewed changes

tnachen changed the title ~~[SPARK-10647][MESOS] Fix zookeeper dir with mesos conf and add docs.~~ [SPARK-12463][SPARK-12464][SPARK-12465][SPARK-10647][MESOS] Fix zookeeper dir with mesos conf and add docs. Dec 21, 2015

tnachen force-pushed the fix_mesos_dir branch from b8fc74c to 2c29939 Compare December 21, 2015 18:45

andrewor14 reviewed Dec 21, 2015
View reviewed changes

tnachen force-pushed the fix_mesos_dir branch from 2c29939 to 32a33ae Compare December 22, 2015 03:42

tnachen force-pushed the fix_mesos_dir branch from 32a33ae to 4a56f6c Compare December 22, 2015 06:46

tnachen added 2 commits January 28, 2016 18:15

Remove spark.mesos.deploy namespace for cluster mode conf.

ee1e56c

Address comments

5ebaf04

tnachen force-pushed the fix_mesos_dir branch from 4a56f6c to 5ebaf04 Compare January 29, 2016 02:28

asfgit closed this in 51b03b7 Feb 1, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-12463][SPARK-12464][SPARK-12465][SPARK-10647][MESOS] Fix zookeeper dir with mesos conf and add docs. #10057

[SPARK-12463][SPARK-12464][SPARK-12465][SPARK-10647][MESOS] Fix zookeeper dir with mesos conf and add docs. #10057

tnachen commented Dec 1, 2015

tnachen commented Dec 1, 2015

SparkQA commented Dec 1, 2015

srowen Dec 1, 2015

andrewor14 Dec 1, 2015

dragos commented Dec 1, 2015

andrewor14 commented Dec 1, 2015

JoshRosen commented Dec 1, 2015

tnachen commented Dec 1, 2015

andrewor14 commented Dec 1, 2015

srowen Dec 2, 2015

tnachen Dec 8, 2015

SparkQA commented Dec 10, 2015

SparkQA commented Dec 10, 2015

andrewor14 Dec 14, 2015

andrewor14 Dec 14, 2015

tnachen Dec 21, 2015

andrewor14 Dec 21, 2015

andrewor14 commented Dec 14, 2015

tnachen commented Dec 21, 2015

SparkQA commented Dec 21, 2015

andrewor14 Dec 21, 2015

SparkQA commented Dec 22, 2015

tnachen commented Dec 22, 2015

SparkQA commented Dec 22, 2015

tnachen commented Jan 4, 2016

andrewor14 commented Jan 28, 2016

SparkQA commented Jan 28, 2016

tnachen commented Jan 29, 2016

SparkQA commented Jan 29, 2016

tnachen commented Jan 29, 2016

SparkQA commented Jan 29, 2016

andrewor14 commented Feb 1, 2016

[SPARK-12463][SPARK-12464][SPARK-12465][SPARK-10647][MESOS] Fix zookeeper dir with mesos conf and add docs. #10057

[SPARK-12463][SPARK-12464][SPARK-12465][SPARK-10647][MESOS] Fix zookeeper dir with mesos conf and add docs. #10057

Conversation

tnachen commented Dec 1, 2015

tnachen commented Dec 1, 2015

SparkQA commented Dec 1, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dragos commented Dec 1, 2015

andrewor14 commented Dec 1, 2015

JoshRosen commented Dec 1, 2015

tnachen commented Dec 1, 2015

andrewor14 commented Dec 1, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 10, 2015

SparkQA commented Dec 10, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewor14 commented Dec 14, 2015

tnachen commented Dec 21, 2015

SparkQA commented Dec 21, 2015

Choose a reason for hiding this comment

SparkQA commented Dec 22, 2015

tnachen commented Dec 22, 2015

SparkQA commented Dec 22, 2015

tnachen commented Jan 4, 2016

andrewor14 commented Jan 28, 2016

SparkQA commented Jan 28, 2016

tnachen commented Jan 29, 2016

SparkQA commented Jan 29, 2016

tnachen commented Jan 29, 2016

SparkQA commented Jan 29, 2016

andrewor14 commented Feb 1, 2016