[Spark-3490] Disable SparkUI for tests #2363

andrewor14 · 2014-09-11T17:27:21Z

We currently open many ephemeral ports during the tests, and as a result we occasionally can't bind to new ones. This has caused the DriverSuite and the SparkSubmitSuite to fail intermittently.

By disabling the SparkUI when it's not needed, we already cut down on the number of ports opened significantly, on the order of the number of SparkContexts ever created. We must keep it enabled for a few tests for the UI itself, however.

Branch-1.2 #2363 (this PR, original)
Branch-1.1 #2415
Branch-1.0 #3959
Branch-0.9 #3961

SparkQA · 2014-09-11T17:34:31Z

QA tests have started for PR 2363 at commit 29c9b5b.

This patch merges cleanly.

SparkQA · 2014-09-11T17:35:32Z

QA tests have finished for PR 2363 at commit 29c9b5b.

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

SparkQA · 2014-09-11T17:54:06Z

QA tests have started for PR 2363 at commit 8f5ae53.

This patch merges cleanly.

tgravescs · 2014-09-11T17:58:36Z

core/src/main/scala/org/apache/spark/SparkContext.scala

-  private[spark] val ui = new SparkUI(this)
-  ui.bind()
+  private[spark] val ui: Option[SparkUI] =
+    if (conf.getBoolean("spark.ui.enabled", true)) {


is this meant for the user to set? If so we should document. If not perhaps we should start naming internal configs internal. or test. for testing ones.

I don't see a particularly strong use case for the user to disable the UI, so I think we should keep this internal.

SparkQA · 2014-09-11T18:44:04Z

QA tests have finished for PR 2363 at commit 8f5ae53.

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

SparkQA · 2014-09-11T18:49:11Z

QA tests have started for PR 2363 at commit a431b84.

This patch merges cleanly.

JoshRosen · 2014-09-11T18:56:15Z

There are two old JIRAs that seem relevant:

SPARK-2100: "Allow users to disable Jetty Spark UI in local mode"
SPARK-1047: "Ability to disable the spark ui server (unit tests)"

andrewor14 · 2014-09-11T18:59:38Z

Oops, thanks @JoshRosen. Mine seems to be a duplicate.

andrewor14 · 2014-09-11T19:36:28Z

@tdas I made a few changes in the streaming code to get this to work, can you verify?

SparkQA · 2014-09-11T19:49:05Z

QA tests have finished for PR 2363 at commit a431b84.

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

SparkQA · 2014-09-11T21:24:13Z

QA tests have started for PR 2363 at commit 30c93a2.

This patch merges cleanly.

tdas · 2014-09-11T21:24:18Z

LGTM from the streaming UI point of view. Though I would run the ignored UI tests locally to make sure we havent broken anything.

SparkQA · 2014-09-11T22:24:11Z

QA tests have finished for PR 2363 at commit 30c93a2.

This patch passes unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class CreateTableAsSelect(
- case class CreateTableAsSelect(

SparkQA · 2014-09-11T22:44:28Z

QA tests have started for PR 2363 at commit 332a7d5.

This patch merges cleanly.

SparkQA · 2014-09-11T23:48:46Z

QA tests have finished for PR 2363 at commit 332a7d5.

This patch passes unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class CreateTableAsSelect(
- case class CreateTableAsSelect(

andrewor14 · 2014-09-12T00:18:27Z

I have tested the ignore tests locally and they pass. I am merging this into master and 1.1, thanks.

We currently open many ephemeral ports during the tests, and as a result we occasionally can't bind to new ones. This has caused the `DriverSuite` and the `SparkSubmitSuite` to fail intermittently. By disabling the `SparkUI` when it's not needed, we already cut down on the number of ports opened significantly, on the order of the number of `SparkContexts` ever created. We must keep it enabled for a few tests for the UI itself, however. Author: Andrew Or <andrewor14@gmail.com> Closes #2363 from andrewor14/disable-ui-for-tests and squashes the following commits: 332a7d5 [Andrew Or] No need to set spark.ui.port to 0 anymore 30c93a2 [Andrew Or] Simplify streaming UISuite a431b84 [Andrew Or] Fix streaming test failures 8f5ae53 [Andrew Or] Fix no new line at the end 29c9b5b [Andrew Or] Disable SparkUI for tests (cherry picked from commit 6324eb7) Signed-off-by: Andrew Or <andrewor14@gmail.com> Conflicts: pom.xml yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala yarn/common/src/main/scala/org/apache/spark/scheduler/cluster/YarnClientSchedulerBackend.scala

tgravescs · 2014-09-12T00:46:52Z

@andrewor14 what about my comment?

tgravescs · 2014-09-12T00:49:00Z

To clarify I suggested possibly naming the config with .internal. In the very least I think we should put a comment by it saying so in case other users see it and add documentation or start using it.

Personally I see configs are user interfaces. If we don't make it clear they are internal they people can start using them and we can't remove them again. Perhaps others disagree but we should decide as community what the policy is.

andrewor14 · 2014-09-12T01:23:09Z

@tgravescs Sorry I wasn't clear. There are tons of other configs in Spark that we do not intend to expose, like spark.cleaner.referenceTracking and spark.storage.blockManagerSlaveTimeoutMs. These are mainly introduced to provide a backdoor to a feature we added in case things go wrong, but not intended for the average user to configure themselves. There are already too many things that are tunable in Spark, and exposing more than the essential configs may confuse the user. Hadoop for example has millions of exposed configs in each component, and we are making an effort to avoid something similar.

As for renaming the configs to *.internal, I don't feel strongly for or against it. Though if we to plan to do that then we also need to do the same for all the existing configs that we haven't exposed, and that is definitely outside the scope of this PR. The main motivation of this patch is to make a few particular test suites less flaky. Perhaps we can file a separate JIRA to fix this.

tgravescs · 2014-09-12T13:17:54Z

@andrewor14 ok I filed https://issues.apache.org/jira/browse/SPARK-3508. Please make sure all user concerns are addressed before committing. I would have like to see at least a comment added here.

tgravescs · 2014-09-12T15:51:21Z

@andrewor14 this seems to also be causing a compilation error on branch-1.1:

core/src/main/scala/org/apache/spark/scheduler/cluster/SimrSchedulerBackend.scala:46: not found: type Configuration
15:41:51 [ERROR] val conf = new Configuration()

andrewor14 · 2014-09-12T17:04:18Z

Oops thanks, this is fixed in #2372.

Original PR: #2363 Author: Andrew Or <andrewor14@gmail.com> Closes #2415 from andrewor14/disable-ui-for-tests-1.1 and squashes the following commits: 8d9df5a [Andrew Or] Oops, missed one. 509507d [Andrew Or] Backport #2363 (SPARK-3490) into branch-1.1

Original PR: apache#2363 Author: Andrew Or <andrewor14@gmail.com> Closes apache#2415 from andrewor14/disable-ui-for-tests-1.1 and squashes the following commits: 8d9df5a [Andrew Or] Oops, missed one. 509507d [Andrew Or] Backport apache#2363 (SPARK-3490) into branch-1.1

Branch-1.2 #2363 (original) Branch-1.1 #2415 Branch-1.0 #3959 Branch-0.9 #3961 (this PR) Author: Andrew Or <andrew@databricks.com> Closes #3961 from andrewor14/ui-ports-0.9 and squashes the following commits: 8644997 [Andrew Or] Disable UI for tests

Disable SparkUI for tests

29c9b5b

Fix no new line at the end

8f5ae53

tgravescs reviewed Sep 11, 2014
View reviewed changes

Fix streaming test failures

a431b84

Simplify streaming UISuite

30c93a2

No need to set spark.ui.port to 0 anymore

332a7d5

asfgit closed this in 6324eb7 Sep 12, 2014

andrewor14 deleted the disable-ui-for-tests branch September 12, 2014 00:24

andrewor14 mentioned this pull request Sep 12, 2014

[HOTFIX] Fix compilation errors in branch-1.1 #2372

Closed

andrewor14 mentioned this pull request Sep 16, 2014

[SPARK-3490] Disable SparkUI for tests (backport into 1.1) #2415

Closed

andrewor14 restored the disable-ui-for-tests branch September 16, 2014 18:35

andrewor14 deleted the disable-ui-for-tests branch September 16, 2014 18:37

andrewor14 added a commit to andrewor14/spark that referenced this pull request Sep 16, 2014

Backport apache#2363 (SPARK-3490) into branch-1.1

509507d

JoshRosen mentioned this pull request Dec 12, 2014

[SPARK-3431] [WIP] Parallelize Scala/Java test execution #3564

Closed

This was referenced Jan 8, 2015

[Spark-3490] Disable SparkUI for tests (backport into 1.0) #3959

Closed

[Spark-3490] Disable SparkUI for tests (backport into 0.9) #3961

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Spark-3490] Disable SparkUI for tests #2363

[Spark-3490] Disable SparkUI for tests #2363

andrewor14 commented Sep 11, 2014

SparkQA commented Sep 11, 2014

SparkQA commented Sep 11, 2014

SparkQA commented Sep 11, 2014

tgravescs Sep 11, 2014

andrewor14 Sep 11, 2014

SparkQA commented Sep 11, 2014

SparkQA commented Sep 11, 2014

JoshRosen commented Sep 11, 2014

andrewor14 commented Sep 11, 2014

andrewor14 commented Sep 11, 2014

SparkQA commented Sep 11, 2014

SparkQA commented Sep 11, 2014

tdas commented Sep 11, 2014

SparkQA commented Sep 11, 2014

SparkQA commented Sep 11, 2014

SparkQA commented Sep 11, 2014

andrewor14 commented Sep 12, 2014

tgravescs commented Sep 12, 2014

tgravescs commented Sep 12, 2014

andrewor14 commented Sep 12, 2014

tgravescs commented Sep 12, 2014

tgravescs commented Sep 12, 2014

andrewor14 commented Sep 12, 2014

[Spark-3490] Disable SparkUI for tests #2363

[Spark-3490] Disable SparkUI for tests #2363

Conversation

andrewor14 commented Sep 11, 2014

SparkQA commented Sep 11, 2014

SparkQA commented Sep 11, 2014

SparkQA commented Sep 11, 2014

tgravescs Sep 11, 2014

Choose a reason for hiding this comment

andrewor14 Sep 11, 2014

Choose a reason for hiding this comment

SparkQA commented Sep 11, 2014

SparkQA commented Sep 11, 2014

JoshRosen commented Sep 11, 2014

andrewor14 commented Sep 11, 2014

andrewor14 commented Sep 11, 2014

SparkQA commented Sep 11, 2014

SparkQA commented Sep 11, 2014

tdas commented Sep 11, 2014

SparkQA commented Sep 11, 2014

SparkQA commented Sep 11, 2014

SparkQA commented Sep 11, 2014

andrewor14 commented Sep 12, 2014

tgravescs commented Sep 12, 2014

tgravescs commented Sep 12, 2014

andrewor14 commented Sep 12, 2014

tgravescs commented Sep 12, 2014

tgravescs commented Sep 12, 2014

andrewor14 commented Sep 12, 2014