[SPARK-11859][Mesos] SparkContext accepts invalid Master URLs in the form zk://host:port for a multi-master Mesos cluster using ZooKeeper #9886
Conversation
…multi-master Mesos cluster using ZooKeeper
|
@andrewor14 I wonder if we shouldn't first warn about this, and defer the actual failure until 2.0. There might be people relying on this loophole. If I understand correctly, people could connect using |
|
LGTM, though I tend to agree there's a little risk here in making something that shouldn't work actually not work. |
|
@toddwan what do you think about writing a separate code path to handle the incorrect |
|
+1 on having a fall back with a warning message as well. Everything else LGTM |
|
A new commit has been appended to the PR. Now a warning message is printed first if the given mesos Master URL starts with |
|
Yes, that's pretty nice and clean. |
|
retest this please |
|
LGTM, I'll merge this once tests pass. |
|
Test build #46618 has finished for PR 9886 at commit
|
There was a problem hiding this comment.
Nitpicking, but I would be more explicit: "This will stop working in Spark 2.0".
|
other than that, LGTM |
|
retest this please |
1 similar comment
|
retest this please |
|
Test build #46705 has finished for PR 9886 at commit
|
|
retest this please |
|
Test build #46711 has finished for PR 9886 at commit
|
|
retest this please |
|
Test build #46732 has finished for PR 9886 at commit
|
|
@andrewor14 I am not sure about the cause of the unit test failure, but the failed test reported in the latest test build can pass on my machine. |
|
Test build #2123 has finished for PR 9886 at commit
|
|
LGTM, will merge soon. |
|
LGTM |
…form zk://host:port for a multi-master Mesos cluster using ZooKeeper * According to below doc and validation logic in [SparkSubmit.scala](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala#L231), master URL for a mesos cluster should always start with `mesos://` http://spark.apache.org/docs/latest/running-on-mesos.html `The Master URLs for Mesos are in the form mesos://host:5050 for a single-master Mesos cluster, or mesos://zk://host:2181 for a multi-master Mesos cluster using ZooKeeper.` * However, [SparkContext.scala](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/SparkContext.scala#L2749) fails the validation and can receive master URL in the form `zk://host:port` * For the master URLs in the form `zk:host:port`, the valid form should be `mesos://zk://host:port` * This PR restrict the validation in `SparkContext.scala`, and now only mesos master URLs prefixed with `mesos://` can be accepted. * This PR also updated corresponding unit test. Author: toddwan <[email protected]> Closes #9886 from toddwan/S11859. (cherry picked from commit e074944) Signed-off-by: Sean Owen <[email protected]>
|
@toddwan let me know your JIRA handle and I'll credit you. Merged to master/1.6 |
7 participants
mesos://http://spark.apache.org/docs/latest/running-on-mesos.html
The Master URLs for Mesos are in the form mesos://host:5050 for a single-master Mesos cluster, or mesos://zk://host:2181 for a multi-master Mesos cluster using ZooKeeper.zk://host:portzk:host:port, the valid form should bemesos://zk://host:portSparkContext.scala, and now only mesos master URLs prefixed withmesos://can be accepted.