[SPARK-10372] [CORE] basic test framework for entire spark scheduler #8559

squito · 2015-09-01T17:12:00Z

This is a basic framework for testing the entire scheduler. The tests this adds aren't very interesting -- the point of this PR is just to setup the framework, to keep the initial change small, but it can be built upon to test more features (eg., speculation, killing tasks, blacklisting, etc.).

SparkQA · 2015-09-01T17:34:48Z

Test build #41877 has finished for PR 8559 at commit 844bd33.

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

squito · 2015-09-01T17:38:27Z

Jenkins, retest this please

squito · 2015-09-01T18:05:32Z

core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala

@@ -1123,8 +1133,15 @@ class DAGScheduler(
            // TODO: Cancel running tasks in the stage
            logInfo(s"Resubmitting $mapStage (${mapStage.name}) and " +
              s"$failedStage (${failedStage.name}) due to fetch failure")
+            // We might get lots of fetch failed for this stage, from lots of executors.
+            // Its better if we can resubmit for all the failed executors at one time, so lets
+            // just wait a *bit* before we resubmit.


I have no idea if this comment is accurate or not -- I just found this pretty confusing and felt it deserved some comment, so I took my best guess. The tests in DAGSchedulerSuite pass if you just post this event directly (with other corresponding changes to go along with it ...)

see #8560 to show that you can remove the messageScheduler completely (the tests pass, anyway ...)

SparkQA · 2015-09-01T20:34:40Z

Test build #41878 has finished for PR 8559 at commit 844bd33.

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

SparkQA · 2015-10-20T18:11:50Z

Test build #43989 has finished for PR 8559 at commit 093a643.

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

SparkQA · 2015-11-05T08:46:06Z

Test build #45102 has finished for PR 8559 at commit 609b61e.

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

SparkQA · 2015-11-10T03:15:26Z

Test build #45448 has finished for PR 8559 at commit 609b61e.

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

SparkQA · 2015-12-03T22:13:27Z

Test build #47164 has finished for PR 8559 at commit d61b13b.

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

SparkQA · 2016-05-10T22:08:45Z

Test build #58266 has finished for PR 8559 at commit 50d9162.

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

SparkQA · 2016-05-10T23:20:22Z

Test build #58275 has finished for PR 8559 at commit 46ebdb6.

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

SparkQA · 2016-05-13T21:00:01Z

Test build #58590 has finished for PR 8559 at commit 6e8562f.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class BasicSchedulerIntegrationSuite extends SchedulerIntegrationSuite[SingleCoreMockBackend]
- class MultiExecutorBackend(
- class ExecutorTaskStatus(val host: String, val executorId: String, var freeCores: Int)

SparkQA · 2016-05-14T00:06:18Z

Test build #58593 has finished for PR 8559 at commit 0b6da39.

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

SparkQA · 2016-05-17T17:39:38Z

Test build #58704 has finished for PR 8559 at commit c091187.

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

squito · 2016-05-17T20:18:52Z

@mateiz @markhamstra @kayousterhout I've updated this to avoid changing the scheduler at all, and instead just run the mock backend in another thread. It slightly complicates writing the tests, but avoids a (small) race in the tests and was probably necessary when we got to more complicated tests in any case.

In particular I want this for more tests on blacklisting (SPARK-8426), but I think there are a handful of open scheduler issues which will benefit from it.

SparkQA · 2016-05-17T20:20:22Z

Test build #58712 has finished for PR 8559 at commit 3b67b2a.

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

squito · 2016-05-18T15:40:47Z

sorry one more update -- I realized that some of the abstractions were too complicated for the simple tests I had, they were a step towards some more complex tests involving multiple executors. I decided to just go ahead and add an example of those tests as well (which actually required some other minor changes in any case).

I'm sure there will be more to add to the framework at some point, but I think this is a good start for letting us add more tests.

squito · 2016-05-20T22:03:43Z

Jenkins, retest this please

SparkQA · 2016-05-21T00:13:40Z

Test build #59038 has finished for PR 8559 at commit 67acce9.

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

markhamstra · 2016-05-23T04:58:24Z

core/src/main/scala/org/apache/spark/scheduler/TaskResult.scala

    val numUpdates = in.readInt
    if (numUpdates == 0) {
-      accumUpdates = null
+      accumUpdates = Seq()


I'm a little surprised that we were getting away with that, given the number of places that are using DirectTaskResult#accumUpdates without checking for null.

yeah, likewise. I guess in all real spark jobs, the number of accum updates is > 0, which is why this didn't particularly matter except for these mocks. OTOH if this has any real danger we should just fix this separately (and be sure to include it in 2.0)

squito · 2016-05-23T14:40:04Z

@markhamstra thanks for the review. I went ahead and added one more commit and removed DummyExternalClusterManager, just used Mock in the old tests. Honestly I kinda prefer having ExternalClusterManagerSuite being very simple and self-contained, but also don't feel particularly strongly about it. (easy to back out since its just one final commit.)

markhamstra · 2016-05-23T14:46:30Z

@squito I don't feel really strongly about it, either. My only concern was for others adding tests needing a mock ExternalClusterManager in the future and not knowing which one to use, why they are different, whether any new mocking needs to be added to both, etc.

If someone does feel strongly about maintaining the separation, then we can put things back the way you had them.

SparkQA · 2016-05-23T16:30:11Z

Test build #59130 has finished for PR 8559 at commit 1fdf2aa.

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

squito · 2016-05-24T21:54:56Z

@markhamstra alright, in that case do you have any objections if I merge this, one commit back? I'll throw in a comment on DummyExternalClusterManager pointing to MockExternalClusterManager, as I think most folks writing tests would rather start from there.

markhamstra · 2016-05-24T21:58:51Z

@squito Yeah, that's fine. I haven't gone through the new tests closely to make sure that they are doing what they say they are doing, but the changes to both non-test code and previous tests look safe.

SparkQA · 2016-05-24T23:56:50Z

Test build #59226 has finished for PR 8559 at commit 278122e.

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

rxin · 2016-05-26T05:53:53Z

This is pretty cool!

rxin · 2016-05-26T05:56:15Z

on a related note, @squito can you in the future leave a msg indicating the branch a pr was merged once you merge it? There have been cases that lead to race conditions in merging and also mistakes in the branches that we needed to go back and audit.

squito · 2016-06-01T20:05:30Z

sure thing, sorry about that @rxin ! fwiw, this was just merged to master.

rxin · 2016-06-01T20:14:18Z

np - a lot of people forget to do this. I am just bugging everybody :)

rxin · 2016-06-01T20:14:49Z

btw i've seen a few flaky tests introduced by this one. If we see more of them we might need to disable and increase timeouts.

squito · 2016-06-01T21:25:20Z

Oh thanks for pointing out the flaky test, I think I figured out the problem, opened https://issues.apache.org/jira/browse/SPARK-15714. Weird I never this running locally -- I'd expect this race to be pretty common. Just tried and an on my laptop it only shows up once in 100 runs.

skonto · 2016-06-03T11:37:36Z

About the flaky test (checked locally on master through my IDE its easily reproducible):

`Map(8 -> 42, 7 -> 42) was not empty
ScalaTestFailureLocation: org.apache.spark.scheduler.BlacklistIntegrationSuite$$anonfun$1 at (BlacklistIntegrationSuite.scala:51)
org.scalatest.exceptions.TestFailedException: Map(8 -> 42, 7 -> 42) was not empty
at org.scalatest.Assertions$class.newAssertionFailedException(Assertions.scala:500)
at org.scalatest.FunSuite.newAssertionFailedException(FunSuite.scala:1555)
at org.scalatest.Assertions$AssertionsHelper.macroAssert(Assertions.scala:466)
at org.apache.spark.scheduler.BlacklistIntegrationSuite$$anonfun$1.apply$mcV$sp(BlacklistIntegrationSuite.scala:51)
at org.apache.spark.scheduler.SchedulerIntegrationSuite$$anonfun$testScheduler$1.apply$mcV$sp(SchedulerIntegrationSuite.scala:88)
at org.apache.spark.scheduler.SchedulerIntegrationSuite$$anonfun$testScheduler$1.apply(SchedulerIntegrationSuite.scala:84)
at org.apache.spark.scheduler.SchedulerIntegrationSuite$$anonfun$testScheduler$1.apply(SchedulerIntegrationSuite.scala:84)
at org.scalatest.Transformer$$anonfun$apply$1.apply$mcV$sp(Transformer.scala:22)
at org.scalatest.OutcomeOf$class.outcomeOf(OutcomeOf.scala:85)
at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
at org.scalatest.Transformer.apply(Transformer.scala:22)
at org.scalatest.Transformer.apply(Transformer.scala:20)
at org.scalatest.FunSuiteLike$$anon$1.apply(FunSuiteLike.scala:166)
at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:57)
at org.scalatest.FunSuiteLike$class.invokeWithFixture$1(FunSuiteLike.scala:163)
at org.scalatest.FunSuiteLike$$anonfun$runTest$1.apply(FunSuiteLike.scala:175)
at org.scalatest.FunSuiteLike$$anonfun$runTest$1.apply(FunSuiteLike.scala:175)
at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306)
at org.scalatest.FunSuiteLike$class.runTest(FunSuiteLike.scala:175)
at

....
Also check here:
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59928/

squito · 2016-06-03T16:51:08Z

@skonto I just merged #13454, please let me know if that does not address the issue.

skonto · 2016-06-03T18:00:23Z

Locally i get now:

Futures timed out after [1 second]
java.util.concurrent.TimeoutException: Futures timed out after [1 second]
at scala.concurrent.impl.Promise$DefaultPromise.ready(Promise.scala:219)
at scala.concurrent.impl.Promise$DefaultPromise.ready(Promise.scala:153)
at scala.concurrent.Await$$anonfun$ready$1.apply(package.scala:169)
at scala.concurrent.Await$$anonfun$ready$1.apply(package.scala:169)
at scala.concurrent.BlockContext$DefaultBlockContext$.blockOn(BlockContext.scala:53)
at scala.concurrent.Await$.ready(package.scala:169)
at org.apache.spark.scheduler.BlacklistIntegrationSuite$$anonfun$3$$anonfun$apply$mcV$sp$6.apply(BlacklistIntegrationSuite.scala:90)
at org.apache.spark.scheduler.BlacklistIntegrationSuite$$anonfun$3$$anonfun$apply$mcV$sp$6.apply(BlacklistIntegrationSuite.scala:87)
at org.apache.spark.scheduler.SchedulerIntegrationSuite.withBackend(SchedulerIntegrationSuite.scala:224)

squito · 2016-06-03T21:09:19Z

@skonto since you seem to be able to reproduce this regularly though I can't -- can you try just increasing the timeouts? honestly I'm surprised you're hitting them, but if we just need to bump up the timeouts a little more, we can do that. If there is something else wrong, eg. another race, than lets turn these tests off for now till I can nail it down.

skonto · 2016-06-06T12:30:25Z

@squito not really check here:
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60037/
Tests should be stable not just a local problem btw. I dont really know why the test takes so long but needs investigation. I suggest disable that test for now or try it in a diff env, i may check it as well locally when time permits. For now i would like to have a successful build for the pr.

squito · 2016-06-06T17:14:15Z

@skonto agreed, I have opened https://issues.apache.org/jira/browse/SPARK-15783 + PR #13528 to turn them off for now.

I was at least able to reproduce the failures with 5k runs on my laptop, (also seems worse when I run all the different tests together vs just running one test repeatedly) so I will try to nail down the cause more before adding back.

squito · 2016-06-06T19:56:32Z

@skonto I just merged #13528 this to turn the tests off for now. Sorry again about the issues, and I'll work on figuring out the cause of the flakiness

squito reviewed Sep 1, 2015
View reviewed changes

squito force-pushed the SPARK-10372-scheduler-integs branch from 844bd33 to 093a643 Compare October 20, 2015 16:08

squito force-pushed the SPARK-10372-scheduler-integs branch from d61b13b to 50d9162 Compare May 10, 2016 20:32

squito force-pushed the SPARK-10372-scheduler-integs branch from 46ebdb6 to 6e8562f Compare May 13, 2016 19:08

squito force-pushed the SPARK-10372-scheduler-integs branch from 6e8562f to 0b6da39 Compare May 13, 2016 22:13

squito added 5 commits May 17, 2016 10:56

basic test framework for entire spark scheduler

a6e94d7

TaskResultGetter now expects there to always be non-null accum updates

20fb3e9

switch to making backend run in another thread

0ca9815

remove MultiExecutorBackend for now

421c2a1

remove uncertain comment about messageScheduler

c091187

squito force-pushed the SPARK-10372-scheduler-integs branch from 0b6da39 to c091187 Compare May 17, 2016 15:58

cleanup

3b67b2a

squito added 2 commits May 18, 2016 10:29

add BlacklistIntegrationSuite and corresponding refactoring

79bc384

cleanup

8349b76

squito mentioned this pull request May 20, 2016

Performance tests for the scheduler squito/spark#5

Merged

markhamstra reviewed May 23, 2016
View reviewed changes

check initialization and add comments

278122e

squito force-pushed the SPARK-10372-scheduler-integs branch from 1fdf2aa to 278122e Compare May 24, 2016 22:04

asfgit closed this in dfc9fc0 May 26, 2016

skonto mentioned this pull request Jun 6, 2016

[SPARK-11714][Mesos] Make Spark on Mesos honor port restrictions on coarse grain mode #11157

Closed

[SPARK-10372] [CORE] basic test framework for entire spark scheduler #8559

[SPARK-10372] [CORE] basic test framework for entire spark scheduler #8559

Uh oh!

Conversation

squito commented Sep 1, 2015 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Sep 1, 2015

Uh oh!

squito commented Sep 1, 2015

Uh oh!

squito Sep 1, 2015

Choose a reason for hiding this comment

Uh oh!

squito Sep 2, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 1, 2015

Uh oh!

SparkQA commented Oct 20, 2015

Uh oh!

SparkQA commented Nov 5, 2015

Uh oh!

SparkQA commented Nov 10, 2015

Uh oh!

SparkQA commented Dec 3, 2015

Uh oh!

SparkQA commented May 10, 2016

Uh oh!

SparkQA commented May 10, 2016

Uh oh!

SparkQA commented May 13, 2016

Uh oh!

SparkQA commented May 14, 2016

Uh oh!

SparkQA commented May 17, 2016

Uh oh!

squito commented May 17, 2016

Uh oh!

SparkQA commented May 17, 2016

Uh oh!

squito commented May 18, 2016

Uh oh!

squito commented May 20, 2016

Uh oh!

SparkQA commented May 21, 2016

Uh oh!

markhamstra May 23, 2016

Choose a reason for hiding this comment

Uh oh!

squito May 23, 2016

Choose a reason for hiding this comment

Uh oh!

squito commented May 23, 2016

Uh oh!

markhamstra commented May 23, 2016

Uh oh!

SparkQA commented May 23, 2016

Uh oh!

squito commented May 24, 2016

Uh oh!

markhamstra commented May 24, 2016

Uh oh!

SparkQA commented May 24, 2016

Uh oh!

rxin commented May 26, 2016

Uh oh!

rxin commented May 26, 2016

Uh oh!

squito commented Jun 1, 2016

Uh oh!

rxin commented Jun 1, 2016

Uh oh!

rxin commented Jun 1, 2016

Uh oh!

squito commented Jun 1, 2016

Uh oh!

skonto commented Jun 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

squito commented Sep 1, 2015 •

edited

Loading

skonto commented Jun 3, 2016 •

edited

Loading

skonto commented Jun 3, 2016 •

edited

Loading

skonto commented Jun 6, 2016 •

edited

Loading