SPARK-1143: refactor TaskSchedulerImplSuite #339

CodingCat · 2014-04-06T22:24:03Z

https://issues.apache.org/jira/browse/SPARK-1143

The current TaskSchedulerImplSuite actually bypasses the scheduling code (mainly resourceOffer())

In this patch, I removed the original dummy ResourceOffer and made the test case call taskScheduler.resourceOffer() directly,

AmplabJenkins · 2014-04-06T22:27:23Z

Merged build triggered.

AmplabJenkins · 2014-04-06T22:27:29Z

Merged build started.

AmplabJenkins · 2014-04-06T22:32:23Z

Merged build triggered.

AmplabJenkins · 2014-04-06T22:32:29Z

Merged build started.

AmplabJenkins · 2014-04-06T22:37:38Z

Merged build finished.

AmplabJenkins · 2014-04-06T22:37:39Z

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13825/

AmplabJenkins · 2014-04-07T00:03:30Z

Merged build finished.

AmplabJenkins · 2014-04-07T00:03:31Z

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13826/

AmplabJenkins · 2014-04-07T00:22:23Z

Merged build triggered.

AmplabJenkins · 2014-04-07T00:22:29Z

Merged build started.

AmplabJenkins · 2014-04-07T01:22:52Z

Merged build finished. All automated tests passed.

AmplabJenkins · 2014-04-07T01:22:52Z

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13828/

CodingCat · 2014-04-07T13:51:58Z

the previous failures are due to timeout and failed to fetch remote repo,

anyone wants to review this?

Conf improvements There are two new features. 1. Allow users to set arbitrary akka configurations via spark conf. 2. Allow configuration to be printed in logs for diagnosis.

CodingCat · 2014-04-11T12:45:27Z

Hi, @kayousterhout , mind reviewing this?

kayousterhout · 2014-04-11T16:30:27Z

I won't have time to review this for a week or so

CodingCat · 2014-04-11T16:49:05Z

sure, take your time

I notified you just because you're the original reporter of the issue and I want to ensure I understand the issue correctly

AmplabJenkins · 2014-04-11T22:43:11Z

Merged build triggered.

AmplabJenkins · 2014-04-11T22:43:16Z

Merged build started.

AmplabJenkins · 2014-04-11T23:21:48Z

Merged build finished. All automated tests passed.

AmplabJenkins · 2014-04-11T23:21:49Z

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14066/

CodingCat · 2014-04-11T23:22:14Z

rebased~

kayousterhout · 2014-04-12T04:04:11Z

The problem I was describing in the JIRA is that all but the last 2 tests in this suite are designed to test the pool / fairness mechanisms (i.e., that the output of rootPool.getSortedTaskSetQueue() is correct), not the scheduler code. I think the right solution here is to move these tests into a separate test suite specifically for the pool stuff -- not to make these tests use the scheduler (since using the scheduler obfuscates where the actually error would be if the tests failed).

CodingCat · 2014-04-13T21:57:19Z

Hi, @kayousterhout, you mean just split the test cases and move some of them to another file?

I can do that...but do you think the test cases specific to the TaskSchedulerImpl class are also missed? (except the last two)

I think the implementation in this patch is equivalent to the original one, as we assume the locality is Any and the assert statements are actually testing the results of rootPool.getSortedTaskSetQueue(),

I think for fully test the critical code path of TaskSchedulerImpl, we need some test cases on the locality?

kayousterhout · 2014-04-13T22:47:01Z

We definitely don't have sufficient testing for TaskSchedulerImpl right now
(since the only tests that test that at all are the last two in this file),
so at some point adding more tests for that would be good too.

On Sun, Apr 13, 2014 at 2:57 PM, Nan Zhu [email protected] wrote:

Hi, @kayousterhout https://github.com/kayousterhout, you mean just
split the test cases and move some of them to another file?

I can do that...but do you think the test cases specific to the
TaskSchedulerImpl class are also missed?

I think the implementation in this patch is equivalent to the original
one, as we assume the locality is Any and the assert statements are
actually testing the results of rootPool.getSortedTaskSetQueue(),

I think for fully test the critical code path of TaskSchedulerImpl, we
need some test cases on the locality?

Reply to this email directly or view it on GitHubhttps://github.com//pull/339#issuecomment-40321243
.

AmplabJenkins · 2014-05-13T01:52:58Z

Merged build triggered.

SparkQA · 2014-09-23T12:07:16Z

QA tests have finished for PR 339 at commit 4341383.

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

SparkQA · 2014-09-23T12:07:19Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20701/

SparkQA · 2014-09-23T12:19:20Z

QA tests have started for PR 339 at commit 0df7499.

This patch merges cleanly.

SparkQA · 2014-09-23T13:09:55Z

QA tests have finished for PR 339 at commit 0df7499.

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

SparkQA · 2014-09-23T13:09:59Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20702/

SparkQA · 2014-09-23T13:39:19Z

QA tests have started for PR 339 at commit 50e2463.

This patch merges cleanly.

nchammas · 2014-09-23T14:19:36Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20702/

I don't know where this message is coming from, and I don't see any recent changes to the dev/run-tests-jenkins script to explain it.

@pwendell?

SparkQA · 2014-09-23T14:46:12Z

QA tests have finished for PR 339 at commit 50e2463.

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

SparkQA · 2014-09-23T14:46:15Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20705/

nchammas · 2015-01-04T20:16:35Z

@CodingCat @kayousterhout What's the status of this PR? It hasn't been updated in several months.

andrewor14 · 2015-01-08T21:45:46Z

@kayousterhout does this PR reflect what you had in mind in your description for SPARK-1143

CodingCat · 2015-01-08T21:46:56Z

@andrewor14 I need to add a design doc then?

kayousterhout · 2015-01-08T21:51:29Z

@CodingCat @andrewor14 This isn't quite what I had in mind; I'm going to submit a (simpler) PR for this in the next few hours. I think this one is more complicated than necessary.

andrewor14 · 2015-01-08T21:51:34Z

No need for design doc; I thought for a second you were refactoring TaskSchedulerImpl itself. My mistake.

andrewor14 · 2015-01-08T21:52:17Z

Ok, since @kayousterhout is planning on submitting a simpler version of this, @CodingCat would you mind closing this PR?

CodingCat · 2015-01-08T21:52:56Z

sure

The current TaskSchedulerImplSuite includes some tests that are actually for the TaskSchedulerImpl, but many of the tests avoid using the TaskSchedulerImpl entirely, and actually test the pool and scheduling algorithm mechanisms. This commit separates the pool/scheduling algorithm tests into their own suite, simplifies those tests, and attempts to more helpfully comment the fair scheduler test. The pull request replaces apache#339.

The current TaskSchedulerImplSuite includes some tests that are actually for the TaskSchedulerImpl, but the remainder of the tests avoid using the TaskSchedulerImpl entirely, and actually test the pool and scheduling algorithm mechanisms. This commit separates the pool/scheduling algorithm tests into their own suite, and also simplifies those tests. The pull request replaces #339. Author: Kay Ousterhout <[email protected]> Closes #3967 from kayousterhout/SPARK-1143 and squashes the following commits: 8a898c4 [Kay Ousterhout] [SPARK-1143] Separate pool tests into their own suite.

…ache#339)

Fix if issue in keyword contain case

CodingCat changed the title ~~SPARK-1143: refactor TaskSchedulerImplSuite using taskScheduler.resourceOffer~~ [WIP]SPARK-1143: refactor TaskSchedulerImplSuite using taskScheduler.resourceOffer Apr 6, 2014

CodingCat changed the title ~~[WIP]SPARK-1143: refactor TaskSchedulerImplSuite using taskScheduler.resourceOffer~~ SPARK-1143: refactor TaskSchedulerImplSuite using taskScheduler.resourceOffer Apr 6, 2014

CodingCat changed the title ~~SPARK-1143: refactor TaskSchedulerImplSuite using taskScheduler.resourceOffer~~ SPARK-1143: refactor TaskSchedulerImplSuite using taskScheduler.resourceOffers Apr 6, 2014

CodingCat changed the title ~~SPARK-1143: refactor TaskSchedulerImplSuite using taskScheduler.resourceOffers~~ SPARK-1143: refactor TaskSchedulerImplSuite May 13, 2014

add more tests for TaskSchedulerImpl

4341383

fix error generated by rebase

0df7499

update SchedulingMode

50e2463

CodingCat closed this Jan 8, 2015

kayousterhout mentioned this pull request Jan 9, 2015

[SPARK-1143] Separate pool tests into their own suite. #3967

Closed

mccheah pushed a commit to mccheah/spark that referenced this pull request Oct 3, 2018

Store/restore ~/.m2 after versioned build (since pom.xml changes) (ap…

cf31e83

…ache#339)

bzhaoopenstack pushed a commit to bzhaoopenstack/spark that referenced this pull request Sep 11, 2019

Merge pull request apache#339 from theopenlab/fix-if-issue

783f656

Fix if issue in keyword contain case

SPARK-1143: refactor TaskSchedulerImplSuite #339

SPARK-1143: refactor TaskSchedulerImplSuite #339

Conversation

CodingCat commented Apr 6, 2014

AmplabJenkins commented Apr 6, 2014

AmplabJenkins commented Apr 6, 2014

AmplabJenkins commented Apr 6, 2014

AmplabJenkins commented Apr 6, 2014

AmplabJenkins commented Apr 6, 2014

AmplabJenkins commented Apr 6, 2014

AmplabJenkins commented Apr 7, 2014

AmplabJenkins commented Apr 7, 2014

AmplabJenkins commented Apr 7, 2014

AmplabJenkins commented Apr 7, 2014

AmplabJenkins commented Apr 7, 2014

AmplabJenkins commented Apr 7, 2014

CodingCat commented Apr 7, 2014

CodingCat commented Apr 11, 2014

kayousterhout commented Apr 11, 2014

CodingCat commented Apr 11, 2014

AmplabJenkins commented Apr 11, 2014

AmplabJenkins commented Apr 11, 2014

AmplabJenkins commented Apr 11, 2014

AmplabJenkins commented Apr 11, 2014

CodingCat commented Apr 11, 2014

kayousterhout commented Apr 12, 2014

CodingCat commented Apr 13, 2014

kayousterhout commented Apr 13, 2014

AmplabJenkins commented May 13, 2014

SparkQA commented Sep 23, 2014

SparkQA commented Sep 23, 2014

SparkQA commented Sep 23, 2014

SparkQA commented Sep 23, 2014

SparkQA commented Sep 23, 2014

SparkQA commented Sep 23, 2014

nchammas commented Sep 23, 2014

SparkQA commented Sep 23, 2014

SparkQA commented Sep 23, 2014

nchammas commented Jan 4, 2015

andrewor14 commented Jan 8, 2015

CodingCat commented Jan 8, 2015

kayousterhout commented Jan 8, 2015

andrewor14 commented Jan 8, 2015

andrewor14 commented Jan 8, 2015

CodingCat commented Jan 8, 2015