You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by erenavsarogullari <gi...@git.apache.org> on 2016/10/23 21:04:33 UTC
[GitHub] spark pull request #15604: Add Pool usage policies test coverage for FIFO & ...
GitHub user erenavsarogullari opened a pull request:
https://github.com/apache/spark/pull/15604
Add Pool usage policies test coverage for FIFO & FAIR Schedulers
## What changes were proposed in this pull request?
The following FIFO & FAIR Schedulers Pool usage cases need to have unit test coverage :
- FIFO Scheduler just uses **root pool** so even if `spark.scheduler.pool` property is set, related pool is not created and `TaskSetManagers` are added to **root pool**.
- FAIR Scheduler uses `default pool` when `spark.scheduler.pool` property is not set. This can be happened when
- `Properties` object is **null**
- `Properties` object is **empty**(`new Properties()`)
- It points **default pool**(`spark.scheduler.pool=default`).
- FAIR Scheduler creates a **new pool** with **default values** when `spark.scheduler.pool` property points **non-existent** pool. This can be happened when **scheduler allocation file** is not set or it does not contain related pool.
## How was this patch tested?
New Unit tests are added.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/erenavsarogullari/spark SPARK-18066
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/15604.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #15604
----
commit 9e588cf33ea3a99b384399a7b444ee7fb9df82d5
Author: erenavsarogullari <er...@gmail.com>
Date: 2016-10-23T20:37:26Z
Add Pool usage policies test coverage for FIFO & FAIR Schedulers
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on the issue:
https://github.com/apache/spark/pull/15604
cc @kayousterhout @markhamstra
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #74410 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74410/testReport)** for PR 15604 at commit [`e9bb2b8`](https://github.com/apache/spark/commit/e9bb2b8c443ea1dbf7412b58babfa2791f969710).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r94504395
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -178,4 +180,97 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
scheduleTaskAndVerifyId(2, rootPool, 6)
scheduleTaskAndVerifyId(3, rootPool, 2)
}
+
+ test("SPARK-18066: FIFO Scheduler just uses root pool") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FIFO, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FIFOSchedulableBuilder(rootPool)
+
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty("spark.scheduler.pool", TEST_POOL)
+
+ // FIFOSchedulableBuilder just uses rootPool so even if properties are set, related pool
+ // (testPool) is not created and TaskSetManagers are added to rootPool
+ schedulableBuilder.addTaskSetManager(taskSetManager0, properties)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, properties)
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) == null)
+ assert(rootPool.schedulableQueue.size == 2)
+ assert(rootPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+ assert(rootPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("SPARK-18066: FAIR Scheduler uses default pool when spark.scheduler.pool property is not " +
+ "set") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ // FAIR Scheduler uses default pool when pool properties are null
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager0, null)
+
+ val defaultPool = rootPool.getSchedulableByName(schedulableBuilder.DEFAULT_POOL_NAME)
+ assert(defaultPool != null)
+ assert(defaultPool.schedulableQueue.size == 1)
+ assert(defaultPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is not set
--- End diff --
Similar to the above, can you eliminate the blank lines here, and change this to "When a task set manager is submitted with spark.scheduler.pool unset, it should be added to the default pool (as above)."
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r94504003
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -178,4 +180,97 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
scheduleTaskAndVerifyId(2, rootPool, 6)
scheduleTaskAndVerifyId(3, rootPool, 2)
}
+
+ test("SPARK-18066: FIFO Scheduler just uses root pool") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FIFO, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FIFOSchedulableBuilder(rootPool)
+
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty("spark.scheduler.pool", TEST_POOL)
+
+ // FIFOSchedulableBuilder just uses rootPool so even if properties are set, related pool
+ // (testPool) is not created and TaskSetManagers are added to rootPool
+ schedulableBuilder.addTaskSetManager(taskSetManager0, properties)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, properties)
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) == null)
+ assert(rootPool.schedulableQueue.size == 2)
+ assert(rootPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+ assert(rootPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("SPARK-18066: FAIR Scheduler uses default pool when spark.scheduler.pool property is not " +
+ "set") {
--- End diff --
nit: +2 spaces here
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72722/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r100646612
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -178,4 +180,97 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
scheduleTaskAndVerifyId(2, rootPool, 6)
scheduleTaskAndVerifyId(3, rootPool, 2)
}
+
+ test("SPARK-18066: FIFO Scheduler just uses root pool") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FIFO, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FIFOSchedulableBuilder(rootPool)
+
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty("spark.scheduler.pool", TEST_POOL)
+
+ // FIFOSchedulableBuilder just uses rootPool so even if properties are set, related pool
+ // (testPool) is not created and TaskSetManagers are added to rootPool
+ schedulableBuilder.addTaskSetManager(taskSetManager0, properties)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, properties)
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) == null)
+ assert(rootPool.schedulableQueue.size == 2)
+ assert(rootPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+ assert(rootPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("SPARK-18066: FAIR Scheduler uses default pool when spark.scheduler.pool property is not " +
+ "set") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ // FAIR Scheduler uses default pool when pool properties are null
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager0, null)
+
+ val defaultPool = rootPool.getSchedulableByName(schedulableBuilder.DEFAULT_POOL_NAME)
+ assert(defaultPool != null)
+ assert(defaultPool.schedulableQueue.size == 1)
+ assert(defaultPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is not set
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager1, new Properties())
+
+ assert(defaultPool.schedulableQueue.size == 2)
+ assert(defaultPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is set as default pool
+ val taskSetManager2 = createTaskSetManager(stageId = 2, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty(schedulableBuilder.FAIR_SCHEDULER_PROPERTIES, schedulableBuilder
+ .DEFAULT_POOL_NAME)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager2, properties)
+
+ assert(defaultPool.schedulableQueue.size == 3)
+ assert(defaultPool.getSchedulableByName(taskSetManager2.name) === taskSetManager2)
+ }
+
+ test("SPARK-18066: FAIR Scheduler creates a new pool when spark.scheduler.pool property points " +
+ "non-existent") {
--- End diff --
In this case, at least, i think we can inform the user by emphasizing that the related pool does not exist and it is created with **default settings**.
Our current logging just points pool creation. WDYT?
Also Pool `var weight / minShare` have been addressed ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #72736 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72736/testReport)** for PR 15604 at commit [`9eafad5`](https://github.com/apache/spark/commit/9eafad502f67282d0bd9670164c8ca88e90c4d3c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the issue:
https://github.com/apache/spark/pull/15604
Jenkins retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on the issue:
https://github.com/apache/spark/pull/15604
Hi @kayousterhout and @squito,
This PR is ready to re-review / merge if it is possible.
All feedbacks are welcome and thanks in advance ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r94504259
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -178,4 +180,97 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
scheduleTaskAndVerifyId(2, rootPool, 6)
scheduleTaskAndVerifyId(3, rootPool, 2)
}
+
+ test("SPARK-18066: FIFO Scheduler just uses root pool") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FIFO, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FIFOSchedulableBuilder(rootPool)
+
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty("spark.scheduler.pool", TEST_POOL)
+
+ // FIFOSchedulableBuilder just uses rootPool so even if properties are set, related pool
+ // (testPool) is not created and TaskSetManagers are added to rootPool
+ schedulableBuilder.addTaskSetManager(taskSetManager0, properties)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, properties)
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) == null)
+ assert(rootPool.schedulableQueue.size == 2)
+ assert(rootPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+ assert(rootPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("SPARK-18066: FAIR Scheduler uses default pool when spark.scheduler.pool property is not " +
+ "set") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ // FAIR Scheduler uses default pool when pool properties are null
--- End diff --
super nit with commenting, but can you eliminate the blank lines on 219 and 221 (which don't help readability) and change this comment to something like
"Submit a new task set manager with pool properties set to null. This should result in the task set manager getting added to the default pool."
(it's helpful in test comments to be clear about what the test is doing itself, versus the expected behavior it's verifying)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #72722 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72722/testReport)** for PR 15604 at commit [`84513b1`](https://github.com/apache/spark/commit/84513b1f90f045963a1d8e0e999806e4e716fa50).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on the issue:
https://github.com/apache/spark/pull/15604
Hi @kayousterhout and @squito,
Last comments are addressed so this PR is ready to re-review / merge if it is possible.
Thanks in advance ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on the issue:
https://github.com/apache/spark/pull/15604
Hi @kayousterhout and @squito,
This PR is ready to re-review.
All feedbacks are welcome and thanks in advance ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #72544 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72544/testReport)** for PR 15604 at commit [`f1cfc9e`](https://github.com/apache/spark/commit/f1cfc9e24a2c08bb96e706c3ee2635aa6fe61bbb).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72792/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r106280052
--- Diff: core/src/main/scala/org/apache/spark/scheduler/SchedulableBuilder.scala ---
@@ -191,7 +191,10 @@ private[spark] class FairSchedulableBuilder(val rootPool: Pool, conf: SparkConf)
parentPool = new Pool(poolName, DEFAULT_SCHEDULING_MODE,
DEFAULT_MINIMUM_SHARE, DEFAULT_WEIGHT)
rootPool.addSchedulable(parentPool)
- logInfo("Created pool: %s, schedulingMode: %s, minShare: %d, weight: %d".format(
+ logWarning(("An Unconfigured pool is found. It is built with default configuration. " +
--- End diff --
Sure, it is updated ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/15604
hmm, so I'm thinking about this again, along with your other pr https://github.com/apache/spark/pull/15237
and it got me thinking -- there also aren't any tests for a mix of FIFO and FAIR (at least, I don't see it in "Nested Pool Test" in PoolSuite). While you are doing this, would you like to add some tests for that as well? (to be perfectly honest, I need to spend a little bit of time thinking about what that behavior should be anyway, but I will have to do that a little later.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/74412/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r94503954
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -178,4 +180,97 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
scheduleTaskAndVerifyId(2, rootPool, 6)
scheduleTaskAndVerifyId(3, rootPool, 2)
}
+
+ test("SPARK-18066: FIFO Scheduler just uses root pool") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FIFO, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FIFOSchedulableBuilder(rootPool)
+
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty("spark.scheduler.pool", TEST_POOL)
+
+ // FIFOSchedulableBuilder just uses rootPool so even if properties are set, related pool
+ // (testPool) is not created and TaskSetManagers are added to rootPool
--- End diff --
Can you re-phrase this as what *should* happen, which makes these comments easier to read? e.g.
"When task sets are submitted, they should be added to the root pool, and no additional pools should be created (even though there's a configured default pool)."
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r94497574
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -178,4 +180,97 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
scheduleTaskAndVerifyId(2, rootPool, 6)
scheduleTaskAndVerifyId(3, rootPool, 2)
}
+
+ test("SPARK-18066: FIFO Scheduler just uses root pool") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FIFO, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FIFOSchedulableBuilder(rootPool)
+
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty("spark.scheduler.pool", TEST_POOL)
+
+ // FIFOSchedulableBuilder just uses rootPool so even if properties are set, related pool
+ // (testPool) is not created and TaskSetManagers are added to rootPool
+ schedulableBuilder.addTaskSetManager(taskSetManager0, properties)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, properties)
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) == null)
+ assert(rootPool.schedulableQueue.size == 2)
+ assert(rootPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+ assert(rootPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("SPARK-18066: FAIR Scheduler uses default pool when spark.scheduler.pool property is not " +
+ "set") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ // FAIR Scheduler uses default pool when pool properties are null
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager0, null)
+
+ val defaultPool = rootPool.getSchedulableByName(schedulableBuilder.DEFAULT_POOL_NAME)
+ assert(defaultPool != null)
+ assert(defaultPool.schedulableQueue.size == 1)
+ assert(defaultPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is not set
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager1, new Properties())
+
+ assert(defaultPool.schedulableQueue.size == 2)
+ assert(defaultPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is set as default pool
--- End diff --
I'm all for more tests, but this final case here seems pretty unnecessary
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r106225480
--- Diff: core/src/main/scala/org/apache/spark/scheduler/SchedulableBuilder.scala ---
@@ -191,7 +191,10 @@ private[spark] class FairSchedulableBuilder(val rootPool: Pool, conf: SparkConf)
parentPool = new Pool(poolName, DEFAULT_SCHEDULING_MODE,
DEFAULT_MINIMUM_SHARE, DEFAULT_WEIGHT)
rootPool.addSchedulable(parentPool)
- logInfo("Created pool: %s, schedulingMode: %s, minShare: %d, weight: %d".format(
+ logWarning(("An Unconfigured pool is found. It is built with default configuration. " +
--- End diff --
It's not totally clear what the first part of this error message means. How about changing the message to:
```
logWarning(s"A job was submitted with scheduler pool $poolName, which has not been configured." +
"This can happen when the file that pools are read from isn't set, or when that file " +
s"doesn't contain $poolName. Created $poolName with default configuration (" +
s"schedulingMode: $DEFAULT_SCHEDULING_MODE, minShare: $DEFAULT_MINIMUM_SHARE, " +
s"weight: $DEFAULT_WEIGHT)")
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on the issue:
https://github.com/apache/spark/pull/15604
Jenkins, retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72544/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/70832/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r94505633
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -178,4 +180,97 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
scheduleTaskAndVerifyId(2, rootPool, 6)
scheduleTaskAndVerifyId(3, rootPool, 2)
}
+
+ test("SPARK-18066: FIFO Scheduler just uses root pool") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FIFO, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FIFOSchedulableBuilder(rootPool)
+
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty("spark.scheduler.pool", TEST_POOL)
+
+ // FIFOSchedulableBuilder just uses rootPool so even if properties are set, related pool
+ // (testPool) is not created and TaskSetManagers are added to rootPool
+ schedulableBuilder.addTaskSetManager(taskSetManager0, properties)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, properties)
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) == null)
+ assert(rootPool.schedulableQueue.size == 2)
+ assert(rootPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+ assert(rootPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("SPARK-18066: FAIR Scheduler uses default pool when spark.scheduler.pool property is not " +
+ "set") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ // FAIR Scheduler uses default pool when pool properties are null
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager0, null)
+
+ val defaultPool = rootPool.getSchedulableByName(schedulableBuilder.DEFAULT_POOL_NAME)
+ assert(defaultPool != null)
+ assert(defaultPool.schedulableQueue.size == 1)
+ assert(defaultPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is not set
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager1, new Properties())
+
+ assert(defaultPool.schedulableQueue.size == 2)
+ assert(defaultPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is set as default pool
+ val taskSetManager2 = createTaskSetManager(stageId = 2, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty(schedulableBuilder.FAIR_SCHEDULER_PROPERTIES, schedulableBuilder
+ .DEFAULT_POOL_NAME)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager2, properties)
+
+ assert(defaultPool.schedulableQueue.size == 3)
+ assert(defaultPool.getSchedulableByName(taskSetManager2.name) === taskSetManager2)
+ }
+
+ test("SPARK-18066: FAIR Scheduler creates a new pool when spark.scheduler.pool property points " +
+ "non-existent") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) == null)
+
+ val taskSetManager = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty(schedulableBuilder.FAIR_SCHEDULER_PROPERTIES, TEST_POOL)
+
+ // FAIR Scheduler creates a new pool with default values when spark.scheduler.pool property
+ // points non-existent pool. This can be happened when scheduler allocation file is not set or
+ // it does not contain related pool
--- End diff --
can you clean up this comment a bit too?
"The fair scheduler should create a new pool with default values when spark.scheduler.pool points to a pool that doesn't exist yet (this can happen when the file that pools are read from isn't set, or when that file doesn't contain the pool name specified by spark.scheduler.pool)."
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on the issue:
https://github.com/apache/spark/pull/15604
The following unit tests look failed due to timeout and _irrelevant_ so triggered again.
**1-** org.apache.spark.storage.BlockManagerProactiveReplicationSuite.proactive block replication - 5 replicas - 4 block manager deletions
**Error Message:** org.scalatest.exceptions.TestFailedDueToTimeoutException: The code passed to eventually never returned normally. Attempted 493 times over 5.007521253999999 seconds. Last failure message: 4 did not equal 5.
**2-** org.apache.spark.deploy.SparkSubmitSuite.includes jars passed in through --packages
**Error Message:** org.scalatest.exceptions.TestFailedDueToTimeoutException: The code passed to failAfter did not complete within 60 seconds.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on the issue:
https://github.com/apache/spark/pull/15604
Jenkins, retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on the issue:
https://github.com/apache/spark/pull/15604
Hi @kayousterhout @markhamstra @squito,
This PR aims to extend Pool usage policies unit test coverage and is also ready for review.
Thanks and all feedbacks are welcome in advance ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #74412 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74412/testReport)** for PR 15604 at commit [`91f3814`](https://github.com/apache/spark/commit/91f3814e4fa3fd6809a0353d6bc95b44e99f2010).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on the issue:
https://github.com/apache/spark/pull/15604
Many thanks @kayousterhout for reviewing this again. Last comment has also been addressed.
Also the following Jira has been created for previous second UT failure.
https://issues.apache.org/jira/browse/SPARK-19964
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #74413 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74413/testReport)** for PR 15604 at commit [`17e11f0`](https://github.com/apache/spark/commit/17e11f0c56e2a581766c06bd52695c2b05bcfcb2).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #72725 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72725/testReport)** for PR 15604 at commit [`f84abe7`](https://github.com/apache/spark/commit/f84abe74dd4426cf520b38f6380dff693dacc92b).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #74413 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74413/testReport)** for PR 15604 at commit [`17e11f0`](https://github.com/apache/spark/commit/17e11f0c56e2a581766c06bd52695c2b05bcfcb2).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r105567716
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -201,12 +202,96 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
verifyPool(rootPool, "pool_with_surrounded_whitespace", 3, 2, FAIR)
}
+ /**
+ * spark.scheduler.pool property should be ignored for the FIFO scheduler,
+ * because pools are only needed for fair scheduling.
+ */
+ test("FIFO scheduler uses root pool and not spark.scheduler.pool property") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FIFO, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FIFOSchedulableBuilder(rootPool)
+
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty("spark.scheduler.pool", TEST_POOL)
+
+ // When FIFO Scheduler is used and task sets are submitted, they should be added to
+ // the root pool, and no additional pools should be created
+ // (even though there's a configured default pool).
+ schedulableBuilder.addTaskSetManager(taskSetManager0, properties)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, properties)
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) === null)
+ assert(rootPool.schedulableQueue.size === 2)
+ assert(rootPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+ assert(rootPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("FAIR Scheduler uses default pool when spark.scheduler.pool property is not set") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ // Submit a new task set manager with pool properties set to null. This should result
+ // in the task set manager getting added to the default pool.
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ schedulableBuilder.addTaskSetManager(taskSetManager0, null)
+
+ val defaultPool = rootPool.getSchedulableByName(schedulableBuilder.DEFAULT_POOL_NAME)
+ assert(defaultPool !== null)
+ assert(defaultPool.schedulableQueue.size === 1)
+ assert(defaultPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+
+ // When a task set manager is submitted with spark.scheduler.pool unset, it should be added to
+ // the default pool (as above).
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, new Properties())
+
+ assert(defaultPool.schedulableQueue.size === 2)
+ assert(defaultPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("FAIR Scheduler creates a new pool when spark.scheduler.pool property points to " +
+ "a non-existent pool") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) === null)
+
+ val taskSetManager = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty(schedulableBuilder.FAIR_SCHEDULER_PROPERTIES, TEST_POOL)
+
+ // The fair scheduler should create a new pool with default values when spark.scheduler.pool
+ // points to a pool that doesn't exist yet (this can happen when the file that pools are read
--- End diff --
Currently, this behavior is **not** highlighted via logs and we just points the pool creation as
`Created pool: ..., schedulingMode: ..., minShare: ..., weight: ...` so i agree it can be useful for the user and it is addressed with the latest commits.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on the issue:
https://github.com/apache/spark/pull/15604
@kayousterhout thanks for query this PR. I will be submitting this and the other one( #15326) once #16813 is merged.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/74413/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/74575/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r94505365
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -178,4 +180,97 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
scheduleTaskAndVerifyId(2, rootPool, 6)
scheduleTaskAndVerifyId(3, rootPool, 2)
}
+
+ test("SPARK-18066: FIFO Scheduler just uses root pool") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FIFO, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FIFOSchedulableBuilder(rootPool)
+
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty("spark.scheduler.pool", TEST_POOL)
+
+ // FIFOSchedulableBuilder just uses rootPool so even if properties are set, related pool
+ // (testPool) is not created and TaskSetManagers are added to rootPool
+ schedulableBuilder.addTaskSetManager(taskSetManager0, properties)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, properties)
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) == null)
+ assert(rootPool.schedulableQueue.size == 2)
+ assert(rootPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+ assert(rootPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("SPARK-18066: FAIR Scheduler uses default pool when spark.scheduler.pool property is not " +
+ "set") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ // FAIR Scheduler uses default pool when pool properties are null
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager0, null)
+
+ val defaultPool = rootPool.getSchedulableByName(schedulableBuilder.DEFAULT_POOL_NAME)
+ assert(defaultPool != null)
+ assert(defaultPool.schedulableQueue.size == 1)
+ assert(defaultPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is not set
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager1, new Properties())
+
+ assert(defaultPool.schedulableQueue.size == 2)
+ assert(defaultPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is set as default pool
+ val taskSetManager2 = createTaskSetManager(stageId = 2, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty(schedulableBuilder.FAIR_SCHEDULER_PROPERTIES, schedulableBuilder
+ .DEFAULT_POOL_NAME)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager2, properties)
+
+ assert(defaultPool.schedulableQueue.size == 3)
+ assert(defaultPool.getSchedulableByName(taskSetManager2.name) === taskSetManager2)
+ }
+
+ test("SPARK-18066: FAIR Scheduler creates a new pool when spark.scheduler.pool property points " +
+ "non-existent") {
--- End diff --
I tracked this down and looks like it was changed [back in 2013](https://github.com/apache/spark/commit/5892393140eb024a32585b6d5b51146ddde8f63a#diff-30ccb25ca46ef204d2446b03ae4f1117R103). @xiajunluan do you remember the motivation for this?
I'm torn because in general I prefer failing fast (e.g., this is better in the case where someone has a small typo they might not notice, and is intending to use a configured pool). On the other hand, I'm pretty sure I've abused this behavior in the past to avoid configuring pools when I want a bunch of equally weighted pools. If others are doing that, we shouldn't change the behavior (although I think that, in that case, we should add a big warning / future-deprecation message).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/15604
Jenkins, ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r94499675
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -178,4 +180,97 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
scheduleTaskAndVerifyId(2, rootPool, 6)
scheduleTaskAndVerifyId(3, rootPool, 2)
}
+
+ test("SPARK-18066: FIFO Scheduler just uses root pool") {
--- End diff --
super nit: the jira doesn't really say anything other than "add these tests", so not much point in listing it here, just name test "FIFO Scheduler just uses root pool".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72736/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #72722 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72722/testReport)** for PR 15604 at commit [`84513b1`](https://github.com/apache/spark/commit/84513b1f90f045963a1d8e0e999806e4e716fa50).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #74616 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74616/testReport)** for PR 15604 at commit [`f13ad3d`](https://github.com/apache/spark/commit/f13ad3de7521ec6e65a9283aa6c6f37a903cead5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/74410/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #74575 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74575/testReport)** for PR 15604 at commit [`17e11f0`](https://github.com/apache/spark/commit/17e11f0c56e2a581766c06bd52695c2b05bcfcb2).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #74412 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74412/testReport)** for PR 15604 at commit [`91f3814`](https://github.com/apache/spark/commit/91f3814e4fa3fd6809a0353d6bc95b44e99f2010).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r102831022
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -201,12 +202,96 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
verifyPool(rootPool, "pool_with_surrounded_whitespace", 3, 2, FAIR)
}
+ /**
+ * spark.scheduler.pool property should be ignored for the FIFO scheduler,
+ * because pools are only needed for fair scheduling.
+ */
+ test("FIFO scheduler uses root pool and not spark.scheduler.pool property") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FIFO, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FIFOSchedulableBuilder(rootPool)
+
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty("spark.scheduler.pool", TEST_POOL)
+
+ // When FIFO Scheduler is used and task sets are submitted, they should be added to
+ // the root pool, and no additional pools should be created
+ // (even though there's a configured default pool).
+ schedulableBuilder.addTaskSetManager(taskSetManager0, properties)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, properties)
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) === null)
+ assert(rootPool.schedulableQueue.size === 2)
+ assert(rootPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+ assert(rootPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("FAIR Scheduler uses default pool when spark.scheduler.pool property is not set") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ // Submit a new task set manager with pool properties set to null. This should result
+ // in the task set manager getting added to the default pool.
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ schedulableBuilder.addTaskSetManager(taskSetManager0, null)
+
+ val defaultPool = rootPool.getSchedulableByName(schedulableBuilder.DEFAULT_POOL_NAME)
+ assert(defaultPool !== null)
+ assert(defaultPool.schedulableQueue.size === 1)
+ assert(defaultPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+
+ // When a task set manager is submitted with spark.scheduler.pool unset, it should be added to
+ // the default pool (as above).
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, new Properties())
+
+ assert(defaultPool.schedulableQueue.size === 2)
+ assert(defaultPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("FAIR Scheduler creates a new pool when spark.scheduler.pool property points to " +
+ "a non-existent pool") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) === null)
+
+ val taskSetManager = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty(schedulableBuilder.FAIR_SCHEDULER_PROPERTIES, TEST_POOL)
+
+ // The fair scheduler should create a new pool with default values when spark.scheduler.pool
+ // points to a pool that doesn't exist yet (this can happen when the file that pools are read
--- End diff --
JOOC does this result in a warning being printed to the user? (I know you can't test for that -- but it does seem like useful behavior to have)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #74408 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74408/testReport)** for PR 15604 at commit [`4793ea8`](https://github.com/apache/spark/commit/4793ea8590234fd3656df224d206f075ce12b229).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #70832 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70832/testReport)** for PR 15604 at commit [`f630cff`](https://github.com/apache/spark/commit/f630cffefa6fb67bfd454bcf3d456045cedaba02).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the issue:
https://github.com/apache/spark/pull/15604
@erenavsarogullari please file a JIRA when you see test failures instead of ignoring them. I updated https://issues.apache.org/jira/browse/SPARK-19803 for the first failure, but please file a JIRA for the second one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r94503677
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -178,4 +180,97 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
scheduleTaskAndVerifyId(2, rootPool, 6)
scheduleTaskAndVerifyId(3, rootPool, 2)
}
+
+ test("SPARK-18066: FIFO Scheduler just uses root pool") {
--- End diff --
+1. Also how about "FIFO scheduler uses root pool and not spark.scheduler.pool" and then adding a comment at the top of the test that says something like "spark.scheduler.pool should be ignored for the FIFO scheduler, because pools are only needed for fair scheduling"? Took me a minute to figure out what was going on here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #74616 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74616/testReport)** for PR 15604 at commit [`f13ad3d`](https://github.com/apache/spark/commit/f13ad3de7521ec6e65a9283aa6c6f37a903cead5).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the issue:
https://github.com/apache/spark/pull/15604
Ok thanks for the update!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #74618 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74618/testReport)** for PR 15604 at commit [`b7f2629`](https://github.com/apache/spark/commit/b7f2629b554116c0a6c3d0674e717dc8c2bf77e3).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #72792 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72792/testReport)** for PR 15604 at commit [`d68bb0c`](https://github.com/apache/spark/commit/d68bb0cfc18074c76d62ebabdd504e24828258da).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r106227249
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -201,12 +202,96 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
verifyPool(rootPool, "pool_with_surrounded_whitespace", 3, 2, FAIR)
}
+ /**
+ * spark.scheduler.pool property should be ignored for the FIFO scheduler,
+ * because pools are only needed for fair scheduling.
+ */
+ test("FIFO scheduler uses root pool and not spark.scheduler.pool property") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FIFO, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FIFOSchedulableBuilder(rootPool)
+
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty("spark.scheduler.pool", TEST_POOL)
+
+ // When FIFO Scheduler is used and task sets are submitted, they should be added to
+ // the root pool, and no additional pools should be created
+ // (even though there's a configured default pool).
+ schedulableBuilder.addTaskSetManager(taskSetManager0, properties)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, properties)
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) === null)
+ assert(rootPool.schedulableQueue.size === 2)
+ assert(rootPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+ assert(rootPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("FAIR Scheduler uses default pool when spark.scheduler.pool property is not set") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ // Submit a new task set manager with pool properties set to null. This should result
+ // in the task set manager getting added to the default pool.
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ schedulableBuilder.addTaskSetManager(taskSetManager0, null)
+
+ val defaultPool = rootPool.getSchedulableByName(schedulableBuilder.DEFAULT_POOL_NAME)
+ assert(defaultPool !== null)
+ assert(defaultPool.schedulableQueue.size === 1)
+ assert(defaultPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+
+ // When a task set manager is submitted with spark.scheduler.pool unset, it should be added to
+ // the default pool (as above).
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, new Properties())
+
+ assert(defaultPool.schedulableQueue.size === 2)
+ assert(defaultPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("FAIR Scheduler creates a new pool when spark.scheduler.pool property points to " +
+ "a non-existent pool") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) === null)
+
+ val taskSetManager = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty(schedulableBuilder.FAIR_SCHEDULER_PROPERTIES, TEST_POOL)
+
+ // The fair scheduler should create a new pool with default values when spark.scheduler.pool
+ // points to a pool that doesn't exist yet (this can happen when the file that pools are read
--- End diff --
Thanks for updating this!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72725/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #70832 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70832/testReport)** for PR 15604 at commit [`f630cff`](https://github.com/apache/spark/commit/f630cffefa6fb67bfd454bcf3d456045cedaba02).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on the issue:
https://github.com/apache/spark/pull/15604
Hi @kayousterhout and @squito,
Firstly, many thanks for the review. All comments are addressed. This is ready to re-review ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #72792 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72792/testReport)** for PR 15604 at commit [`d68bb0c`](https://github.com/apache/spark/commit/d68bb0cfc18074c76d62ebabdd504e24828258da).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r94504436
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -178,4 +180,97 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
scheduleTaskAndVerifyId(2, rootPool, 6)
scheduleTaskAndVerifyId(3, rootPool, 2)
}
+
+ test("SPARK-18066: FIFO Scheduler just uses root pool") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FIFO, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FIFOSchedulableBuilder(rootPool)
+
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty("spark.scheduler.pool", TEST_POOL)
+
+ // FIFOSchedulableBuilder just uses rootPool so even if properties are set, related pool
+ // (testPool) is not created and TaskSetManagers are added to rootPool
+ schedulableBuilder.addTaskSetManager(taskSetManager0, properties)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, properties)
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) == null)
+ assert(rootPool.schedulableQueue.size == 2)
+ assert(rootPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+ assert(rootPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("SPARK-18066: FAIR Scheduler uses default pool when spark.scheduler.pool property is not " +
+ "set") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ // FAIR Scheduler uses default pool when pool properties are null
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager0, null)
+
+ val defaultPool = rootPool.getSchedulableByName(schedulableBuilder.DEFAULT_POOL_NAME)
+ assert(defaultPool != null)
+ assert(defaultPool.schedulableQueue.size == 1)
+ assert(defaultPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is not set
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager1, new Properties())
+
+ assert(defaultPool.schedulableQueue.size == 2)
+ assert(defaultPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is set as default pool
--- End diff --
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the issue:
https://github.com/apache/spark/pull/15604
Ok thanks for the update!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/74408/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/74616/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #72544 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72544/testReport)** for PR 15604 at commit [`f1cfc9e`](https://github.com/apache/spark/commit/f1cfc9e24a2c08bb96e706c3ee2635aa6fe61bbb).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on the issue:
https://github.com/apache/spark/pull/15604
Jenkins, retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the issue:
https://github.com/apache/spark/pull/15604
Thanks for your work on this @erenavsarogullari. I've merged this into master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r94499385
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -178,4 +180,97 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
scheduleTaskAndVerifyId(2, rootPool, 6)
scheduleTaskAndVerifyId(3, rootPool, 2)
}
+
+ test("SPARK-18066: FIFO Scheduler just uses root pool") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FIFO, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FIFOSchedulableBuilder(rootPool)
+
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty("spark.scheduler.pool", TEST_POOL)
+
+ // FIFOSchedulableBuilder just uses rootPool so even if properties are set, related pool
+ // (testPool) is not created and TaskSetManagers are added to rootPool
+ schedulableBuilder.addTaskSetManager(taskSetManager0, properties)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, properties)
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) == null)
+ assert(rootPool.schedulableQueue.size == 2)
+ assert(rootPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+ assert(rootPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("SPARK-18066: FAIR Scheduler uses default pool when spark.scheduler.pool property is not " +
+ "set") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ // FAIR Scheduler uses default pool when pool properties are null
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager0, null)
+
+ val defaultPool = rootPool.getSchedulableByName(schedulableBuilder.DEFAULT_POOL_NAME)
+ assert(defaultPool != null)
+ assert(defaultPool.schedulableQueue.size == 1)
+ assert(defaultPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is not set
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager1, new Properties())
+
+ assert(defaultPool.schedulableQueue.size == 2)
+ assert(defaultPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is set as default pool
+ val taskSetManager2 = createTaskSetManager(stageId = 2, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty(schedulableBuilder.FAIR_SCHEDULER_PROPERTIES, schedulableBuilder
+ .DEFAULT_POOL_NAME)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager2, properties)
+
+ assert(defaultPool.schedulableQueue.size == 3)
+ assert(defaultPool.getSchedulableByName(taskSetManager2.name) === taskSetManager2)
+ }
+
+ test("SPARK-18066: FAIR Scheduler creates a new pool when spark.scheduler.pool property points " +
+ "non-existent") {
--- End diff --
minor nit: rename to "FAIR Scheduler creates a new pool when spark.scheduler.pool property points to a non-existent pool"
bigger question:
@markhamstra @kayousterhout Is this really the desired behavior? Or is there a bug -- should it fail fast? It looks like [this behavior was intentional](https://github.com/apache/spark/blob/b67b35f76b684c5176dc683e7491fd01b43f4467/core/src/main/scala/org/apache/spark/scheduler/SchedulableBuilder.scala#L143-L145). Then again, that comment makes it sound like the user is going to directly modify the `weight` and `minShare` of the constructed pool, but though [they are `var`s](https://github.com/apache/spark/blob/b67b35f76b684c5176dc683e7491fd01b43f4467/core/src/main/scala/org/apache/spark/scheduler/Pool.scala#L40-L41), its `private [spark]` and there isn't anything else exposing it to the user.
I prefer fail-fast behavior, but it seems like fair scheduler configuration prefers to assume defaults when there is misconfiguration. If you think this the right behavior, than there is probably some minor cleanup to do in Pool & SchedulableBuilder to clarify.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #74408 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74408/testReport)** for PR 15604 at commit [`4793ea8`](https://github.com/apache/spark/commit/4793ea8590234fd3656df224d206f075ce12b229).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r94670256
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -178,4 +180,97 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
scheduleTaskAndVerifyId(2, rootPool, 6)
scheduleTaskAndVerifyId(3, rootPool, 2)
}
+
+ test("SPARK-18066: FIFO Scheduler just uses root pool") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FIFO, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FIFOSchedulableBuilder(rootPool)
+
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty("spark.scheduler.pool", TEST_POOL)
+
+ // FIFOSchedulableBuilder just uses rootPool so even if properties are set, related pool
+ // (testPool) is not created and TaskSetManagers are added to rootPool
+ schedulableBuilder.addTaskSetManager(taskSetManager0, properties)
+ schedulableBuilder.addTaskSetManager(taskSetManager1, properties)
+
+ assert(rootPool.getSchedulableByName(TEST_POOL) == null)
+ assert(rootPool.schedulableQueue.size == 2)
+ assert(rootPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+ assert(rootPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+ }
+
+ test("SPARK-18066: FAIR Scheduler uses default pool when spark.scheduler.pool property is not " +
+ "set") {
+ sc = new SparkContext("local", "PoolSuite")
+ val taskScheduler = new TaskSchedulerImpl(sc)
+
+ val rootPool = new Pool("", SchedulingMode.FAIR, initMinShare = 0, initWeight = 0)
+ val schedulableBuilder = new FairSchedulableBuilder(rootPool, sc.conf)
+ schedulableBuilder.buildPools()
+
+ // FAIR Scheduler uses default pool when pool properties are null
+ val taskSetManager0 = createTaskSetManager(stageId = 0, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager0, null)
+
+ val defaultPool = rootPool.getSchedulableByName(schedulableBuilder.DEFAULT_POOL_NAME)
+ assert(defaultPool != null)
+ assert(defaultPool.schedulableQueue.size == 1)
+ assert(defaultPool.getSchedulableByName(taskSetManager0.name) === taskSetManager0)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is not set
+ val taskSetManager1 = createTaskSetManager(stageId = 1, numTasks = 1, taskScheduler)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager1, new Properties())
+
+ assert(defaultPool.schedulableQueue.size == 2)
+ assert(defaultPool.getSchedulableByName(taskSetManager1.name) === taskSetManager1)
+
+ // FAIR Scheduler uses default pool when spark.scheduler.pool property is set as default pool
+ val taskSetManager2 = createTaskSetManager(stageId = 2, numTasks = 1, taskScheduler)
+
+ val properties = new Properties()
+ properties.setProperty(schedulableBuilder.FAIR_SCHEDULER_PROPERTIES, schedulableBuilder
+ .DEFAULT_POOL_NAME)
+
+ schedulableBuilder.addTaskSetManager(taskSetManager2, properties)
+
+ assert(defaultPool.schedulableQueue.size == 3)
+ assert(defaultPool.getSchedulableByName(taskSetManager2.name) === taskSetManager2)
+ }
+
+ test("SPARK-18066: FAIR Scheduler creates a new pool when spark.scheduler.pool property points " +
+ "non-existent") {
--- End diff --
yeah I prefer failing fast for the same reason. But I guess we can't actually change this, since there is a valid use case for this behavior. We don't really know if others are using this, and I don't think we should change it under them in the next release.
In any case, I still think we should also change `Pool.weight` and `Pool.minShare` to `val`s.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #72725 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72725/testReport)** for PR 15604 at commit [`f84abe7`](https://github.com/apache/spark/commit/f84abe74dd4426cf520b38f6380dff693dacc92b).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/15604#discussion_r102829684
--- Diff: core/src/test/scala/org/apache/spark/scheduler/PoolSuite.scala ---
@@ -31,16 +31,17 @@ class PoolSuite extends SparkFunSuite with LocalSparkContext {
val LOCAL = "local"
val APP_NAME = "PoolSuite"
val SCHEDULER_ALLOCATION_FILE_PROPERTY = "spark.scheduler.allocation.file"
+ val TEST_POOL = "testPool"
- def createTaskSetManager(stageId: Int, numTasks: Int, taskScheduler: TaskSchedulerImpl)
+ private def createTaskSetManager(stageId: Int, numTasks: Int, taskScheduler: TaskSchedulerImpl)
--- End diff --
It's not necessary to make test functions private -- they're not accessible anyway (and we don't do this elsewhere in tests)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #74618 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74618/testReport)** for PR 15604 at commit [`b7f2629`](https://github.com/apache/spark/commit/b7f2629b554116c0a6c3d0674e717dc8c2bf77e3).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #74410 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74410/testReport)** for PR 15604 at commit [`e9bb2b8`](https://github.com/apache/spark/commit/e9bb2b8c443ea1dbf7412b58babfa2791f969710).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the issue:
https://github.com/apache/spark/pull/15604
@erenavsarogullari what's the status with this PR? I know you have at least one other fair scheduler PR outstanding, and it makes reviews easier to minimize the number of outstanding things (and helps avoid merge conflicts).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15604
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by erenavsarogullari <gi...@git.apache.org>.
Github user erenavsarogullari commented on the issue:
https://github.com/apache/spark/pull/15604
build failure looks unrelated (due to timeout at KafkaSourceSuite level)
Jenkins, retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage polic...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/15604
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #74575 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74575/testReport)** for PR 15604 at commit [`17e11f0`](https://github.com/apache/spark/commit/17e11f0c56e2a581766c06bd52695c2b05bcfcb2).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15604: [SPARK-18066] [CORE] [TESTS] Add Pool usage policies tes...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15604
**[Test build #72736 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72736/testReport)** for PR 15604 at commit [`9eafad5`](https://github.com/apache/spark/commit/9eafad502f67282d0bd9670164c8ca88e90c4d3c).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org