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 2022/08/30 17:45:53 UTC
[GitHub] [spark] warrenzhu25 opened a new pull request, #37733: [SPARK-40267][DOC] Add description for ExecutorAllocationManager metrics
warrenzhu25 opened a new pull request, #37733:
URL: https://github.com/apache/spark/pull/37733
### What changes were proposed in this pull request?
Add description for ExecutorAllocationManager metrics
### Why are the changes needed?
Some ExecutorAllocationManager metrics are hard to know what stands for just from metric name.
### Does this PR introduce _any_ user-facing change?
no
### How was this patch tested?
No code change.
--
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 #37733: [SPARK-40267][DOC] Add description for ExecutorAllocationManager metrics
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #37733:
URL: https://github.com/apache/spark/pull/37733#issuecomment-1233284355
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.
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] github-actions[bot] closed pull request #37733: [SPARK-40267][DOC] Add description for ExecutorAllocationManager metrics
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #37733: [SPARK-40267][DOC] Add description for ExecutorAllocationManager metrics
URL: https://github.com/apache/spark/pull/37733
--
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] warrenzhu25 commented on pull request #37733: [SPARK-40267][DOC] Add description for ExecutorAllocationManager metrics
Posted by GitBox <gi...@apache.org>.
warrenzhu25 commented on PR #37733:
URL: https://github.com/apache/spark/pull/37733#issuecomment-1231977183
@dongjoon-hyun Could you help 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.
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] github-actions[bot] commented on pull request #37733: [SPARK-40267][DOC] Add description for ExecutorAllocationManager metrics
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #37733:
URL: https://github.com/apache/spark/pull/37733#issuecomment-1362244513
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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] warrenzhu25 commented on a diff in pull request #37733: [SPARK-40267][DOC] Add description for ExecutorAllocationManager metrics
Posted by GitBox <gi...@apache.org>.
warrenzhu25 commented on code in PR #37733:
URL: https://github.com/apache/spark/pull/37733#discussion_r967965938
##########
docs/monitoring.md:
##########
@@ -1207,12 +1207,12 @@ This is the component with the largest amount of instrumented metrics
- namespace=ExecutorAllocationManager
- **note:** these metrics are only emitted when using dynamic allocation. Conditional to a configuration
parameter `spark.dynamicAllocation.enabled` (default is false)
- - executors.numberExecutorsToAdd
- - executors.numberExecutorsPendingToRemove
- - executors.numberAllExecutors
- - executors.numberTargetExecutors
- - executors.numberMaxNeededExecutors
- - executors.numberDecommissioningExecutors
+ - executors.numberExecutorsToAdd: Number of executors to add in the next round sync with cluster manager
+ - executors.numberExecutorsPendingToRemove: Number of executors to remove in the next round sync with cluster manager
+ - executors.numberAllExecutors: Number of all running executors including normal and decommissioning
+ - executors.numberTargetExecutors: Desired number of executors totally needed from cluster manager
+ - executors.numberMaxNeededExecutors: Maximum number of executors needed under the current load to satisfy all running and pending tasks, rounded up.
+ - executors.numberDecommissioningExecutors: Number of executors are decommissioning
Review Comment:
Yes, most metrics are self-explanation, but these metrics are not. Putting comments just after metrics name will be easier for users to understand. If you think there's better place, I'm happy to move it around.
--
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] warrenzhu25 commented on pull request #37733: [SPARK-40267][DOC] Add description for ExecutorAllocationManager metrics
Posted by GitBox <gi...@apache.org>.
warrenzhu25 commented on PR #37733:
URL: https://github.com/apache/spark/pull/37733#issuecomment-1239685272
@Ngone51 Could you help 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.
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 diff in pull request #37733: [SPARK-40267][DOC] Add description for ExecutorAllocationManager metrics
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #37733:
URL: https://github.com/apache/spark/pull/37733#discussion_r967366412
##########
docs/monitoring.md:
##########
@@ -1207,12 +1207,12 @@ This is the component with the largest amount of instrumented metrics
- namespace=ExecutorAllocationManager
- **note:** these metrics are only emitted when using dynamic allocation. Conditional to a configuration
parameter `spark.dynamicAllocation.enabled` (default is false)
- - executors.numberExecutorsToAdd
- - executors.numberExecutorsPendingToRemove
- - executors.numberAllExecutors
- - executors.numberTargetExecutors
- - executors.numberMaxNeededExecutors
- - executors.numberDecommissioningExecutors
+ - executors.numberExecutorsToAdd: Number of executors to add in the next round sync with cluster manager
+ - executors.numberExecutorsPendingToRemove: Number of executors to remove in the next round sync with cluster manager
+ - executors.numberAllExecutors: Number of all running executors including normal and decommissioning
+ - executors.numberTargetExecutors: Desired number of executors totally needed from cluster manager
+ - executors.numberMaxNeededExecutors: Maximum number of executors needed under the current load to satisfy all running and pending tasks, rounded up.
+ - executors.numberDecommissioningExecutors: Number of executors are decommissioning
Review Comment:
We don't have comments in this way in this doc, do we, @warrenzhu25 ? I'm wondering if this is a right place to have these comments.
--
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 diff in pull request #37733: [SPARK-40267][DOC] Add description for ExecutorAllocationManager metrics
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #37733:
URL: https://github.com/apache/spark/pull/37733#discussion_r967367956
##########
docs/monitoring.md:
##########
@@ -1207,12 +1207,12 @@ This is the component with the largest amount of instrumented metrics
- namespace=ExecutorAllocationManager
- **note:** these metrics are only emitted when using dynamic allocation. Conditional to a configuration
parameter `spark.dynamicAllocation.enabled` (default is false)
- - executors.numberExecutorsToAdd
- - executors.numberExecutorsPendingToRemove
- - executors.numberAllExecutors
- - executors.numberTargetExecutors
- - executors.numberMaxNeededExecutors
- - executors.numberDecommissioningExecutors
+ - executors.numberExecutorsToAdd: Number of executors to add in the next round sync with cluster manager
+ - executors.numberExecutorsPendingToRemove: Number of executors to remove in the next round sync with cluster manager
+ - executors.numberAllExecutors: Number of all running executors including normal and decommissioning
Review Comment:
This looks misleading because `decommissioning` is just a status.
--
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