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 2021/07/23 08:49:05 UTC
[GitHub] [spark] attilapiros opened a new pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
attilapiros opened a new pull request #33492:
URL: https://github.com/apache/spark/pull/33492
### What changes were proposed in this pull request?
Introducing a limit for pending PODs (newly created/requested executors included).
### Why are the changes needed?
Without this PR dynamic allocation could request too many PODs and the K8S scheduler could be overloaded and scheduling of PODs will be affected by the load.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
With new unit tests.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun edited a comment on pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-900367204
Thank you, @gengliangwang . According to your suggestion, I added `releasenotes` label to SPARK-36052.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r678728865
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -347,14 +373,10 @@ private[spark] class ExecutorPodsAllocator(
}
private def requestNewExecutors(
- expected: Int,
- running: Int,
+ numExecutorsToAllocate: Int,
applicationId: String,
resourceProfileId: Int,
pvcsInUse: Seq[String]): Unit = {
- val numExecutorsToAllocate = math.min(expected - running, podAllocationSize)
- logInfo(s"Going to request $numExecutorsToAllocate executors from Kubernetes for " +
- s"ResourceProfile Id: $resourceProfileId, target: $expected running: $running.")
Review comment:
This message had better be here inside `requestNewExecutors`.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-889144997
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46345/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun commented on pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-899874316
Hi, @attilapiros and @holdenk .
I also hit this issue in the production environment when the PVC creation pending.
I'll backport this to `branch-3.2` to fix the bug.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r675425508
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -217,9 +219,15 @@ private[spark] class ExecutorPodsAllocator(
}
}
+ // sum of all the pending pods unknown by the scheduler (total for all the resources)
var totalPendingCount = 0
- // The order we request executors for each ResourceProfile is not guaranteed.
- totalExpectedExecutorsPerResourceProfileId.asScala.foreach { case (rpId, targetNum) =>
+ // total not running pods (including scheduler known & unknown, pending & newly requested ones)
+ var totalNotRunningPodCount = 0
+ val podsToAllocateWithRpId = totalExpectedExecutorsPerResourceProfileId
+ .asScala
+ .toSeq
+ .sortBy(_._1)
+ .flatMap { case (rpId, targetNum) =>
Review comment:
Instead of `foreach` here is `flatMap` as we need to do the process in two steps for counting all the not running PODs for all the resource profiles before we decide how to split the remaining pending PODs slot between the resource profiles.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885531446
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46066/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885552341
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46067/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r678727346
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -293,32 +302,49 @@ private[spark] class ExecutorPodsAllocator(
.withLabelIn(SPARK_EXECUTOR_ID_LABEL, toDelete.sorted.map(_.toString): _*)
.delete()
newlyCreatedExecutors --= newlyCreatedToDelete
- knownPendingCount -= knownPendingToDelete.size
+ pendingCountForRpId -= pendingToDelete.size
+ notRunningPodCountForRpId -= toDelete.size
}
}
}
-
- if (newlyCreatedExecutorsForRpId.isEmpty
- && knownPodCount < targetNum) {
- requestNewExecutors(targetNum, knownPodCount, applicationId, rpId, k8sKnownPVCNames)
- }
- totalPendingCount += knownPendingCount
+ totalPendingCount += pendingCountForRpId
+ totalNotRunningPodCount += notRunningPodCountForRpId
// The code below just prints debug messages, which are only useful when there's a change
// in the snapshot state. Since the messages are a little spammy, avoid them when we know
// there are no useful updates.
if (log.isDebugEnabled && snapshots.nonEmpty) {
- val outstanding = knownPendingCount + newlyCreatedExecutorsForRpId.size
+ val outstanding = pendingCountForRpId + newlyCreatedExecutorsForRpId.size
if (currentRunningCount >= targetNum && !dynamicAllocationEnabled) {
logDebug(s"Current number of running executors for ResourceProfile Id $rpId is " +
"equal to the number of requested executors. Not scaling up further.")
} else {
- if (outstanding > 0) {
- logDebug(s"Still waiting for $outstanding executors for ResourceProfile " +
- s"Id $rpId before requesting more.")
+ if (newlyCreatedExecutorsForRpId.nonEmpty) {
+ logDebug(s"Still waiting for ${newlyCreatedExecutorsForRpId.size} executors for " +
+ s"ResourceProfile Id $rpId before requesting more.")
}
}
}
+ if (newlyCreatedExecutorsForRpId.isEmpty && podCountForRpId < targetNum) {
+ Some(rpId, podCountForRpId, targetNum)
+ } else {
+ // for this resource profile we do not request more PODs
+ None
Review comment:
I removed the previous my comment because I'm also not sure what is the best way to inform the users this situation. Do you think we have a good way to inform to the users when we hit this limitation, @attilapiros ?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-888994063
> In general, it looks like a good improvement. BTW, I'm curious if you are going to support maxPendingPods per resource profile later.
I am happy you like this.
Actually I haven't thought about supporting maxPendingPods per resource profile but if you think that would be valuable for our users I can do that easily in a new PR.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-889075756
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46345/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r675408649
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -264,21 +272,22 @@ private[spark] class ExecutorPodsAllocator(
//
// TODO: with dynamic allocation off, handle edge cases if we end up with more running
// executors than expected.
- val knownPodCount = currentRunningCount +
+ var notRunningPodCountForRpId =
currentPendingExecutorsForRpId.size + schedulerKnownPendingExecsForRpId.size +
newlyCreatedExecutorsForRpId.size + schedulerKnownNewlyCreatedExecsForRpId.size
+ val podCountForRpId = currentRunningCount + notRunningPodCountForRpId
Review comment:
This again a rename to avoid "known" prefix as it not for scheduler known PODs but PODs for this resource profile.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r675409750
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -264,21 +272,22 @@ private[spark] class ExecutorPodsAllocator(
//
// TODO: with dynamic allocation off, handle edge cases if we end up with more running
// executors than expected.
- val knownPodCount = currentRunningCount +
+ var notRunningPodCountForRpId =
currentPendingExecutorsForRpId.size + schedulerKnownPendingExecsForRpId.size +
newlyCreatedExecutorsForRpId.size + schedulerKnownNewlyCreatedExecsForRpId.size
+ val podCountForRpId = currentRunningCount + notRunningPodCountForRpId
- if (knownPodCount > targetNum) {
- val excess = knownPodCount - targetNum
+ if (podCountForRpId > targetNum) {
+ val excess = podCountForRpId - targetNum
val newlyCreatedToDelete = newlyCreatedExecutorsForRpId
.filter { case (_, (_, createTime)) =>
currentTime - createTime > executorIdleTimeout
}.keys.take(excess).toList
- val knownPendingToDelete = currentPendingExecutorsForRpId
+ val pendingToDelete = currentPendingExecutorsForRpId
Review comment:
Last rename for the same reason as earlier: this are PODs unknow by the scheduler so safe to be removed here (no task can be scheduled on them).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-896496519
@dongjoon-hyun, @holdenk Thanks to all of you!
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun commented on pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-899874595
cc @gengliangwang , too
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885511204
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141548/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r675424047
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -293,32 +302,49 @@ private[spark] class ExecutorPodsAllocator(
.withLabelIn(SPARK_EXECUTOR_ID_LABEL, toDelete.sorted.map(_.toString): _*)
.delete()
newlyCreatedExecutors --= newlyCreatedToDelete
- knownPendingCount -= knownPendingToDelete.size
+ pendingCountForRpId -= pendingToDelete.size
+ notRunningPodCountForRpId -= toDelete.size
}
}
}
-
- if (newlyCreatedExecutorsForRpId.isEmpty
- && knownPodCount < targetNum) {
- requestNewExecutors(targetNum, knownPodCount, applicationId, rpId, k8sKnownPVCNames)
- }
- totalPendingCount += knownPendingCount
+ totalPendingCount += pendingCountForRpId
+ totalNotRunningPodCount += notRunningPodCountForRpId
// The code below just prints debug messages, which are only useful when there's a change
// in the snapshot state. Since the messages are a little spammy, avoid them when we know
// there are no useful updates.
if (log.isDebugEnabled && snapshots.nonEmpty) {
- val outstanding = knownPendingCount + newlyCreatedExecutorsForRpId.size
+ val outstanding = pendingCountForRpId + newlyCreatedExecutorsForRpId.size
if (currentRunningCount >= targetNum && !dynamicAllocationEnabled) {
logDebug(s"Current number of running executors for ResourceProfile Id $rpId is " +
"equal to the number of requested executors. Not scaling up further.")
} else {
- if (outstanding > 0) {
- logDebug(s"Still waiting for $outstanding executors for ResourceProfile " +
- s"Id $rpId before requesting more.")
+ if (newlyCreatedExecutorsForRpId.nonEmpty) {
Review comment:
The condition previously used "outstanding" which contains pending (unknown by the scheduler) PODs as well but the old condition to call `requestNewExecutors` was:
```
if (newlyCreatedExecutorsForRpId.isEmpty
&& knownPodCount < targetNum) {
requestNewExecutors(targetNum, knownPodCount, applicationId, rpId, k8sKnownPVCNames)
}
```
So here really the emptiness of `newlyCreatedExecutorsForRpId` should be checked.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885511677
**[Test build #141549 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141549/testReport)** for PR 33492 at commit [`adc512d`](https://github.com/apache/spark/commit/adc512d4e1837713713fefc6f64af3b0c6c8cdc8).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r675407048
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -235,7 +243,7 @@ private[spark] class ExecutorPodsAllocator(
}
// This variable is used later to print some debug logs. It's updated when cleaning up
// excess pod requests, since currentPendingExecutorsForRpId is immutable.
- var knownPendingCount = currentPendingExecutorsForRpId.size
+ var pendingCountForRpId = currentPendingExecutorsForRpId.size
Review comment:
This is a rename but the old name was misleading as it counts the pending PODs which unknown by the scheduler.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885511677
**[Test build #141549 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141549/testReport)** for PR 33492 at commit [`adc512d`](https://github.com/apache/spark/commit/adc512d4e1837713713fefc6f64af3b0c6c8cdc8).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-889039566
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141832/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885511204
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141548/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-889018622
**[Test build #141832 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141832/testReport)** for PR 33492 at commit [`2e78f6c`](https://github.com/apache/spark/commit/2e78f6c08a8a066cc52123c35a5de852b92457c4).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] gengliangwang commented on pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-899952978
@attilapiros Thanks for the work.
@dongjoon-hyun Thanks for pinging me. This one should be in the release note.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885550545
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46066/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] holdenk commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
holdenk commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r678536355
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
##########
@@ -568,6 +568,16 @@ private[spark] object Config extends Logging {
.checkValue(delay => delay > 0, "delay must be a positive time value")
.createWithDefaultString("30s")
+ val KUBERNETES_MAX_PENDING_PODS =
+ ConfigBuilder("spark.kubernetes.allocation.maxPendingPods")
+ .doc("Maximum number of pending PODs allowed during executor allocation for this " +
+ "application. Those newly requested executors which are unknown by Kubernetes yet are " +
+ "also counted into this limit as they will change into pending PODs by time.")
Review comment:
Makes sense, can we still include the note to clarify how this interacts with the existing per-RPI allocations? Since at first read I didn't pick up on that and I imagine other folks might miss that too.
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -293,32 +302,49 @@ private[spark] class ExecutorPodsAllocator(
.withLabelIn(SPARK_EXECUTOR_ID_LABEL, toDelete.sorted.map(_.toString): _*)
.delete()
newlyCreatedExecutors --= newlyCreatedToDelete
- knownPendingCount -= knownPendingToDelete.size
+ pendingCountForRpId -= pendingToDelete.size
+ notRunningPodCountForRpId -= toDelete.size
}
}
}
-
- if (newlyCreatedExecutorsForRpId.isEmpty
- && knownPodCount < targetNum) {
- requestNewExecutors(targetNum, knownPodCount, applicationId, rpId, k8sKnownPVCNames)
- }
- totalPendingCount += knownPendingCount
+ totalPendingCount += pendingCountForRpId
+ totalNotRunningPodCount += notRunningPodCountForRpId
// The code below just prints debug messages, which are only useful when there's a change
// in the snapshot state. Since the messages are a little spammy, avoid them when we know
// there are no useful updates.
if (log.isDebugEnabled && snapshots.nonEmpty) {
- val outstanding = knownPendingCount + newlyCreatedExecutorsForRpId.size
+ val outstanding = pendingCountForRpId + newlyCreatedExecutorsForRpId.size
if (currentRunningCount >= targetNum && !dynamicAllocationEnabled) {
logDebug(s"Current number of running executors for ResourceProfile Id $rpId is " +
"equal to the number of requested executors. Not scaling up further.")
} else {
- if (outstanding > 0) {
- logDebug(s"Still waiting for $outstanding executors for ResourceProfile " +
- s"Id $rpId before requesting more.")
+ if (newlyCreatedExecutorsForRpId.nonEmpty) {
+ logDebug(s"Still waiting for ${newlyCreatedExecutorsForRpId.size} executors for " +
+ s"ResourceProfile Id $rpId before requesting more.")
}
}
}
+ if (newlyCreatedExecutorsForRpId.isEmpty && podCountForRpId < targetNum) {
+ Some(rpId, podCountForRpId, targetNum)
+ } else {
+ // for this resource profile we do not request more PODs
+ None
+ }
+ }
Review comment:
Cool :)
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r681616371
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -347,14 +373,10 @@ private[spark] class ExecutorPodsAllocator(
}
private def requestNewExecutors(
- expected: Int,
- running: Int,
+ numExecutorsToAllocate: Int,
applicationId: String,
resourceProfileId: Int,
pvcsInUse: Seq[String]): Unit = {
- val numExecutorsToAllocate = math.min(expected - running, podAllocationSize)
- logInfo(s"Going to request $numExecutorsToAllocate executors from Kubernetes for " +
- s"ResourceProfile Id: $resourceProfileId, target: $expected running: $running.")
Review comment:
@dongjoon-hyun what is your opinion?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-891754968
@holdenk, @dongjoon-hyun may I ask one more pass?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885563877
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885510988
**[Test build #141548 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141548/testReport)** for PR 33492 at commit [`269a0cd`](https://github.com/apache/spark/commit/269a0cda49c27d6ba58cda576389c4ed35070292).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885535098
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46067/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r675961263
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
##########
@@ -568,6 +568,16 @@ private[spark] object Config extends Logging {
.checkValue(delay => delay > 0, "delay must be a positive time value")
.createWithDefaultString("30s")
+ val KUBERNETES_MAX_PENDING_PODS =
+ ConfigBuilder("spark.kubernetes.allocation.maxPendingPods")
+ .doc("Maximum number of pending PODs allowed during executor allocation for this " +
+ "application. Those newly requested executors which are unknown by Kubernetes yet are " +
+ "also counted into this limit as they will change into pending PODs by time.")
Review comment:
I was not intend to change the old behavior. So this new limit is global for all the resources profiles within one Spark app because on the Kubernetes point of view it is better to have one global limit than limiting only the resource profiles separately. In the latter case for example an earlier limit set to N would be multiplied by just changing the app and using an extra resource profile. Moreover if some resource profiles just active in a few stages then the user might need to choose a lower limit but latter when those profiles are not active then the low limit won't be sufficient and the active ones cannot use those free slots.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885518895
**[Test build #141549 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141549/testReport)** for PR 33492 at commit [`adc512d`](https://github.com/apache/spark/commit/adc512d4e1837713713fefc6f64af3b0c6c8cdc8).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-889126171
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46345/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885572220
The failure is a tpc-ds query run which must be unrelated.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun closed pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #33492:
URL: https://github.com/apache/spark/pull/33492
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-889144997
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46345/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun commented on pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-900367204
Thank you, @gengliangwang . I added `releasenotes` label to SPARK-36052.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun commented on pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-899878208
- During backporting, I updated the config version 3.2.0 at branch-3.2.
- This is a follow-up PR for `master` branch, https://github.com/apache/spark/pull/33755, to match 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-889005797
**[Test build #141832 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141832/testReport)** for PR 33492 at commit [`2e78f6c`](https://github.com/apache/spark/commit/2e78f6c08a8a066cc52123c35a5de852b92457c4).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885563875
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun commented on pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885701365
Thank you for pinging me, @attilapiros .
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r679020395
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -347,14 +373,10 @@ private[spark] class ExecutorPodsAllocator(
}
private def requestNewExecutors(
- expected: Int,
- running: Int,
+ numExecutorsToAllocate: Int,
applicationId: String,
resourceProfileId: Int,
pvcsInUse: Seq[String]): Unit = {
- val numExecutorsToAllocate = math.min(expected - running, podAllocationSize)
- logInfo(s"Going to request $numExecutorsToAllocate executors from Kubernetes for " +
- s"ResourceProfile Id: $resourceProfileId, target: $expected running: $running.")
Review comment:
My reason to change it was to avoid passing more variables for that method.
So from this
https://github.com/apache/spark/blob/adc512d4e1837713713fefc6f64af3b0c6c8cdc8/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala#L343-L346
we would need to pass:
- `targetNum`
- `podCountForRpId`
- `sharedSlotFromPendingPods`
and they only needed for the log line and to calculate `numExecutorsToAllocate`.
With the current solution `numExecutorsToAllocate` is enough and when we will extend the current logic to consider more limits to use for allocation then `numExecutorsToAllocate` will be still enough.
WDYT?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-889039566
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141832/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885502904
**[Test build #141548 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141548/testReport)** for PR 33492 at commit [`269a0cd`](https://github.com/apache/spark/commit/269a0cda49c27d6ba58cda576389c4ed35070292).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r678725534
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -293,32 +302,49 @@ private[spark] class ExecutorPodsAllocator(
.withLabelIn(SPARK_EXECUTOR_ID_LABEL, toDelete.sorted.map(_.toString): _*)
.delete()
newlyCreatedExecutors --= newlyCreatedToDelete
- knownPendingCount -= knownPendingToDelete.size
+ pendingCountForRpId -= pendingToDelete.size
+ notRunningPodCountForRpId -= toDelete.size
}
}
}
-
- if (newlyCreatedExecutorsForRpId.isEmpty
- && knownPodCount < targetNum) {
- requestNewExecutors(targetNum, knownPodCount, applicationId, rpId, k8sKnownPVCNames)
- }
- totalPendingCount += knownPendingCount
+ totalPendingCount += pendingCountForRpId
+ totalNotRunningPodCount += notRunningPodCountForRpId
// The code below just prints debug messages, which are only useful when there's a change
// in the snapshot state. Since the messages are a little spammy, avoid them when we know
// there are no useful updates.
if (log.isDebugEnabled && snapshots.nonEmpty) {
- val outstanding = knownPendingCount + newlyCreatedExecutorsForRpId.size
+ val outstanding = pendingCountForRpId + newlyCreatedExecutorsForRpId.size
if (currentRunningCount >= targetNum && !dynamicAllocationEnabled) {
logDebug(s"Current number of running executors for ResourceProfile Id $rpId is " +
"equal to the number of requested executors. Not scaling up further.")
} else {
- if (outstanding > 0) {
- logDebug(s"Still waiting for $outstanding executors for ResourceProfile " +
- s"Id $rpId before requesting more.")
+ if (newlyCreatedExecutorsForRpId.nonEmpty) {
+ logDebug(s"Still waiting for ${newlyCreatedExecutorsForRpId.size} executors for " +
+ s"ResourceProfile Id $rpId before requesting more.")
}
}
}
+ if (newlyCreatedExecutorsForRpId.isEmpty && podCountForRpId < targetNum) {
+ Some(rpId, podCountForRpId, targetNum)
+ } else {
+ // for this resource profile we do not request more PODs
Review comment:
Shall we add an info message 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885502904
**[Test build #141548 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141548/testReport)** for PR 33492 at commit [`269a0cd`](https://github.com/apache/spark/commit/269a0cda49c27d6ba58cda576389c4ed35070292).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885534080
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141549/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r675962645
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -293,32 +302,49 @@ private[spark] class ExecutorPodsAllocator(
.withLabelIn(SPARK_EXECUTOR_ID_LABEL, toDelete.sorted.map(_.toString): _*)
.delete()
newlyCreatedExecutors --= newlyCreatedToDelete
- knownPendingCount -= knownPendingToDelete.size
+ pendingCountForRpId -= pendingToDelete.size
+ notRunningPodCountForRpId -= toDelete.size
}
}
}
-
- if (newlyCreatedExecutorsForRpId.isEmpty
- && knownPodCount < targetNum) {
- requestNewExecutors(targetNum, knownPodCount, applicationId, rpId, k8sKnownPVCNames)
- }
- totalPendingCount += knownPendingCount
+ totalPendingCount += pendingCountForRpId
+ totalNotRunningPodCount += notRunningPodCountForRpId
// The code below just prints debug messages, which are only useful when there's a change
// in the snapshot state. Since the messages are a little spammy, avoid them when we know
// there are no useful updates.
if (log.isDebugEnabled && snapshots.nonEmpty) {
- val outstanding = knownPendingCount + newlyCreatedExecutorsForRpId.size
+ val outstanding = pendingCountForRpId + newlyCreatedExecutorsForRpId.size
if (currentRunningCount >= targetNum && !dynamicAllocationEnabled) {
logDebug(s"Current number of running executors for ResourceProfile Id $rpId is " +
"equal to the number of requested executors. Not scaling up further.")
} else {
- if (outstanding > 0) {
- logDebug(s"Still waiting for $outstanding executors for ResourceProfile " +
- s"Id $rpId before requesting more.")
+ if (newlyCreatedExecutorsForRpId.nonEmpty) {
+ logDebug(s"Still waiting for ${newlyCreatedExecutorsForRpId.size} executors for " +
+ s"ResourceProfile Id $rpId before requesting more.")
}
}
}
+ if (newlyCreatedExecutorsForRpId.isEmpty && podCountForRpId < targetNum) {
+ Some(rpId, podCountForRpId, targetNum)
+ } else {
+ // for this resource profile we do not request more PODs
+ None
+ }
+ }
Review comment:
This is kept intentionally, see https://github.com/apache/spark/pull/33492#discussion_r675961263.
The new limit is enforced at https://github.com/apache/spark/blob/adc512d4e1837713713fefc6f64af3b0c6c8cdc8/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala#L336-L339
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] holdenk commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
holdenk commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r675906423
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -293,32 +302,49 @@ private[spark] class ExecutorPodsAllocator(
.withLabelIn(SPARK_EXECUTOR_ID_LABEL, toDelete.sorted.map(_.toString): _*)
.delete()
newlyCreatedExecutors --= newlyCreatedToDelete
- knownPendingCount -= knownPendingToDelete.size
+ pendingCountForRpId -= pendingToDelete.size
+ notRunningPodCountForRpId -= toDelete.size
}
}
}
-
- if (newlyCreatedExecutorsForRpId.isEmpty
- && knownPodCount < targetNum) {
- requestNewExecutors(targetNum, knownPodCount, applicationId, rpId, k8sKnownPVCNames)
- }
- totalPendingCount += knownPendingCount
+ totalPendingCount += pendingCountForRpId
+ totalNotRunningPodCount += notRunningPodCountForRpId
// The code below just prints debug messages, which are only useful when there's a change
// in the snapshot state. Since the messages are a little spammy, avoid them when we know
// there are no useful updates.
if (log.isDebugEnabled && snapshots.nonEmpty) {
- val outstanding = knownPendingCount + newlyCreatedExecutorsForRpId.size
+ val outstanding = pendingCountForRpId + newlyCreatedExecutorsForRpId.size
if (currentRunningCount >= targetNum && !dynamicAllocationEnabled) {
logDebug(s"Current number of running executors for ResourceProfile Id $rpId is " +
"equal to the number of requested executors. Not scaling up further.")
} else {
- if (outstanding > 0) {
- logDebug(s"Still waiting for $outstanding executors for ResourceProfile " +
- s"Id $rpId before requesting more.")
+ if (newlyCreatedExecutorsForRpId.nonEmpty) {
+ logDebug(s"Still waiting for ${newlyCreatedExecutorsForRpId.size} executors for " +
+ s"ResourceProfile Id $rpId before requesting more.")
}
}
}
+ if (newlyCreatedExecutorsForRpId.isEmpty && podCountForRpId < targetNum) {
+ Some(rpId, podCountForRpId, targetNum)
+ } else {
+ // for this resource profile we do not request more PODs
+ None
+ }
+ }
Review comment:
I get this is the old behaviour, but I would think that we might allocate up to max pending so requiring newlyCreated to be empty seems strange to me.
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
##########
@@ -568,6 +568,16 @@ private[spark] object Config extends Logging {
.checkValue(delay => delay > 0, "delay must be a positive time value")
.createWithDefaultString("30s")
+ val KUBERNETES_MAX_PENDING_PODS =
+ ConfigBuilder("spark.kubernetes.allocation.maxPendingPods")
+ .doc("Maximum number of pending PODs allowed during executor allocation for this " +
+ "application. Those newly requested executors which are unknown by Kubernetes yet are " +
+ "also counted into this limit as they will change into pending PODs by time.")
Review comment:
Maybe include a note that if there are pending pods for a given resource profile we won't allocate more pods for the rpi since on my first reading of this I assumed this flag changed that behaviour.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885534080
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141549/
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-889005797
**[Test build #141832 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141832/testReport)** for PR 33492 at commit [`2e78f6c`](https://github.com/apache/spark/commit/2e78f6c08a8a066cc52123c35a5de852b92457c4).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on a change in pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #33492:
URL: https://github.com/apache/spark/pull/33492#discussion_r679028868
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -293,32 +302,49 @@ private[spark] class ExecutorPodsAllocator(
.withLabelIn(SPARK_EXECUTOR_ID_LABEL, toDelete.sorted.map(_.toString): _*)
.delete()
newlyCreatedExecutors --= newlyCreatedToDelete
- knownPendingCount -= knownPendingToDelete.size
+ pendingCountForRpId -= pendingToDelete.size
+ notRunningPodCountForRpId -= toDelete.size
}
}
}
-
- if (newlyCreatedExecutorsForRpId.isEmpty
- && knownPodCount < targetNum) {
- requestNewExecutors(targetNum, knownPodCount, applicationId, rpId, k8sKnownPVCNames)
- }
- totalPendingCount += knownPendingCount
+ totalPendingCount += pendingCountForRpId
+ totalNotRunningPodCount += notRunningPodCountForRpId
// The code below just prints debug messages, which are only useful when there's a change
// in the snapshot state. Since the messages are a little spammy, avoid them when we know
// there are no useful updates.
if (log.isDebugEnabled && snapshots.nonEmpty) {
- val outstanding = knownPendingCount + newlyCreatedExecutorsForRpId.size
+ val outstanding = pendingCountForRpId + newlyCreatedExecutorsForRpId.size
if (currentRunningCount >= targetNum && !dynamicAllocationEnabled) {
logDebug(s"Current number of running executors for ResourceProfile Id $rpId is " +
"equal to the number of requested executors. Not scaling up further.")
} else {
- if (outstanding > 0) {
- logDebug(s"Still waiting for $outstanding executors for ResourceProfile " +
- s"Id $rpId before requesting more.")
+ if (newlyCreatedExecutorsForRpId.nonEmpty) {
+ logDebug(s"Still waiting for ${newlyCreatedExecutorsForRpId.size} executors for " +
+ s"ResourceProfile Id $rpId before requesting more.")
}
}
}
+ if (newlyCreatedExecutorsForRpId.isEmpty && podCountForRpId < targetNum) {
+ Some(rpId, podCountForRpId, targetNum)
+ } else {
+ // for this resource profile we do not request more PODs
+ None
Review comment:
We could change this to logInfo:
https://github.com/apache/spark/blob/adc512d4e1837713713fefc6f64af3b0c6c8cdc8/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala#L323-L324
But for a higher batch allocation size this message could be annoying as every POD status change will generate such a log line while it reaches 0.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] attilapiros commented on pull request #33492: [SPARK-36052][K8S] Introducing a limit for pending PODs
Posted by GitBox <gi...@apache.org>.
attilapiros commented on pull request #33492:
URL: https://github.com/apache/spark/pull/33492#issuecomment-885514802
cc @holdenk, @dongjoon-hyun
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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