[SPARK-13492][MESOS] Configurable Mesos framework webui URL. #11369

s-urbaniak · 2016-02-25T13:19:56Z

What changes were proposed in this pull request?

Previously the Mesos framework webui URL was being derived only from the Spark UI address leaving no possibility to configure it. This commit makes it configurable. If unset it falls back to the previous behavior.

Motivation:
This change is necessary in order to be able to install Spark on DCOS and to be able to give it a custom service link. The configured webui_url is configured to point to a reverse proxy in the DCOS environment.

How was this patch tested?

Locally, using unit tests and on DCOS testing and stable revision.

mgummelt · 2016-02-25T17:07:27Z

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

Why expose both --webui-url and spark.mesos.dispatcher.webui.url. Isn't one sufficient?

We could only take spark.mesos.dispatcher.webui.url, sure. I thought this is idiomatic in this context.

The code here is a bit strange it forces you to pass the master string for example with --master flag although you can also define the master url in a file (as spark.master) with the --properties-file option.

Check here main args are parsed first and then any properties coming from a file: https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/deploy/mesos/MesosClusterDispatcherArguments.scala#L33-L35

In the latter case we dont really need the --master flag yet it will fail if you dont provide it. The restriction is not there for other parameters like --zk and spark.mesos.deploy.zookeeper.url.

https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/deploy/mesos/MesosClusterDispatcher.scala#L98-L103

There are a trillion ways to set configuration in Spark, half of them are deprecated, and some only work with certain entrypoint classes.

Some entrypoints, like SparkSubmit, support setting arbitrary properties in commands w/ something like --conf. MesosClusterDispatcher does not. So I say we just rely on --properties-file, rather than adding a command argument just for this one property. It's already arbitrary which properties are exposed as special command parameters, so let's not add a new one.

agreed, I removed the config option and will go the --properties-file route.

andrewor14 · 2016-02-26T01:19:25Z

ok to test

SparkQA · 2016-02-26T01:24:30Z

Test build #52011 has finished for PR 11369 at commit e6dedd5.

This patch fails Scala style tests.
This patch does not merge cleanly.
This patch adds no public classes.

SparkQA · 2016-02-26T14:39:53Z

Test build #52050 has finished for PR 11369 at commit 586d6de.

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

SparkQA · 2016-02-26T15:44:30Z

Test build #52054 has finished for PR 11369 at commit e105b1c.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

skonto · 2016-02-26T18:03:03Z

docs/running-on-mesos.md

Worths to mention that properties coming from a driver submission request are propagated from here:
https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/deploy/rest/mesos/MesosRestServer.scala#L87-L97

Also there are properties coming from MesosClusterScheduler's conf (initialized from MesosClusterDispatcher conf- https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/deploy/mesos/MesosClusterDispatcher.scala#L62):

eg. spark.mesos.uris at
https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala#L407

Both of these sets of properties seem to merge to form the final set of driver's spark properties IMHO these should be isolated or clarified into the docs the concept seems complicated.

Yes, it is complicated and opaque. A JIRA for this problem is welcome.

tnachen · 2016-02-26T18:31:25Z

@s-urbaniak You need to fix the scala style error:
/home/jenkins/workspace/SparkPullRequestBuilder/core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/CoarseMesosSchedulerBackendSuite.scala:229: File line length exceeds 100 characters

SparkQA · 2016-02-26T21:35:53Z

Test build #52067 has finished for PR 11369 at commit 3acdf74.

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

SparkQA · 2016-02-29T10:29:18Z

Test build #52181 has finished for PR 11369 at commit 42f46dd.

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

SparkQA · 2016-02-29T12:44:54Z

Test build #52184 has finished for PR 11369 at commit fa8570d.

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

tnachen · 2016-03-01T18:25:31Z

core/src/main/resources/org/apache/spark/ui/static/historypage.js

What was the problem with the /?

After discussing with @s-urbaniak directly, I see it's a bug where the link doesn't work when application proxy is set.

tnachen · 2016-03-01T23:53:42Z

@s-urbaniak I think we should keep the existing -p since users might actually depend on it. Otherwise this patch LGTM. @dragos you want to take a look?

dragos · 2016-03-03T10:11:02Z

LGTM.

tnachen · 2016-03-07T18:00:14Z

@andrewor14 this LGTM to me and @dragos , can you take a look?

Previously the Mesos framework webui URL was being derived only from the Spark UI address leaving no possibility to configure it. This commit makes it configurable. If unset it falls back to the previous behavior.

SparkQA · 2016-03-09T10:57:52Z

Test build #52742 has finished for PR 11369 at commit 46ea0ab.

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

andrewor14 · 2016-03-10T02:08:50Z

Merging into master.

Previously the Mesos framework webui URL was being derived only from the Spark UI address leaving no possibility to configure it. This commit makes it configurable. If unset it falls back to the previous behavior. Motivation: This change is necessary in order to be able to install Spark on DCOS and to be able to give it a custom service link. The configured `webui_url` is configured to point to a reverse proxy in the DCOS environment. Locally, using unit tests and on DCOS testing and stable revision. Author: Sergiusz Urbaniak <[email protected]> Closes apache#11369 from s-urbaniak/sur-webui-url. Conflicts: core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosSchedulerBackendSuite.scala

## What changes were proposed in this pull request? Previously the Mesos framework webui URL was being derived only from the Spark UI address leaving no possibility to configure it. This commit makes it configurable. If unset it falls back to the previous behavior. Motivation: This change is necessary in order to be able to install Spark on DCOS and to be able to give it a custom service link. The configured `webui_url` is configured to point to a reverse proxy in the DCOS environment. ## How was this patch tested? Locally, using unit tests and on DCOS testing and stable revision. Author: Sergiusz Urbaniak <[email protected]> Closes apache#11369 from s-urbaniak/sur-webui-url.

Previously the Mesos framework webui URL was being derived only from the Spark UI address leaving no possibility to configure it. This commit makes it configurable. If unset it falls back to the previous behavior. Motivation: This change is necessary in order to be able to install Spark on DCOS and to be able to give it a custom service link. The configured `webui_url` is configured to point to a reverse proxy in the DCOS environment. Locally, using unit tests and on DCOS testing and stable revision. Author: Sergiusz Urbaniak <[email protected]> Closes apache#11369 from s-urbaniak/sur-webui-url. Conflicts: core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosSchedulerBackendSuite.scala

mgummelt reviewed Feb 25, 2016
View reviewed changes

s-urbaniak force-pushed the sur-webui-url branch from e6dedd5 to 586d6de Compare February 26, 2016 12:41

skonto reviewed Feb 26, 2016
View reviewed changes

tnachen reviewed Mar 1, 2016
View reviewed changes

Sergiusz Urbaniak added 5 commits March 9, 2016 09:33

Mesos: Configurable Mesos framework webui URL.

9fbc059

Previously the Mesos framework webui URL was being derived only from the Spark UI address leaving no possibility to configure it. This commit makes it configurable. If unset it falls back to the previous behavior.

fix NPE

1245008

scala style fix

06334a4

MesosClusterDispatcher: remove --webui-url config

1ffc6d9

history-server: use relative urls

46ea0ab

s-urbaniak force-pushed the sur-webui-url branch from fa8570d to 46ea0ab Compare March 9, 2016 08:56

asfgit closed this in a4a0add Mar 10, 2016

mgummelt deleted the sur-webui-url branch August 2, 2016 21:01

[SPARK-13492][MESOS] Configurable Mesos framework webui URL. #11369

[SPARK-13492][MESOS] Configurable Mesos framework webui URL. #11369

Uh oh!

Conversation

s-urbaniak commented Feb 25, 2016

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewor14 commented Feb 26, 2016

Uh oh!

SparkQA commented Feb 26, 2016

Uh oh!

SparkQA commented Feb 26, 2016

Uh oh!

SparkQA commented Feb 26, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnachen commented Feb 26, 2016

Uh oh!

SparkQA commented Feb 26, 2016

Uh oh!

SparkQA commented Feb 29, 2016

Uh oh!

SparkQA commented Feb 29, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnachen commented Mar 1, 2016

Uh oh!

dragos commented Mar 3, 2016

Uh oh!

tnachen commented Mar 7, 2016

Uh oh!

SparkQA commented Mar 9, 2016

Uh oh!

andrewor14 commented Mar 10, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants