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/04/23 03:25:07 UTC
[GitHub] [spark] weixiuli opened a new pull request #32306: [SPARK-35200][CORE] Avoid to recompute the pending speculative tasks in the ExecutorAllocationManager and remove some unnecessary code.
weixiuli opened a new pull request #32306:
URL: https://github.com/apache/spark/pull/32306
### What changes were proposed in this pull request?
Avoid to recompute the pending speculative tasks in the ExecutorAllocationManager, and remove some unnecessary code.
### Why are the changes needed?
The number of the pending speculative tasks is recomputed in the ExecutorAllocationManager to calculate the maximum number of executors required. While , it only needs to be computed once to improve performance.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Existing 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.
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] weixiuli commented on a change in pull request #32306: [SPARK-35200][CORE] Avoid to recompute the pending speculative tasks in the ExecutorAllocationManager and remove some unnecessary code.
Posted by GitBox <gi...@apache.org>.
weixiuli commented on a change in pull request #32306:
URL: https://github.com/apache/spark/pull/32306#discussion_r618942209
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -290,11 +290,11 @@ private[spark] class ExecutorAllocationManager(
* under the current load to satisfy all running and pending tasks, rounded up.
*/
private[spark] def maxNumExecutorsNeededPerResourceProfile(rpId: Int): Int = {
- val pending = listener.totalPendingTasksPerResourceProfile(rpId)
+ val pendingTask = listener.pendingTasksPerResourceProfile(rpId)
val pendingSpeculative = listener.pendingSpeculativeTasksPerResourceProfile(rpId)
val unschedulableTaskSets = listener.pendingUnschedulableTaskSetsPerResourceProfile(rpId)
val running = listener.totalRunningTasksPerResourceProfile(rpId)
- val numRunningOrPendingTasks = pending + running
+ val numRunningOrPendingTasks = pendingTask + pendingSpeculative + running
Review comment:
previously the `pendingSpeculativeTasksPerResourceProfile(rp)` will be called not only by `totalPendingTasksPerResourceProfile(rpId)` but also by ` val pendingSpeculative = listener.pendingSpeculativeTasksPerResourceProfile(rpId)`, this PR we only call the `pendingSpeculativeTasksPerResourceProfile(rp) ` once to get the pendingSpeculativeTasks , and use it for `numRunningOrPendingTasks` and `pendingSpeculative`.
--
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] cloud-fan commented on a change in pull request #32306: [SPARK-35200][CORE] Avoid to recompute the pending speculative tasks in the ExecutorAllocationManager and remove some unnecessary code.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32306:
URL: https://github.com/apache/spark/pull/32306#discussion_r618935372
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -290,11 +290,11 @@ private[spark] class ExecutorAllocationManager(
* under the current load to satisfy all running and pending tasks, rounded up.
*/
private[spark] def maxNumExecutorsNeededPerResourceProfile(rpId: Int): Int = {
- val pending = listener.totalPendingTasksPerResourceProfile(rpId)
+ val pendingTask = listener.pendingTasksPerResourceProfile(rpId)
val pendingSpeculative = listener.pendingSpeculativeTasksPerResourceProfile(rpId)
val unschedulableTaskSets = listener.pendingUnschedulableTaskSetsPerResourceProfile(rpId)
val running = listener.totalRunningTasksPerResourceProfile(rpId)
- val numRunningOrPendingTasks = pending + running
+ val numRunningOrPendingTasks = pendingTask + pendingSpeculative + running
Review comment:
previously `pending` is `pendingTasksPerResourceProfile(rp) + pendingSpeculativeTasksPerResourceProfile(rp)`, now we use `pendingTasksPerResourceProfile() + pendingSpeculative`, and `pendingSpeculative` simply calls `pendingSpeculativeTasksPerResourceProfile(...)`
seems like a straightforward code clean up.
--
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] dongjoon-hyun closed pull request #32306: [SPARK-35200][CORE] Avoid to recompute the pending speculative tasks in the ExecutorAllocationManager and remove some unnecessary code.
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #32306:
URL: https://github.com/apache/spark/pull/32306
--
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] dongjoon-hyun commented on pull request #32306: [SPARK-35200][CORE] Avoid to recompute the pending speculative tasks in the ExecutorAllocationManager and remove some unnecessary code.
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32306:
URL: https://github.com/apache/spark/pull/32306#issuecomment-826156238
Thank you, @weixiuli and all.
Merged to master for Apache Spark 3.2.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.
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] Ngone51 commented on pull request #32306: [SPARK-35200][CORE] Avoid to recompute the pending speculative tasks in the ExecutorAllocationManager and remove some unnecessary code.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on pull request #32306:
URL: https://github.com/apache/spark/pull/32306#issuecomment-825391073
cc @tgravescs
--
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 #32306: [SPARK-35200][CORE] Avoid to recompute the pending speculative tasks in the ExecutorAllocationManager and remove some unnecessary code.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32306:
URL: https://github.com/apache/spark/pull/32306#issuecomment-825359749
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] dongjoon-hyun commented on pull request #32306: [SPARK-35200][CORE] Avoid to recompute the pending speculative tasks in the ExecutorAllocationManager and remove some unnecessary code.
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32306:
URL: https://github.com/apache/spark/pull/32306#issuecomment-826156238
Thank you, @weixiuli and all.
Merged to master for Apache Spark 3.2.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.
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 #32306: [SPARK-35200][CORE] Avoid to recompute the pending speculative tasks in the ExecutorAllocationManager and remove some unnecessary code.
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #32306:
URL: https://github.com/apache/spark/pull/32306
--
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] weixiuli commented on pull request #32306: [SPARK-35200][CORE] Avoid to recompute the pending speculative tasks in the ExecutorAllocationManager and remove some unnecessary code.
Posted by GitBox <gi...@apache.org>.
weixiuli commented on pull request #32306:
URL: https://github.com/apache/spark/pull/32306#issuecomment-825361223
@cloud-fan @Ngone51 Kindly review, 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