You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/04/22 00:12:10 UTC
[GitHub] [spark] venkata91 opened a new pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
venkata91 opened a new pull request #28287:
URL: https://github.com/apache/spark/pull/28287
### What changes were proposed in this pull request?
In this change, in the case of dynamic allocation is enabled instead of aborting an unschedulable blacklisted task blacklist immediately using the SparkListener pass an event saying UnschedulableBlacklistTaskSubmitted which will be handled by ExecutorAllocationManager and request more executors to schedule the unschedulable blacklisted task. Once the event is sent, we start the abortTimer similar to [SPARK-22148][SPARK-15815]
### How was this patch tested?
Currently manually tested it in our clusters. Also trying to figure out how to add unit tests.
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce any user-facing change?
<!--
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657764959
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125783/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-619031231
it might have been discussions no on PR or buried in comments, I don't have time to go looking. there are many different conditions to consider. The main one we were focusing on is that you had as many executors as you needed to execute the task you had left. This means the allocation manager was not going to ask for more. The problem is that some or all of those executor can get blacklisted. The only way for the dynamic allocation manager to know it needs to ask for more is for it to know that nodes are blacklisted and it needs to ask for some more executors - thus internally incrementing is count of executors needed and asking yarn or other resource manager for more. so now the number of executors the allocation manager is different then what its normal calculations would figure out. simplified: #executors = (#tasks * #cpus per task/#cores per executor). So you have to change the number of executors and you have to keep taking that it account because the allocation manager is always trying to calculate if it needs more or less executors. You also have to notify it when executors become unblacklisted, or perhaps the ones that were blacklisted idle timeout, etc. The allocation manager has to know a lot more details about the blacklisting and take that into account when its calculating the number of executors it needs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656882557
**[Test build #125637 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125637/testReport)** for PR 28287 at commit [`0784dc3`](https://github.com/apache/spark/commit/0784dc3d8f5419822b2e26f2c1144e0a2e285a9e).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659088466
**[Test build #125921 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125921/testReport)** for PR 28287 at commit [`0d07845`](https://github.com/apache/spark/commit/0d07845459e6e0f606e5f0920c973de256a309e1).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658979158
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658180547
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658977388
> you can make a common function that has most of the code that gets called from 2 separate tests. one test passes with dynamic allocation on, the other with it off. that will reduce code duplication.
nevermind, I made some changes to the test so that it goes to the `None` block where we check if dynamic allocation is enabled or not and request accordingly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657686732
test this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659142963
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658960595
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656806574
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659693491
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657263420
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656882557
**[Test build #125637 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125637/testReport)** for PR 28287 at commit [`0784dc3`](https://github.com/apache/spark/commit/0784dc3d8f5419822b2e26f2c1144e0a2e285a9e).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658179776
**[Test build #125838 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125838/testReport)** for PR 28287 at commit [`b4c27fb`](https://github.com/apache/spark/commit/b4c27fb2539d10356bb4208e0509f7a32a577613).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658951229
yes we have a test in TaskSchedulerImplSuite that checks to make sure it aborted, but I don't think it covers when dynamic allocation on, so it doesn't hit your new code. So we would want to add a test where it can't acquire a new executor and aborts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659596725
**[Test build #125996 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125996/testReport)** for PR 28287 at commit [`0d07845`](https://github.com/apache/spark/commit/0d07845459e6e0f606e5f0920c973de256a309e1).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659148768
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125918/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662136865
**[Test build #126283 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126283/testReport)** for PR 28287 at commit [`d9f473d`](https://github.com/apache/spark/commit/d9f473d748b206426106a9c4e97e153aec2f508f).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659428051
Sorry that failure might be related to https://issues.apache.org/jira/browse/SPARK-32287 I kicked it again
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656806580
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125613/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662698601
**[Test build #126352 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126352/testReport)** for PR 28287 at commit [`d6f1e73`](https://github.com/apache/spark/commit/d6f1e73ae49d9641a8efcb1f5e9016bd378a70c6).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
mridulm commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r449746678
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -289,13 +290,23 @@ private[spark] class ExecutorAllocationManager(
s" tasksperexecutor: $tasksPerExecutor")
val maxNeeded = math.ceil(numRunningOrPendingTasks * executorAllocationRatio /
tasksPerExecutor).toInt
- if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
+ val totalNeed = if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
// If we have pending speculative tasks and only need a single executor, allocate one more
// to satisfy the locality requirements of speculation
maxNeeded + 1
} else {
maxNeeded
}
+
+ // Request additional executors to schedule the unschedulable tasks as well
+ if (numUnschedulables > 0) {
+ val maxNeededForUnschedulables = math.ceil(numUnschedulables * executorAllocationRatio /
+ tasksPerExecutor).toInt
+ math.max(totalNeed, executorMonitor.executorCountWithResourceProfile(rpId)) +
+ maxNeededForUnschedulables
+ } else {
Review comment:
Scratch that - what I had seen was probably a modified version, not original ...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r455205081
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -829,12 +873,28 @@ private[spark] class ExecutorAllocationManager(
numTotalTasks - numRunning
}
+ // Currently TaskSetManager.getCompletelyBlacklistedTaskIfAny only takes the first unschedulable
+ // task found due to blacklisting. This way we only need to keep track of the unschedulable
+ // tasksets which is an indirect way to get the current number of unschedulable tasks.
+ def pendingUnschedulableTaskSetsPerResourceProfile(rp: Int): Int = {
+ val attempts = resourceProfileIdToStageAttempt.getOrElse(rp, Set.empty).toSeq
+ attempts.filter(attempt => unschedulableTaskSets.contains(attempt)).size
+ }
+
+ def hasPendingUnschedulableTasks: Boolean = {
+ val attemptSets = resourceProfileIdToStageAttempt.values
+ attemptSets.exists { attempts =>
+ attempts.exists(unschedulableTaskSets.contains(_))
+ }
+ }
+
def hasPendingTasks: Boolean = {
- hasPendingSpeculativeTasks || hasPendingRegularTasks
+ hasPendingSpeculativeTasks || hasPendingRegularTasks || hasPendingUnschedulableTasks
Review comment:
No, this is not needed. I will remove this. Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657263279
**[Test build #125723 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125723/testReport)** for PR 28287 at commit [`b4c27fb`](https://github.com/apache/spark/commit/b4c27fb2539d10356bb4208e0509f7a32a577613).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659131774
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658957953
> yes we have a test in TaskSchedulerImplSuite that checks to make sure it aborted, but I don't think it covers when dynamic allocation on, so it doesn't hit your new code. So we would want to add a test where it can't acquire a new executor and aborts.
I think the new test which I added is just duplicated. Do you think its better to just add the config to enable dynamic allocation to the other test itself in order to avoid the duplication
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662185107
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659086627
**[Test build #125919 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125919/testReport)** for PR 28287 at commit [`112bb3a`](https://github.com/apache/spark/commit/112bb3a3e72c664f2cbb5bebca0265f5e9c23303).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659086627
**[Test build #125919 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125919/testReport)** for PR 28287 at commit [`112bb3a`](https://github.com/apache/spark/commit/112bb3a3e72c664f2cbb5bebca0265f5e9c23303).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r449798406
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -789,6 +811,23 @@ private[spark] class ExecutorAllocationManager(
}
}
+ override def onUnschedulableBlacklistTaskSubmitted
+ (blacklistedTask: SparkListenerUnschedulableBlacklistTaskSubmitted): Unit = {
+ val stageId = blacklistedTask.stageId
+ val stageAttemptId = blacklistedTask.stageAttemptId
+ allocationManager.synchronized {
+ (stageId, stageAttemptId) match {
+ case (Some(stageId), Some(stageAttemptId)) =>
+ val stageAttempt = StageAttempt(stageId, stageAttemptId)
+ unschedulableTaskSets.add(stageAttempt)
+ case (None, _) =>
+ // Clear unschedulableTaskSets since atleast one task becomes schedulable now
+ unschedulableTaskSets.clear()
Review comment:
https://github.com/apache/spark/blob/263f04db865920d9c10251517b00a1b477b58ff1/core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala#L670 - Based on the previous implementation, we want to defer killing taskSets as long as we have a non blacklisted executor which can be used to schedule a task. The problem is if we can only schedule only one of the task set then the other taskset might expire and the stage gets aborted. This is the reason I think why they cleared the expiry in the previous implementation which is why we are also clearing the `unschedulableTaskSets`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on issue #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on issue #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-617786378
so took a quick look and I don't follow. You send a message that says all blacklisted, but all you do is call the onSchedulerBacklogged, how does that add another executor if they are all blacklisted? that will update when it needs to add more, but it doesn't change the number if calculated. So for instance say I needed 3 executors. All of them blacklisted, updating the time to get a new one won't change that it thinks it needs 3.
If we change that to tell it to get one more, then my question is how do you do the proper accounting on that. This is why in the jira and other PR we said the allocation manager needs to be closely tied with the blacklist manager.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-649759229
> yes its possible, I saw some issues on a few other pro although it was with different parts. It maybe early next week before I can review.
Thanks that should be fine.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-617476515
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-661170133
@tgravescs Can you please take a look again? Now that the tests are passing. Thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656872295
> **[Test build #125613 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125613/testReport)** for PR 28287 at commit [`0784dc3`](https://github.com/apache/spark/commit/0784dc3d8f5419822b2e26f2c1144e0a2e285a9e).
>
> * This patch **fails Spark unit tests**.
> * This patch merges cleanly.
> * This patch adds the following public classes _(experimental)_:
> * `case class UnschedulableTaskSetAdded(stageId: Int, stageAttemptId: Int)`
> * `case class UnschedulableTaskSetRemoved(stageId: Int, stageAttemptId: Int)`
> * `case class SparkListenerUnschedulableTaskSetAdded(`
> * `case class SparkListenerUnschedulableTaskSetRemoved(`
It seems like the failed tests are unrelated seems like transient issues. @tgravescs Should we rerun it again?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659597488
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656679442
**[Test build #125613 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125613/testReport)** for PR 28287 at commit [`0784dc3`](https://github.com/apache/spark/commit/0784dc3d8f5419822b2e26f2c1144e0a2e285a9e).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
mridulm commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r451694092
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -789,6 +811,23 @@ private[spark] class ExecutorAllocationManager(
}
}
+ override def onUnschedulableBlacklistTaskSubmitted
+ (blacklistedTask: SparkListenerUnschedulableBlacklistTaskSubmitted): Unit = {
+ val stageId = blacklistedTask.stageId
+ val stageAttemptId = blacklistedTask.stageAttemptId
+ allocationManager.synchronized {
+ (stageId, stageAttemptId) match {
+ case (Some(stageId), Some(stageAttemptId)) =>
+ val stageAttempt = StageAttempt(stageId, stageAttemptId)
+ unschedulableTaskSets.add(stageAttempt)
+ case (None, _) =>
+ // Clear unschedulableTaskSets since atleast one task becomes schedulable now
+ unschedulableTaskSets.clear()
Review comment:
There is a difference in both cases.
If all tasksets are removed from `TaskSchedulerImpl.unschedulableTaskSetToExpiryTime`, it would defer aborting unschedulable tasksets in the assumption that some progress is being made and eventually tasksets might become scheduleable (or gets readded subsequently).
On other hand, if an unschedulable taskset remains in `unschedulableTaskSets`, this can trigger a successful executor allocation sooner - which could make it (and other taskset's) schedulable.
If we clear `unschedulableTaskSets` here, an eventual resourceOffer will result in re-addition to `unschedulableTaskSets` after a delay. If we want to defer resource acquisition, that sounds fine - even with this, current PR is still a strict improvement over what we have.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-648956170
Can someone help me with why this Generate documents check is failing? Not sure I'm understanding the issue here. Any pointers would be appreciated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658180547
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662134628
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
mridulm commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r447233292
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -696,7 +713,9 @@ private[spark] class ExecutorAllocationManager(
// If this is the last stage with pending tasks, mark the scheduler queue as empty
// This is needed in case the stage is aborted for any reason
- if (stageAttemptToNumTasks.isEmpty && stageAttemptToNumSpeculativeTasks.isEmpty) {
+ if (stageAttemptToNumTasks.isEmpty &&
+ stageAttemptToNumSpeculativeTasks.isEmpty &&
+ unschedulableTaskSets.isEmpty) {
Review comment:
Is this required ? If `unschedulableTaskSets` is not empty, then `stageAttemptToNumTasks` should also be non empty right ?
##########
File path: core/src/main/java/org/apache/spark/SparkFirehoseListener.java
##########
@@ -162,6 +162,11 @@ public void onSpeculativeTaskSubmitted(SparkListenerSpeculativeTaskSubmitted spe
onEvent(speculativeTask);
}
+ public void onUnschedulableBlacklistTaskSubmitted(
Review comment:
Couple of things here:
* The task is not submitted, but rather has become unschedulable (perhaps due to lost executors).
* The event details are about unschedulable task set - does not give task information.
Can we rename this to something better ?
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -789,6 +811,23 @@ private[spark] class ExecutorAllocationManager(
}
}
+ override def onUnschedulableBlacklistTaskSubmitted
+ (blacklistedTask: SparkListenerUnschedulableBlacklistTaskSubmitted): Unit = {
+ val stageId = blacklistedTask.stageId
+ val stageAttemptId = blacklistedTask.stageAttemptId
+ allocationManager.synchronized {
+ (stageId, stageAttemptId) match {
+ case (Some(stageId), Some(stageAttemptId)) =>
+ val stageAttempt = StageAttempt(stageId, stageAttemptId)
+ unschedulableTaskSets.add(stageAttempt)
+ case (None, _) =>
+ // Clear unschedulableTaskSets since atleast one task becomes schedulable now
+ unschedulableTaskSets.clear()
Review comment:
Why are we clearing all tasksets ? Shouldn't this not remove the specific taskset which became schedulable ?
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -289,13 +290,23 @@ private[spark] class ExecutorAllocationManager(
s" tasksperexecutor: $tasksPerExecutor")
val maxNeeded = math.ceil(numRunningOrPendingTasks * executorAllocationRatio /
tasksPerExecutor).toInt
- if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
+ val totalNeed = if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
// If we have pending speculative tasks and only need a single executor, allocate one more
// to satisfy the locality requirements of speculation
maxNeeded + 1
} else {
maxNeeded
}
+
+ // Request additional executors to schedule the unschedulable tasks as well
+ if (numUnschedulables > 0) {
+ val maxNeededForUnschedulables = math.ceil(numUnschedulables * executorAllocationRatio /
+ tasksPerExecutor).toInt
+ math.max(totalNeed, executorMonitor.executorCountWithResourceProfile(rpId)) +
+ maxNeededForUnschedulables
+ } else {
Review comment:
Can you update with details about how this was resolved ? The original looked fine to me, but I want to make sure I am not missing something.
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -829,12 +868,25 @@ private[spark] class ExecutorAllocationManager(
numTotalTasks - numRunning
}
+ def pendingUnschedulableTasksPerResourceProfile(rp: Int): Int = {
Review comment:
The method is returning number of task sets which are not schedulable - not number of tasks which are not scheduable.
`maxNumExecutorsNeededPerResourceProfile` is relying on this being task count - we will need to fix it here and in the event being fired.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662598378
> @venkata91 I assume your updates keep doing rebase? If possible its great if you can just do up merges because doing rebases makes seeing the diffs between requested changes much harder.
Sure, makes sense. Will keep that in mind for the future.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658960595
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-617476121
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656880529
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662185107
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r451864474
##########
File path: core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala
##########
@@ -311,6 +311,16 @@ private[spark] class DAGScheduler(
eventProcessLoop.post(SpeculativeTaskSubmitted(task))
}
+ /**
+ * Called by the TaskSetManager when there is an unschedulable blacklist task and dynamic
+ * allocation is enabled
+ */
+ def unschedulableBlacklistTaskSubmitted(
Review comment:
I thought I removed everything, seems I missed this one. Thanks for pointing out.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657689826
**[Test build #125783 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125783/testReport)** for PR 28287 at commit [`b4c27fb`](https://github.com/apache/spark/commit/b4c27fb2539d10356bb4208e0509f7a32a577613).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662603049
**[Test build #126352 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126352/testReport)** for PR 28287 at commit [`d6f1e73`](https://github.com/apache/spark/commit/d6f1e73ae49d9641a8efcb1f5e9016bd378a70c6).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xkrogen commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
xkrogen commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r447136222
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -289,13 +290,23 @@ private[spark] class ExecutorAllocationManager(
s" tasksperexecutor: $tasksPerExecutor")
val maxNeeded = math.ceil(numRunningOrPendingTasks * executorAllocationRatio /
tasksPerExecutor).toInt
- if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
+ val totalNeed = if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
// If we have pending speculative tasks and only need a single executor, allocate one more
// to satisfy the locality requirements of speculation
maxNeeded + 1
} else {
maxNeeded
}
+
+ // Request additional executors to schedule the unschedulable tasks as well
+ if (numUnschedulables > 0) {
+ val maxNeededForUnschedulables = math.ceil(numUnschedulables * executorAllocationRatio /
+ tasksPerExecutor).toInt
+ math.max(totalNeed, executorMonitor.executorCountWithResourceProfile(rpId)) +
Review comment:
We discussed offline that using the current executor count can result in too many executors getting allocated, if the scheduling loop runs between when an executor is allocated and when tasks are scheduled on it (thus clearing out `unschedulableTaskSets`). Did you come up with any solution for this? If not and we think it's a small enough issue to ignore, let's at least add a comment about this caveat.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xkrogen commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
xkrogen commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r447134667
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -289,13 +290,23 @@ private[spark] class ExecutorAllocationManager(
s" tasksperexecutor: $tasksPerExecutor")
val maxNeeded = math.ceil(numRunningOrPendingTasks * executorAllocationRatio /
tasksPerExecutor).toInt
- if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
+ val totalNeed = if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
Review comment:
I think `totalNeed` is a little confusing. Maybe `maxNeededWithSpeculationLocalityOffset` or something else more descriptive like that?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659142331
**[Test build #125921 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125921/testReport)** for PR 28287 at commit [`0d07845`](https://github.com/apache/spark/commit/0d07845459e6e0f606e5f0920c973de256a309e1).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659036401
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662133746
> couple minor things otherwise looks good
Awesome. Thanks. Addressed the review comments. Hopefully the tests should pass fine the first time itself.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662603049
**[Test build #126352 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126352/testReport)** for PR 28287 at commit [`d6f1e73`](https://github.com/apache/spark/commit/d6f1e73ae49d9641a8efcb1f5e9016bd378a70c6).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659148089
**[Test build #125918 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125918/testReport)** for PR 28287 at commit [`e167da5`](https://github.com/apache/spark/commit/e167da53b0b8ffde746c86e0564d7971363f3746).
* This patch **fails PySpark pip packaging tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656879502
test this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r455204743
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -696,7 +717,8 @@ private[spark] class ExecutorAllocationManager(
Review comment:
I think as long as the stageattempt is schedulable we can remove it, although it can fail but eventually if it becomes unschedulable again, we would add it back again to unschedulableTaskSets right? Wouldn't that be simpler?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656676553
ok to test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656804835
**[Test build #125613 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125613/testReport)** for PR 28287 at commit [`0784dc3`](https://github.com/apache/spark/commit/0784dc3d8f5419822b2e26f2c1144e0a2e285a9e).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class UnschedulableTaskSetAdded(stageId: Int, stageAttemptId: Int)`
* `case class UnschedulableTaskSetRemoved(stageId: Int, stageAttemptId: Int)`
* `case class SparkListenerUnschedulableTaskSetAdded(`
* `case class SparkListenerUnschedulableTaskSetRemoved(`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658979158
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-649532575
yes its possible, I saw some issues on a few other pro although it was with different parts. It maybe early next week before I can review.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r448697727
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -289,13 +290,23 @@ private[spark] class ExecutorAllocationManager(
s" tasksperexecutor: $tasksPerExecutor")
val maxNeeded = math.ceil(numRunningOrPendingTasks * executorAllocationRatio /
tasksPerExecutor).toInt
- if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
+ val totalNeed = if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
// If we have pending speculative tasks and only need a single executor, allocate one more
// to satisfy the locality requirements of speculation
maxNeeded + 1
} else {
maxNeeded
}
+
+ // Request additional executors to schedule the unschedulable tasks as well
+ if (numUnschedulables > 0) {
+ val maxNeededForUnschedulables = math.ceil(numUnschedulables * executorAllocationRatio /
+ tasksPerExecutor).toInt
+ math.max(totalNeed, executorMonitor.executorCountWithResourceProfile(rpId)) +
Review comment:
Currently ExecutorAllocationManager runs every 100ms and resourceOffer happens every 1sec. `unschedulableTaskSets` gets cleared only if any task becomes schedulable and also we'll send the `unschedulable` event only once unless its removed from the `unschedulableTaskSets`. So I'm guessing it should be fine. Please correct me if I'm forgetting something.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r458798547
##########
File path: core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala
##########
@@ -311,6 +311,26 @@ private[spark] class DAGScheduler(
eventProcessLoop.post(SpeculativeTaskSubmitted(task))
}
+ /**
+ * Called by the TaskSetManager when a taskset becomes unschedulable due to blacklisting and
+ * dynamic allocation is enabled.
+ */
+ def unschedulableTaskSetAdded(
+ stageId: Int,
+ stageAttemptId: Int): Unit = {
+ eventProcessLoop.post(UnschedulableTaskSetAdded(stageId, stageAttemptId))
+ }
+
+ /**
+ * Called by the TaskSetManager when an unschedulable taskset becomes schedulable and dynamic
+ * allocation is enabled.
+ */
+ def unschedulableTaskSetRemoved(
+ stageId: Int,
Review comment:
indentation needs to be 4 spaces
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657764950
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r455153176
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -789,6 +811,28 @@ private[spark] class ExecutorAllocationManager(
}
}
+ override def onUnschedulableTaskSetAdded(
+ unschedulableTaskSetAdded: SparkListenerUnschedulableTaskSetAdded): Unit = {
+ val stageId = unschedulableTaskSetAdded.stageId
+ val stageAttemptId = unschedulableTaskSetAdded.stageAttemptId
+ allocationManager.synchronized {
+ val stageAttempt = StageAttempt(stageId, stageAttemptId)
+ unschedulableTaskSets.add(stageAttempt)
+ }
+ allocationManager.onSchedulerBacklogged()
+ }
+
+ override def onUnschedulableTaskSetRemoved(
+ unschedulableTaskSetRemoved: SparkListenerUnschedulableTaskSetRemoved): Unit = {
+ val stageId = unschedulableTaskSetRemoved.stageId
+ val stageAttemptId = unschedulableTaskSetRemoved.stageAttemptId
+ allocationManager.synchronized {
+ // Clear unschedulableTaskSets since atleast one task becomes schedulable now
+ val stageAttempt = StageAttempt(stageId, stageAttemptId)
Review comment:
similar move stageAttempt outside of synchronized
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -289,13 +290,27 @@ private[spark] class ExecutorAllocationManager(
s" tasksperexecutor: $tasksPerExecutor")
val maxNeeded = math.ceil(numRunningOrPendingTasks * executorAllocationRatio /
tasksPerExecutor).toInt
- if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
+
+ val maxNeededWithSpeculationLocalityOffset =
+ if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
// If we have pending speculative tasks and only need a single executor, allocate one more
// to satisfy the locality requirements of speculation
maxNeeded + 1
} else {
maxNeeded
}
+
+ if (unschedulableTaskSets > 0) {
+ // Request additional executors only if maxNeededWithSpeculationLocalityOffset is less than
Review comment:
Can we simply this comment to be something like:
Request additional executors to account for task sets that have tasks that are unschedulable due to blacklisting when the active executor count has already reached the the max needed we would normally get.
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -789,6 +811,28 @@ private[spark] class ExecutorAllocationManager(
}
}
+ override def onUnschedulableTaskSetAdded(
+ unschedulableTaskSetAdded: SparkListenerUnschedulableTaskSetAdded): Unit = {
+ val stageId = unschedulableTaskSetAdded.stageId
+ val stageAttemptId = unschedulableTaskSetAdded.stageAttemptId
+ allocationManager.synchronized {
+ val stageAttempt = StageAttempt(stageId, stageAttemptId)
Review comment:
setting stageAttempt can be done outside of synchronized
##########
File path: core/src/main/scala/org/apache/spark/scheduler/SparkListener.scala
##########
@@ -339,6 +349,20 @@ private[spark] trait SparkListenerInterface {
*/
def onNodeUnblacklisted(nodeUnblacklisted: SparkListenerNodeUnblacklisted): Unit
+ /**
+ * Called when a taskset becomes unschedulable due to blacklisting and dynamic allocation
+ * is enabled
+ */
+ def onUnschedulableTaskSetAdded(
+ unschedulableTaskSetAdded: SparkListenerUnschedulableTaskSetAdded): Unit
+
+ /**
+ * Called when an unschedulable taskset becomes schedulable and dynamic allocation
+ * is enabled
Review comment:
add period
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -696,7 +717,8 @@ private[spark] class ExecutorAllocationManager(
// If this is the last stage with pending tasks, mark the scheduler queue as empty
// This is needed in case the stage is aborted for any reason
- if (stageAttemptToNumTasks.isEmpty && stageAttemptToNumSpeculativeTasks.isEmpty) {
+ if (stageAttemptToNumTasks.isEmpty &&
+ stageAttemptToNumSpeculativeTasks.isEmpty) {
Review comment:
revert this change as its unneeded
##########
File path: core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala
##########
@@ -311,6 +311,26 @@ private[spark] class DAGScheduler(
eventProcessLoop.post(SpeculativeTaskSubmitted(task))
}
+ /**
+ * Called by the TaskSetManager when a taskset becomes unschedulable due to blacklisting and
+ * dynamic allocation is enabled
+ */
+ def unschedulableTaskSetAdded(
+ stageId: Int,
+ stageAttemptId: Int): Unit = {
+ eventProcessLoop.post(UnschedulableTaskSetAdded(stageId, stageAttemptId))
+ }
+
+ /**
+ * Called by the TaskSetManager when an unschedulable taskset becomes schedulable and dynamic
+ * allocation is enabled
Review comment:
nit add period
##########
File path: core/src/main/scala/org/apache/spark/scheduler/SparkListener.scala
##########
@@ -339,6 +349,20 @@ private[spark] trait SparkListenerInterface {
*/
def onNodeUnblacklisted(nodeUnblacklisted: SparkListenerNodeUnblacklisted): Unit
+ /**
+ * Called when a taskset becomes unschedulable due to blacklisting and dynamic allocation
+ * is enabled
Review comment:
nit add period.
##########
File path: core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala
##########
@@ -1014,6 +1034,20 @@ private[spark] class DAGScheduler(
listenerBus.post(SparkListenerSpeculativeTaskSubmitted(task.stageId, task.stageAttemptId))
}
+ private[scheduler] def handleUnschedulableTaskSetAdded(
+ stageId: Int,
+ stageAttemptId: Int): Unit = {
+ listenerBus.post(
+ SparkListenerUnschedulableTaskSetAdded(stageId, stageAttemptId))
+ }
+
+ private[scheduler] def handleUnschedulableTaskSetRemoved(
+ stageId: Int,
+ stageAttemptId: Int): Unit = {
+ listenerBus.post(
+ SparkListenerUnschedulableTaskSetRemoved(stageId, stageAttemptId))
Review comment:
same here put on previous line
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -829,12 +873,28 @@ private[spark] class ExecutorAllocationManager(
numTotalTasks - numRunning
}
+ // Currently TaskSetManager.getCompletelyBlacklistedTaskIfAny only takes the first unschedulable
+ // task found due to blacklisting. This way we only need to keep track of the unschedulable
+ // tasksets which is an indirect way to get the current number of unschedulable tasks.
+ def pendingUnschedulableTaskSetsPerResourceProfile(rp: Int): Int = {
+ val attempts = resourceProfileIdToStageAttempt.getOrElse(rp, Set.empty).toSeq
+ attempts.filter(attempt => unschedulableTaskSets.contains(attempt)).size
+ }
+
+ def hasPendingUnschedulableTasks: Boolean = {
+ val attemptSets = resourceProfileIdToStageAttempt.values
+ attemptSets.exists { attempts =>
+ attempts.exists(unschedulableTaskSets.contains(_))
+ }
+ }
+
def hasPendingTasks: Boolean = {
- hasPendingSpeculativeTasks || hasPendingRegularTasks
+ hasPendingSpeculativeTasks || hasPendingRegularTasks || hasPendingUnschedulableTasks
Review comment:
this doesn't make sense to me. Did you have a specific bug without this?
The tasks themselves are counted as regular tasks.
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -696,7 +717,8 @@ private[spark] class ExecutorAllocationManager(
Review comment:
I think we should remove the stage attempt from unschedulableTaskSets when the stage completes in case it fails or something and never send the unschedulabletasksetremoved
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -789,6 +811,28 @@ private[spark] class ExecutorAllocationManager(
}
}
+ override def onUnschedulableTaskSetAdded(
+ unschedulableTaskSetAdded: SparkListenerUnschedulableTaskSetAdded): Unit = {
+ val stageId = unschedulableTaskSetAdded.stageId
+ val stageAttemptId = unschedulableTaskSetAdded.stageAttemptId
+ allocationManager.synchronized {
+ val stageAttempt = StageAttempt(stageId, stageAttemptId)
+ unschedulableTaskSets.add(stageAttempt)
+ }
+ allocationManager.onSchedulerBacklogged()
Review comment:
this should be moved inside of synchronized
##########
File path: core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala
##########
@@ -311,6 +311,26 @@ private[spark] class DAGScheduler(
eventProcessLoop.post(SpeculativeTaskSubmitted(task))
}
+ /**
+ * Called by the TaskSetManager when a taskset becomes unschedulable due to blacklisting and
+ * dynamic allocation is enabled
Review comment:
nit, add period at end
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -829,12 +873,28 @@ private[spark] class ExecutorAllocationManager(
numTotalTasks - numRunning
}
+ // Currently TaskSetManager.getCompletelyBlacklistedTaskIfAny only takes the first unschedulable
+ // task found due to blacklisting. This way we only need to keep track of the unschedulable
+ // tasksets which is an indirect way to get the current number of unschedulable tasks.
+ def pendingUnschedulableTaskSetsPerResourceProfile(rp: Int): Int = {
+ val attempts = resourceProfileIdToStageAttempt.getOrElse(rp, Set.empty).toSeq
+ attempts.filter(attempt => unschedulableTaskSets.contains(attempt)).size
+ }
+
+ def hasPendingUnschedulableTasks: Boolean = {
+ val attemptSets = resourceProfileIdToStageAttempt.values
+ attemptSets.exists { attempts =>
+ attempts.exists(unschedulableTaskSets.contains(_))
+ }
+ }
+
def hasPendingTasks: Boolean = {
- hasPendingSpeculativeTasks || hasPendingRegularTasks
+ hasPendingSpeculativeTasks || hasPendingRegularTasks || hasPendingUnschedulableTasks
}
def totalPendingTasksPerResourceProfile(rp: Int): Int = {
- pendingTasksPerResourceProfile(rp) + pendingSpeculativeTasksPerResourceProfile(rp)
+ pendingTasksPerResourceProfile(rp) +
+ pendingSpeculativeTasksPerResourceProfile(rp)
Review comment:
revert
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -829,12 +873,28 @@ private[spark] class ExecutorAllocationManager(
numTotalTasks - numRunning
}
+ // Currently TaskSetManager.getCompletelyBlacklistedTaskIfAny only takes the first unschedulable
+ // task found due to blacklisting. This way we only need to keep track of the unschedulable
+ // tasksets which is an indirect way to get the current number of unschedulable tasks.
Review comment:
so we should clarify further. I realize its indirect way, but its not really the number of unschedulable tasks. Perhaps we can just this to say something a bit more generic like:
Currently we only know when a task set has an unschedulable task, we don't know the exact number and since the allocation manager isn't tied closely with the scheduler we use the number of tasks sets that are unschedulable as a heuristic to add more executors.
##########
File path: core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala
##########
@@ -1014,6 +1034,20 @@ private[spark] class DAGScheduler(
listenerBus.post(SparkListenerSpeculativeTaskSubmitted(task.stageId, task.stageAttemptId))
}
+ private[scheduler] def handleUnschedulableTaskSetAdded(
+ stageId: Int,
+ stageAttemptId: Int): Unit = {
+ listenerBus.post(
+ SparkListenerUnschedulableTaskSetAdded(stageId, stageAttemptId))
Review comment:
I think this will fit on previous line
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659692926
**[Test build #125996 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125996/testReport)** for PR 28287 at commit [`0d07845`](https://github.com/apache/spark/commit/0d07845459e6e0f606e5f0920c973de256a309e1).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on issue #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on issue #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-617497333
Can you please review @squito @tgravescs @mridulm ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658177196
I still need to look at your changes as well, haven't had time yet
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662134628
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659083280
**[Test build #125918 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125918/testReport)** for PR 28287 at commit [`e167da5`](https://github.com/apache/spark/commit/e167da53b0b8ffde746c86e0564d7971363f3746).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656955277
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656880529
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662134122
**[Test build #126282 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126282/testReport)** for PR 28287 at commit [`2f019d5`](https://github.com/apache/spark/commit/2f019d5dc1844eba53cdebabb7adfed8c911830c).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662184570
**[Test build #126282 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126282/testReport)** for PR 28287 at commit [`2f019d5`](https://github.com/apache/spark/commit/2f019d5dc1844eba53cdebabb7adfed8c911830c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662190703
**[Test build #126283 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126283/testReport)** for PR 28287 at commit [`d9f473d`](https://github.com/apache/spark/commit/d9f473d748b206426106a9c4e97e153aec2f508f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656954246
**[Test build #125637 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125637/testReport)** for PR 28287 at commit [`0784dc3`](https://github.com/apache/spark/commit/0784dc3d8f5419822b2e26f2c1144e0a2e285a9e).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class UnschedulableTaskSetAdded(stageId: Int, stageAttemptId: Int)`
* `case class UnschedulableTaskSetRemoved(stageId: Int, stageAttemptId: Int)`
* `case class SparkListenerUnschedulableTaskSetAdded(`
* `case class SparkListenerUnschedulableTaskSetRemoved(`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658896316
> it would also be nice to add a test to make sure the abort timer still works as expected. Please check TaskSchedulerImplSuite
@tgravescs Do you mean a test where allocation manager is not able to acquire a new executor with in the timeout and the stage should get aborted?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662599924
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662191204
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657689826
**[Test build #125783 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125783/testReport)** for PR 28287 at commit [`b4c27fb`](https://github.com/apache/spark/commit/b4c27fb2539d10356bb4208e0509f7a32a577613).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-649263104
No worries @tgravescs Thanks for taking a look. For some reason, these checks keeps failing but it doesn't look to be related to my changes. some cache issue probably?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659595794
test this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662165707
@tgravescs it seems like couple of tests are failing. Can you please kick this off again?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658176997
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657764128
**[Test build #125783 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125783/testReport)** for PR 28287 at commit [`b4c27fb`](https://github.com/apache/spark/commit/b4c27fb2539d10356bb4208e0509f7a32a577613).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class UnschedulableTaskSetAdded(stageId: Int, stageAttemptId: Int)`
* `case class UnschedulableTaskSetRemoved(stageId: Int, stageAttemptId: Int)`
* `case class SparkListenerUnschedulableTaskSetAdded(`
* `case class SparkListenerUnschedulableTaskSetRemoved(`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658308926
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659148755
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656679442
**[Test build #125613 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125613/testReport)** for PR 28287 at commit [`0784dc3`](https://github.com/apache/spark/commit/0784dc3d8f5419822b2e26f2c1144e0a2e285a9e).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662134122
**[Test build #126282 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126282/testReport)** for PR 28287 at commit [`2f019d5`](https://github.com/apache/spark/commit/2f019d5dc1844eba53cdebabb7adfed8c911830c).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657279975
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125723/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659036401
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659088466
**[Test build #125921 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125921/testReport)** for PR 28287 at commit [`0d07845`](https://github.com/apache/spark/commit/0d07845459e6e0f606e5f0920c973de256a309e1).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656955277
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] asfgit closed pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #28287:
URL: https://github.com/apache/spark/pull/28287
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656806574
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659503642
@tgravescs it seems like it again failed somewhere in pyspark module. Not sure if this is transient or not.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657263279
**[Test build #125723 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125723/testReport)** for PR 28287 at commit [`b4c27fb`](https://github.com/apache/spark/commit/b4c27fb2539d10356bb4208e0509f7a32a577613).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r449798608
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -829,12 +868,25 @@ private[spark] class ExecutorAllocationManager(
numTotalTasks - numRunning
}
+ def pendingUnschedulableTasksPerResourceProfile(rp: Int): Int = {
Review comment:
In the blacklisting logic, https://github.com/apache/spark/blob/263f04db865920d9c10251517b00a1b477b58ff1/core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala#L615 - here we just check for the first task which is currently unschedulable (note: there could be more which we would eventually come back to), which is why we will only have 1 unschedulable task per taskset, thats why only we keep track of the unschedulableTaskSets and not the individual tasks in the taskSets themselves. So in short # of unschedulableTaskSets = # of unschedulableTasks fr now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662599924
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656955280
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125637/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r455204743
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -696,7 +717,8 @@ private[spark] class ExecutorAllocationManager(
Review comment:
I think as long as the stageattempt is schedulable we can remove it, although it can fail but eventually if it becomes unschedulable again, we would add it back again to unschedulableTaskSets right? Wouldn't that be simpler? Do you see any issues with that?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657860016
> **[Test build #125783 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125783/testReport)** for PR 28287 at commit [`b4c27fb`](https://github.com/apache/spark/commit/b4c27fb2539d10356bb4208e0509f7a32a577613).
>
> * This patch **fails Spark unit tests**.
> * This patch merges cleanly.
> * This patch adds the following public classes _(experimental)_:
> * `case class UnschedulableTaskSetAdded(stageId: Int, stageAttemptId: Int)`
> * `case class UnschedulableTaskSetRemoved(stageId: Int, stageAttemptId: Int)`
> * `case class SparkListenerUnschedulableTaskSetAdded(`
> * `case class SparkListenerUnschedulableTaskSetRemoved(`
It seems like it fails every time with different test and everyone of them seems transient. @tgravescs I saw a recent change by @HyukjinKwon `SPARK-32245` to run Spark tests as Github actions primarily addressing the transient issues due to jenkins machines slowness. Do you think we should try running these tests using Github Actions? I am not sure how does that work. It would be great if you can help here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
mridulm commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r451701402
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -829,12 +868,25 @@ private[spark] class ExecutorAllocationManager(
numTotalTasks - numRunning
}
+ def pendingUnschedulableTasksPerResourceProfile(rp: Int): Int = {
Review comment:
We should either base additional allocations on:
* Number of unschedulable tasks; we need to make sure it is not aggressive overallocation.
* Do something similar to speculative tasks - so that there is atleast one executor allocation to make progress; this could result in taking longer to make progress.
Currently, we are using unschedulable tasksets as a proxy for number of unschedulable tasks - which is a hybrid of both of these approaches.
Also note that given the discussion on clearing `unschedulableTaskSets`, we could toggle between asking for more resources and asking for less (as soon as a taskset becomes schedulable).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659142963
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659693491
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-663138482
> merged to master, thanks @venkata91
Thanks @tgravescs for patiently doing multiple rounds of reviews. :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662458128
@venkata91 I assume your updates keep doing rebase? If possible its great if you can just do up merges because doing rebases makes seeing the diffs between requested changes much harder.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-617476515
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
mridulm commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r451701402
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -829,12 +868,25 @@ private[spark] class ExecutorAllocationManager(
numTotalTasks - numRunning
}
+ def pendingUnschedulableTasksPerResourceProfile(rp: Int): Int = {
Review comment:
We should either base additional allocations on:
* Number of unschedulable tasks - this could potentially result in overallocation
* Do something similar to speculative tasks - so that there is atleast one executor allocation to make progress.
Currently, we are using unschedulable tasksets as a proxy for number of unschedulable tasks - which is a hybrid of both of these approaches.
Also note that given the discussion on clearing `unschedulableTaskSets`, we could toggle between asking for more resources and asking for less (as soon as a taskset becomes schedulable).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r458133638
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -829,6 +872,23 @@ private[spark] class ExecutorAllocationManager(
numTotalTasks - numRunning
}
+ /**
+ * Currently we only know when a task set has an unschedulable task, we don't know
+ * the exact number and since the allocation manager isn't tied closely with the scheduler,
+ * we use the number of tasks sets that are unschedulable as a heuristic to add more executors.
+ */
+ def pendingUnschedulableTaskSetsPerResourceProfile(rp: Int): Int = {
+ val attempts = resourceProfileIdToStageAttempt.getOrElse(rp, Set.empty).toSeq
+ attempts.filter(attempt => unschedulableTaskSets.contains(attempt)).size
+ }
+
+ def hasPendingUnschedulableTasks: Boolean = {
Review comment:
This isn't used anymore, correct? Remove it if not
##########
File path: core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala
##########
@@ -1014,6 +1034,18 @@ private[spark] class DAGScheduler(
listenerBus.post(SparkListenerSpeculativeTaskSubmitted(task.stageId, task.stageAttemptId))
}
+ private[scheduler] def handleUnschedulableTaskSetAdded(
+ stageId: Int,
+ stageAttemptId: Int): Unit = {
+ listenerBus.post(SparkListenerUnschedulableTaskSetAdded(stageId, stageAttemptId))
+ }
+
+ private[scheduler] def handleUnschedulableTaskSetRemoved(
+ stageId: Int,
Review comment:
indentation here should be 4 spaces
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657764950
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r449801132
##########
File path: core/src/main/java/org/apache/spark/SparkFirehoseListener.java
##########
@@ -162,6 +162,11 @@ public void onSpeculativeTaskSubmitted(SparkListenerSpeculativeTaskSubmitted spe
onEvent(speculativeTask);
}
+ public void onUnschedulableBlacklistTaskSubmitted(
Review comment:
Done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657279972
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on issue #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on issue #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-617973718
> so took a quick look and I don't follow. You send a message that says all blacklisted, but all you do is call the onSchedulerBacklogged, how does that add another executor if they are all blacklisted? that will update when it needs to add more, but it doesn't change the number if calculated. So for instance say I needed 3 executors. All of them blacklisted, updating the time to get a new one won't change that it thinks it needs 3.
> If we change that to tell it to get one more, then my question is how do you do the proper accounting on that. This is why in the jira and other PR we said the allocation manager needs to be closely tied with the blacklist manager.
Ok. Got it, makes sense now. Let me think more about it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656679528
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662136865
**[Test build #126283 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126283/testReport)** for PR 28287 at commit [`d9f473d`](https://github.com/apache/spark/commit/d9f473d748b206426106a9c4e97e153aec2f508f).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-618744948
@tgravescs https://github.com/apache/spark/pull/22288 I went through the discussions as part of this PR. Is there any other PR you're referring to with discussions around Dynamic allocation and BlacklistManager? Also can you please explain a bit more on the proper accounting? Lets say we're requesting one executor (its possible, we're doing multiple rounds of requesting one executor) and also ensuring its with in the bound, won't dynamic allocation eventually get the num executors to consistent state with idle executors getting removed periodically?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r449798635
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -829,12 +868,25 @@ private[spark] class ExecutorAllocationManager(
numTotalTasks - numRunning
}
+ def pendingUnschedulableTasksPerResourceProfile(rp: Int): Int = {
Review comment:
I will refactor the name of this method to make it clear.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657279972
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659597488
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r449798696
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -696,7 +713,9 @@ private[spark] class ExecutorAllocationManager(
// If this is the last stage with pending tasks, mark the scheduler queue as empty
// This is needed in case the stage is aborted for any reason
- if (stageAttemptToNumTasks.isEmpty && stageAttemptToNumSpeculativeTasks.isEmpty) {
+ if (stageAttemptToNumTasks.isEmpty &&
+ stageAttemptToNumSpeculativeTasks.isEmpty &&
+ unschedulableTaskSets.isEmpty) {
Review comment:
Yes, thats right. Makes sense, will fix it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xkrogen commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
xkrogen commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r447133442
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -289,13 +290,23 @@ private[spark] class ExecutorAllocationManager(
s" tasksperexecutor: $tasksPerExecutor")
val maxNeeded = math.ceil(numRunningOrPendingTasks * executorAllocationRatio /
tasksPerExecutor).toInt
- if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
+ val totalNeed = if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
// If we have pending speculative tasks and only need a single executor, allocate one more
// to satisfy the locality requirements of speculation
maxNeeded + 1
} else {
maxNeeded
}
+
+ // Request additional executors to schedule the unschedulable tasks as well
+ if (numUnschedulables > 0) {
+ val maxNeededForUnschedulables = math.ceil(numUnschedulables * executorAllocationRatio /
+ tasksPerExecutor).toInt
+ math.max(totalNeed, executorMonitor.executorCountWithResourceProfile(rpId)) +
+ maxNeededForUnschedulables
+ } else {
Review comment:
I think this should be:
```
math.max(totalNeed, executorMonitor.executorCountWithResourceProfile(rpId) + maxNeededForUnschedules)
```
If we were already going to request `maxNeededForUnschedulables` _more_ executors than we currently have as part of `totalNeed`, that already satisfies the requirements.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659148755
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-647940759
@tgravescs After thinking about the problem and also after discussing with @mridulm, I have handled this problem now by just keeping track of unschedulable task sets in order to add more executors when dynamic allocation is enabled. Now once some task becomes schedulable, we'll clear this set since some executor got free or we have just acquired a new executor and found a way to make progress. Let me know what do you think about this change. Thanks for taking a look previously and giving the overall context
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-656679528
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657682658
> **[Test build #125723 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125723/testReport)** for PR 28287 at commit [`b4c27fb`](https://github.com/apache/spark/commit/b4c27fb2539d10356bb4208e0509f7a32a577613).
>
> * This patch **fails Spark unit tests**.
> * This patch merges cleanly.
> * This patch adds the following public classes _(experimental)_:
> * `case class UnschedulableTaskSetAdded(stageId: Int, stageAttemptId: Int)`
> * `case class UnschedulableTaskSetRemoved(stageId: Int, stageAttemptId: Int)`
> * `case class SparkListenerUnschedulableTaskSetAdded(`
> * `case class SparkListenerUnschedulableTaskSetRemoved(`
@tgravescs it seems like it again failed on a transient issue on one of the test. Shall we rerun this one?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-649017016
sorry haven't had a chance to look at your rework, I rekicked the checks as it might have been transient issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658308092
**[Test build #125838 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125838/testReport)** for PR 28287 at commit [`b4c27fb`](https://github.com/apache/spark/commit/b4c27fb2539d10356bb4208e0509f7a32a577613).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class UnschedulableTaskSetAdded(stageId: Int, stageAttemptId: Int)`
* `case class UnschedulableTaskSetRemoved(stageId: Int, stageAttemptId: Int)`
* `case class SparkListenerUnschedulableTaskSetAdded(`
* `case class SparkListenerUnschedulableTaskSetRemoved(`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-663137078
merged to master, thanks @venkata91
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 edited a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 edited a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658977388
> you can make a common function that has most of the code that gets called from 2 separate tests. one test passes with dynamic allocation on, the other with it off. that will reduce code duplication.
nevermind, I made some changes to the test so that it tests the dynamic allocation block of code properly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659426971
org.apache.spark.ExecutorAllocationManagerSuite failed in the GitHub Actions please take a look
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r455205236
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -289,13 +290,27 @@ private[spark] class ExecutorAllocationManager(
s" tasksperexecutor: $tasksPerExecutor")
val maxNeeded = math.ceil(numRunningOrPendingTasks * executorAllocationRatio /
tasksPerExecutor).toInt
- if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
+
+ val maxNeededWithSpeculationLocalityOffset =
+ if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
// If we have pending speculative tasks and only need a single executor, allocate one more
// to satisfy the locality requirements of speculation
maxNeeded + 1
} else {
maxNeeded
}
+
+ if (unschedulableTaskSets > 0) {
+ // Request additional executors only if maxNeededWithSpeculationLocalityOffset is less than
Review comment:
Thanks for the suggestion. This is much better.
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -829,12 +873,28 @@ private[spark] class ExecutorAllocationManager(
numTotalTasks - numRunning
}
+ // Currently TaskSetManager.getCompletelyBlacklistedTaskIfAny only takes the first unschedulable
+ // task found due to blacklisting. This way we only need to keep track of the unschedulable
+ // tasksets which is an indirect way to get the current number of unschedulable tasks.
Review comment:
Thanks for the suggestion!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662699290
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658308926
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657279814
**[Test build #125723 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125723/testReport)** for PR 28287 at commit [`b4c27fb`](https://github.com/apache/spark/commit/b4c27fb2539d10356bb4208e0509f7a32a577613).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class UnschedulableTaskSetAdded(stageId: Int, stageAttemptId: Int)`
* `case class UnschedulableTaskSetRemoved(stageId: Int, stageAttemptId: Int)`
* `case class SparkListenerUnschedulableTaskSetAdded(`
* `case class SparkListenerUnschedulableTaskSetRemoved(`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657690538
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662191204
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-662699290
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r448697727
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -289,13 +290,23 @@ private[spark] class ExecutorAllocationManager(
s" tasksperexecutor: $tasksPerExecutor")
val maxNeeded = math.ceil(numRunningOrPendingTasks * executorAllocationRatio /
tasksPerExecutor).toInt
- if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
+ val totalNeed = if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
// If we have pending speculative tasks and only need a single executor, allocate one more
// to satisfy the locality requirements of speculation
maxNeeded + 1
} else {
maxNeeded
}
+
+ // Request additional executors to schedule the unschedulable tasks as well
+ if (numUnschedulables > 0) {
+ val maxNeededForUnschedulables = math.ceil(numUnschedulables * executorAllocationRatio /
+ tasksPerExecutor).toInt
+ math.max(totalNeed, executorMonitor.executorCountWithResourceProfile(rpId)) +
Review comment:
Currently `ExecutorAllocationManager` runs every 100ms and `resourceOffer` happens every 1sec. `unschedulableTaskSets` gets cleared only if any task becomes schedulable and also we'll send the `unschedulable` event only once unless its removed from the `unschedulableTaskSets`. So I'm guessing it should be fine. Please correct me if I'm forgetting something.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659131779
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125919/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #28287:
URL: https://github.com/apache/spark/pull/28287#discussion_r451820344
##########
File path: core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala
##########
@@ -652,18 +652,26 @@ private[spark] class TaskSchedulerImpl(
case Some ((executorId, _)) =>
Review comment:
I think the above comment needs to be updated
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -281,6 +281,7 @@ private[spark] class ExecutorAllocationManager(
private def maxNumExecutorsNeededPerResourceProfile(rpId: Int): Int = {
val pending = listener.totalPendingTasksPerResourceProfile(rpId)
val pendingSpeculative = listener.pendingSpeculativeTasksPerResourceProfile(rpId)
+ val numUnschedulables = listener.pendingUnschedulableTaskSetsPerResourceProfile(rpId)
Review comment:
we should clarify that this is unschedulable task sets, not unschedulable number of tasks.
##########
File path: core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala
##########
@@ -311,6 +311,16 @@ private[spark] class DAGScheduler(
eventProcessLoop.post(SpeculativeTaskSubmitted(task))
}
+ /**
+ * Called by the TaskSetManager when there is an unschedulable blacklist task and dynamic
+ * allocation is enabled
+ */
+ def unschedulableBlacklistTaskSubmitted(
Review comment:
I think we should rename. Maybe just remove Submitted. since this isn't really Task Submission
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -789,6 +810,23 @@ private[spark] class ExecutorAllocationManager(
}
}
+ override def onUnschedulableTaskSet
+ (unschedulableTaskSet: SparkListenerUnschedulableTaskSet): Unit = {
Review comment:
nit, fix indentation and style. "(" should be on previous line, parameters indented 4 spaces
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -829,12 +867,28 @@ private[spark] class ExecutorAllocationManager(
numTotalTasks - numRunning
}
+ // Currently TaskSetManager.getCompletelyBlacklistedTaskIfAny only takes the first
+ // unschedulable task due to blacklisting. So keeping track of unschedulableTaskSets
+ // should be enough as we'll always have no more than a task unschedulable at any time.
Review comment:
I think this comment is confusing, specifically "as we'll always have no more than a task unschedulable at any time".
That isn't true. you could have lots of tasks blacklisted and thus unschedulable at any time. That function only returns the first one it finds.
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -789,6 +811,23 @@ private[spark] class ExecutorAllocationManager(
}
}
+ override def onUnschedulableBlacklistTaskSubmitted
+ (blacklistedTask: SparkListenerUnschedulableBlacklistTaskSubmitted): Unit = {
+ val stageId = blacklistedTask.stageId
+ val stageAttemptId = blacklistedTask.stageAttemptId
+ allocationManager.synchronized {
+ (stageId, stageAttemptId) match {
+ case (Some(stageId), Some(stageAttemptId)) =>
+ val stageAttempt = StageAttempt(stageId, stageAttemptId)
+ unschedulableTaskSets.add(stageAttempt)
+ case (None, _) =>
+ // Clear unschedulableTaskSets since atleast one task becomes schedulable now
+ unschedulableTaskSets.clear()
Review comment:
Yeah I could go either way on this as well. if we only remove the task set it could help get more faster if lots of blacklisted or you only requested a few to begin with. I think its a big out of the norm for us to rely on the parameters here being None as well, traditionally we would have 2 messages, 1 for it being added, one for it being removed. perhaps onUnschedulableTaskSetAdded and onUnschedulableTaskSetRemoved.
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -289,13 +290,26 @@ private[spark] class ExecutorAllocationManager(
s" tasksperexecutor: $tasksPerExecutor")
val maxNeeded = math.ceil(numRunningOrPendingTasks * executorAllocationRatio /
tasksPerExecutor).toInt
- if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
+ val maxNeededWithSpeculationLocalityOffset =
+ if (tasksPerExecutor > 1 && maxNeeded == 1 && pendingSpeculative > 0) {
// If we have pending speculative tasks and only need a single executor, allocate one more
// to satisfy the locality requirements of speculation
maxNeeded + 1
} else {
maxNeeded
}
+
+ // Since the maxNeededWithSpeculationLocalityOffset already includes the num executors needed
+ // to run unschedulable tasks, we would only try to add when the
+ // maxNeededWithSpeculationLocalityOffset is lesser than the active executors
Review comment:
this comment is also confusing to me. its also above a check for numUnschedulables, which has nothing to with the comment. Can we move it down next to the check on line 308 and clarify. I think you mean we don't add any tasks for the unschedulables if active executors is less than the number we are already asking for.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658179776
**[Test build #125838 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125838/testReport)** for PR 28287 at commit [`b4c27fb`](https://github.com/apache/spark/commit/b4c27fb2539d10356bb4208e0509f7a32a577613).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659131774
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659596725
**[Test build #125996 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125996/testReport)** for PR 28287 at commit [`0d07845`](https://github.com/apache/spark/commit/0d07845459e6e0f606e5f0920c973de256a309e1).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657690538
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-657263420
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659131260
**[Test build #125919 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125919/testReport)** for PR 28287 at commit [`112bb3a`](https://github.com/apache/spark/commit/112bb3a3e72c664f2cbb5bebca0265f5e9c23303).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659083280
**[Test build #125918 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125918/testReport)** for PR 28287 at commit [`e167da5`](https://github.com/apache/spark/commit/e167da53b0b8ffde746c86e0564d7971363f3746).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-617476121
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-659805515
phew! Finally the tests are passing
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #28287: [SPARK-31418][SCHEDULER] Request more executors in case of dynamic allocation is enabled and a task becomes unschedulable due to spark's blacklisting feature.
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #28287:
URL: https://github.com/apache/spark/pull/28287#issuecomment-658974291
you can make a common function that has most of the code that gets called from 2 separate tests. one test passes with dynamic allocation on, the other with it off. that will reduce code duplication.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org