You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by njwhite <gi...@git.apache.org> on 2016/05/06 09:45:53 UTC
[GitHub] spark pull request: [SPARK-15176][Core] Add maxShares setting to P...
GitHub user njwhite opened a pull request:
https://github.com/apache/spark/pull/12951
[SPARK-15176][Core] Add maxShares setting to Pools
## What changes were proposed in this pull request?
Help guarantee resource availablity by (hierarchically) limiting the amount of tasks a given pool can run. The maximum number of tasks for a given pool can be configured by the allocation XML file, and child pools are limited to at most the number of tasks of their parent.
## How was this patch tested?
Unit tests run and new unit tests added for functionality.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/njwhite/spark feature/pool
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12951.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 #12951
----
commit 1b046be04d18e19be33078e2b9f5e26ac8e6aa67
Author: Nick White <nw...@palantir.com>
Date: 2016-05-06T09:42:18Z
[SPARK-15176][Core] Add maxShares setting to Pools
Help guarantee resource availablity by (hierarchically) limiting the amount of
tasks a given pool can run.
----
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-220379290
I commented on the JIRA.
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-220742176
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59030/
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12951
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60036/
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pool...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/12951#discussion_r65433748
--- Diff: core/src/test/scala/org/apache/spark/scheduler/TaskSetManagerSuite.scala ---
@@ -316,6 +316,36 @@ class TaskSetManagerSuite extends SparkFunSuite with LocalSparkContext with Logg
assert(manager.resourceOffer("exec2", "host2", ANY).get.index === 3)
}
+ test("Scheduler respects maxRunningTasks setting of its pool") {
+ sc = new SparkContext("local", "test")
+ val sched = new FakeTaskScheduler(sc, ("exec1", "host1"))
+ val taskSet = FakeTask.createTaskSet(5)
+ val clock = new ManualClock
+ val manager = new TaskSetManager(sched, taskSet, MAX_TASK_FAILURES, clock)
+ manager.parent = new Pool("pool", SchedulingMode.FAIR, 0, 2, 0)
+
+ // pool has two tasks...
+ assert(manager.resourceOffer("exec1", "host1", ANY).get.index === 0)
+ assert(manager.resourceOffer("exec1", "host1", ANY).get.index === 1)
+
+ // but not three!
+ assert(manager.resourceOffer("exec1", "host1", ANY).isEmpty)
+
--- End diff --
The test case should include scheduling another taskset to a different pool which does not share the limitation, and making sure it can still schedule tasks even when the first task set gets limited.
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/12951#discussion_r63911572
--- Diff: core/src/main/scala/org/apache/spark/scheduler/SchedulableBuilder.scala ---
@@ -90,10 +92,16 @@ private[spark] class FairSchedulableBuilder(val rootPool: Pool, conf: SparkConf)
private def buildDefaultPool() {
if (rootPool.getSchedulableByName(DEFAULT_POOL_NAME) == null) {
val pool = new Pool(DEFAULT_POOL_NAME, DEFAULT_SCHEDULING_MODE,
- DEFAULT_MINIMUM_SHARE, DEFAULT_WEIGHT)
+ DEFAULT_MINIMUM_SHARE, DEFAULT_MAXIMUM_SHARE, DEFAULT_WEIGHT)
rootPool.addSchedulable(pool)
- logInfo("Created default pool %s, schedulingMode: %s, minShare: %d, weight: %d".format(
- DEFAULT_POOL_NAME, DEFAULT_SCHEDULING_MODE, DEFAULT_MINIMUM_SHARE, DEFAULT_WEIGHT))
+ logInfo(
--- End diff --
as long as you're touching this, switch to using string interpolation. (eg. `s"Created default pool $DEFAULT_POOL_NAME, ...`). Also since this is repeated a few times, you might just add a helper `logPoolCreated(pool)` or something.
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pool...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/12951#discussion_r66881687
--- Diff: core/src/main/scala/org/apache/spark/scheduler/SchedulableBuilder.scala ---
@@ -59,12 +59,15 @@ private[spark] class FairSchedulableBuilder(val rootPool: Pool, conf: SparkConf)
val FAIR_SCHEDULER_PROPERTIES = "spark.scheduler.pool"
val DEFAULT_POOL_NAME = "default"
val MINIMUM_SHARES_PROPERTY = "minShare"
+ val MAXIMUM_SHARES_PROPERTY = "maxRunningTasks"
val SCHEDULING_MODE_PROPERTY = "schedulingMode"
+ val PARENT_PROPERTY = "parent"
val WEIGHT_PROPERTY = "weight"
val POOL_NAME_PROPERTY = "@name"
val POOLS_PROPERTY = "pool"
val DEFAULT_SCHEDULING_MODE = SchedulingMode.FIFO
val DEFAULT_MINIMUM_SHARE = 0
+ val DEFAULT_MAXIMUM_SHARE = Int.MaxValue
--- End diff --
I think this should be MAXIMUM_TASKS? (and same with the MAXIMUM_SHARES_PROPERTY)
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12951
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by njwhite <gi...@git.apache.org>.
Github user njwhite commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222297641
Thanks @markhamstra! The Jenkins build failed because a single test, `ExternalAppendOnlyMapSuite#spilling with compression`, failed. It seems unrelated (and passes locally for me) - are there known issues with it?
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222175470
**[Test build #59497 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59497/consoleFull)** for PR 12951 at commit [`122b122`](https://github.com/apache/spark/commit/122b122869bc7b4b9df81813af91a9ff4aaa4ce5).
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-220720199
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-220720729
**[Test build #59030 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59030/consoleFull)** for PR 12951 at commit [`c4082c5`](https://github.com/apache/spark/commit/c4082c5ffc5cd486b0e83fcd6e1ee5ebc98d3299).
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/12951#discussion_r62332468
--- Diff: core/src/main/scala/org/apache/spark/scheduler/Pool.scala ---
@@ -21,6 +21,7 @@ import java.util.concurrent.{ConcurrentHashMap, ConcurrentLinkedQueue}
import scala.collection.JavaConverters._
import scala.collection.mutable.ArrayBuffer
+import scala.math.{max,min}
--- End diff --
It's a tiny nit, but while here, I usually see `math.max` just written out in Scala code; no need to import a standard class's methods.
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pool...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/12951#discussion_r66881361
--- Diff: core/src/main/scala/org/apache/spark/scheduler/Pool.scala ---
@@ -47,6 +48,15 @@ private[spark] class Pool(
var name = poolName
var parent: Pool = null
+ override def maxRunningTasks: Int = {
--- End diff --
Does it make sense to move this to Schedulable.scala? It looks like Pool and TaskSetManager both have the same implementation (assuming that Int.MAX_VALUE is the default).
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222175839
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222211646
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222175841
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59497/
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-220742174
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222211467
**[Test build #59501 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59501/consoleFull)** for PR 12951 at commit [`0669b49`](https://github.com/apache/spark/commit/0669b49ebb1e6f1c3fd08186fde1762a7740c2ba).
* 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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12951
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-15176][Core] Add maxShares setting to P...
Posted by markhamstra <gi...@git.apache.org>.
Github user markhamstra commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222230643
Added my comments to the JIRA. In short, I think there is a legitimate use case for this, and there is a significant gap in our current fair-scheduling pool API. Implementing a maxShare property is actually something that has been on my todo list for awhile.
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the issue:
https://github.com/apache/spark/pull/12951
@njwhite sorry to let this idle for so long. I just read through the comments here and on the [JIRA](https://issues.apache.org/jira/browse/SPARK-15176) and it looks like the consensus on the JIRA was that it would be better to implement maxShare instead of maxRunningTasks, because it's likely easier to configure, and also is less brittle to the cluster size. Can you implement that change?
Alternately if you think this should remain maxRunningTasks, comment on the JIRA and we can continue the discussion there.
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/12951#discussion_r62330957
--- Diff: core/src/main/scala/org/apache/spark/scheduler/Pool.scala ---
@@ -47,6 +49,15 @@ private[spark] class Pool(
var name = poolName
var parent: Pool = null
+ override def maxShare = {
--- End diff --
Maybe specifying return types? (See Return types in https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide)
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-220615364
ah, I completely overlooked `Pool.maxShare` referencing `runningTasks`, makes more sense now.
The added tests verify that `maxShare` is getting propagated correctly, and limited by the parent's max share. But there aren't any tests which show that scheduling is actually limited by it at all. We'd want tests to cover that. The code needs more comments explaining what is going on. I'd even add comments on the tests explaining what is being tested (eg., the line where the maxShare is limited by the parent pool).
Calling it "maxShare" is pretty confusing -- with this implementation it should probably be called "maxRunningTasks" or something. It also seems pretty hard to configure, though, I wonder if users really do want maxShare. We should be sure that whatever we add is what want long-term, so we're not stuck with complexity from a legacy setting.
honestly I am still uncertain about adding the feature, need to think about it more -- I'm just giving my comments on the code here. A very clean, well-tested PR can help make your case, but OTOH can also turn into wasted effort ...
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-220276101
cc @kayousterhout for 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 pull request: [SPARK-15176][Core] Add maxShares setting to P...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222207170
**[Test build #59498 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59498/consoleFull)** for PR 12951 at commit [`e100683`](https://github.com/apache/spark/commit/e1006835111765ea347c0a7432cc9231e26ad4a2).
* 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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12951
**[Test build #60036 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60036/consoleFull)** for PR 12951 at commit [`b8eaaef`](https://github.com/apache/spark/commit/b8eaaefe8620f63ec436ae3c2480006c4a578381).
* 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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222190773
**[Test build #59501 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59501/consoleFull)** for PR 12951 at commit [`0669b49`](https://github.com/apache/spark/commit/0669b49ebb1e6f1c3fd08186fde1762a7740c2ba).
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the issue:
https://github.com/apache/spark/pull/12951
The naming on this PR is somewhat confusing, because it looks like maxShares is supposed to return the maximum number of *remaining* tasks that can be run, rather than the maximum number of tasks that can be running at a time. The current name implies the latter. Is it possible to use a more descriptive name for this? maxRemainingTasks? I don't have a great idea here but maybe others do?
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222207408
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59498/
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/12951#discussion_r62330358
--- Diff: core/src/main/scala/org/apache/spark/scheduler/Pool.scala ---
@@ -21,6 +21,7 @@ import java.util.concurrent.{ConcurrentHashMap, ConcurrentLinkedQueue}
import scala.collection.JavaConverters._
import scala.collection.mutable.ArrayBuffer
+import scala.math.{max,min}
--- End diff --
I belive indentation should be as below:
```scala
scala.math.{max, min}
```
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12951
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12951
**[Test build #60036 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60036/consoleFull)** for PR 12951 at commit [`b8eaaef`](https://github.com/apache/spark/commit/b8eaaefe8620f63ec436ae3c2480006c4a578381).
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on the issue:
https://github.com/apache/spark/pull/12951
Should we process with this PR or should we close this? @kayousterhout @njwhite
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by njwhite <gi...@git.apache.org>.
Github user njwhite commented on the issue:
https://github.com/apache/spark/pull/12951
Actually, @kayousterhout - I'm not entirely sure what you expect for the semantics of maxShares in general. Maybe a worked example would help: if I have a pool X with 5 running tasks from Taskset A and a maxShares of 7. Pool X is a child of pool Y which has a maxShares of 8. I want to the schedule another task from Taskset A, so should the scheduler allow it or not? Do you need to know how many executors are currently running (and so the maximum number of tasks that could be run)?
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by ash211 <gi...@git.apache.org>.
Github user ash211 commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-219463293
Ping, anything more needed on this PR before merging?
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-217597424
@njwhite I am not a committer but just one of contributors. I guess most of codes were written by @kayousterhout.
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12951
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/61724/
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-220741994
**[Test build #59030 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59030/consoleFull)** for PR 12951 at commit [`c4082c5`](https://github.com/apache/spark/commit/c4082c5ffc5cd486b0e83fcd6e1ee5ebc98d3299).
* 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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222180851
**[Test build #59498 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59498/consoleFull)** for PR 12951 at commit [`e100683`](https://github.com/apache/spark/commit/e1006835111765ea347c0a7432cc9231e26ad4a2).
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by njwhite <gi...@git.apache.org>.
Github user njwhite commented on the issue:
https://github.com/apache/spark/pull/12951
Hi @kayousterhout - I've renamed all references to `maxRunningTasks` and updated the Markdown documentation in the repo. Is this OK? Thanks -
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the issue:
https://github.com/apache/spark/pull/12951
Also, once naming is settled on, this PR should include a documentation update to this page: https://spark.apache.org/docs/latest/job-scheduling.html to describe 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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12951
**[Test build #61724 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61724/consoleFull)** for PR 12951 at commit [`156f711`](https://github.com/apache/spark/commit/156f711b5a9f3b3e942ac36f42d6d4cc6c44aa50).
* 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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/12951#discussion_r62330929
--- Diff: core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala ---
@@ -98,6 +98,14 @@ private[spark] class TaskSetManager(
var totalResultSize = 0L
var calculatedTasks = 0
+ override def maxShare = {
--- End diff --
Maybe specifying return types? (See Return types in https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide)
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by njwhite <gi...@git.apache.org>.
Github user njwhite commented on the issue:
https://github.com/apache/spark/pull/12951
@kayousterhout minShares is a configuration parameter for the fair scheduler algorithm only - what would the semantics of a maxShares setting for the FIFO algorithm be?
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by njwhite <gi...@git.apache.org>.
Github user njwhite commented on the issue:
https://github.com/apache/spark/pull/12951
@squito is this OK?
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by njwhite <gi...@git.apache.org>.
Github user njwhite commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-217498019
@HyukjinKwon I've run `run-tests` and fixed all the style issues. Could you take another look? Thanks -
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12951
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12951
**[Test build #61724 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61724/consoleFull)** for PR 12951 at commit [`156f711`](https://github.com/apache/spark/commit/156f711b5a9f3b3e942ac36f42d6d4cc6c44aa50).
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by njwhite <gi...@git.apache.org>.
Github user njwhite commented on the issue:
https://github.com/apache/spark/pull/12951
ping?
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222211649
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59501/
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pool...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12951
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-15176][Core] Add maxShares setting to P...
Posted by njwhite <gi...@git.apache.org>.
Github user njwhite commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-220555299
Thanks for the review @squito - I've commented on the JIRA about why this feature would be useful. As for the implementation - maybe "maxShare" is the wrong word, as the change doesn't relate to the fair scheduler at all. Instead it limits the maximum number of tasks a `Schedulable` can have running at any one time. It really is just a one line change - `resourceOffer` now won't accept any more resources (i.e. won't run any of its tasks) if the calculated current value of `maxShares` means there isn't any free space in the pool. The `maxShares` method just returns the maximum number of tasks allowed in the pool, minus the number of tasks running in the pool. You can see the propagation of the `maxShares` limit in the assertions I added to the `PoolSuite` 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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-217399531
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 issue #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12951
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-15176][Core] Add maxShares setting to P...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/12951#discussion_r62331312
--- Diff: core/src/main/scala/org/apache/spark/scheduler/Pool.scala ---
@@ -21,6 +21,7 @@ import java.util.concurrent.{ConcurrentHashMap, ConcurrentLinkedQueue}
import scala.collection.JavaConverters._
import scala.collection.mutable.ArrayBuffer
+import scala.math.{max,min}
--- End diff --
(FYI, running `./dev/run-tests` will trigger style check first. Running the first part of this script before submitting more commits will show some comments I said 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 pull request: [SPARK-15176][Core] Add maxShares setting to P...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-220379391
Hi @njwhite, I'm not sure I see a strong need for this -- I posted a msg on jira (as Kay had earlier). We should keep discussion about the feature in general there, for archive / searchability.
In any case I did look at the code, so a couple of comments about that, if we do decide we want this feature. Unless I'm missing something, it doesn't seem like `maxShare` is being used anywhere (except for one trivial spot to check its > 0). I was expecting a change to `FairSchedulingAlgorithm` at least, though I think it might actually be a more complicated change to `TaskSchedulerImpl` that is required.
Also to go along with that, we'd want new test cases demonstrating how `maxShare` gets used.
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222207406
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/12951#discussion_r62329769
--- Diff: core/src/main/scala/org/apache/spark/scheduler/SchedulableBuilder.scala ---
@@ -90,10 +92,11 @@ private[spark] class FairSchedulableBuilder(val rootPool: Pool, conf: SparkConf)
private def buildDefaultPool() {
if (rootPool.getSchedulableByName(DEFAULT_POOL_NAME) == null) {
val pool = new Pool(DEFAULT_POOL_NAME, DEFAULT_SCHEDULING_MODE,
- DEFAULT_MINIMUM_SHARE, DEFAULT_WEIGHT)
+ DEFAULT_MINIMUM_SHARE, DEFAULT_MAXIMUM_SHARE, DEFAULT_WEIGHT)
rootPool.addSchedulable(pool)
- logInfo("Created default pool %s, schedulingMode: %s, minShare: %d, weight: %d".format(
- DEFAULT_POOL_NAME, DEFAULT_SCHEDULING_MODE, DEFAULT_MINIMUM_SHARE, DEFAULT_WEIGHT))
+ logInfo(
+ "Created default pool %s, schedulingMode: %s, minShare: %d, maxShare: %d, weight: %d".format(
+ DEFAULT_POOL_NAME, DEFAULT_SCHEDULING_MODE, DEFAULT_MINIMUM_SHARE, DEFAULT_MAXIMUM_SHARE, DEFAULT_WEIGHT))
--- End diff --
I believe this line exceeds 100 characters.
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by njwhite <gi...@git.apache.org>.
Github user njwhite commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222175368
Thanks @squito; I've renamed the setting to `maxRunningTasks` and added the tests you asked for to `TaskSetManagerSuite`. I've also added support (& tests) for configuring the parent pool in the XML config file, as that came in useful.
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by njwhite <gi...@git.apache.org>.
Github user njwhite commented on the issue:
https://github.com/apache/spark/pull/12951
@squito thanks - I've expanded the `Scheduler respects maxRunningTasks setting of its pool` test to cover the cases you mention (and a couple of others).
---
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 #12951: [SPARK-15176][Core] Add maxShares setting to Pools
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the issue:
https://github.com/apache/spark/pull/12951
@njwhite do you have time to work on this and implement maxShares? If not, can you close the PR?
---
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: [SPARK-15176][Core] Add maxShares setting to P...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12951#issuecomment-222175830
**[Test build #59497 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59497/consoleFull)** for PR 12951 at commit [`122b122`](https://github.com/apache/spark/commit/122b122869bc7b4b9df81813af91a9ff4aaa4ce5).
* 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