You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by GitBox <gi...@apache.org> on 2022/08/18 01:32:10 UTC
[GitHub] [yunikorn-core] ycr-oss opened a new pull request, #430: [YUNIKORN-1282] Resource metrics are not tracked correctly for all queue types
ycr-oss opened a new pull request, #430:
URL: https://github.com/apache/yunikorn-core/pull/430
### What is this PR for?
* Track resource usage in metrics for non-leaf queues
* Update resource metrics during config and node updates
### What type of PR is it?
* [ ] - Bug Fix
* [ ] - Improvement
* [ ] - Feature
* [ ] - Documentation
* [ ] - Hot Fix
* [ ] - Refactoring
### Todos
* [ ] - Task
### What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-1282
### How should this be tested?
### Screenshots (if appropriate)
### Questions:
* [ ] - The licenses files need update.
* [ ] - There is breaking changes for older versions.
* [ ] - It needs documentation.
--
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@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] codecov[bot] commented on pull request #430: [YUNIKORN-1282] Resource metrics are not tracked correctly for all queue types
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #430:
URL: https://github.com/apache/yunikorn-core/pull/430#issuecomment-1218886030
# [Codecov](https://codecov.io/gh/apache/yunikorn-core/pull/430?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#430](https://codecov.io/gh/apache/yunikorn-core/pull/430?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ac337b7) into [master](https://codecov.io/gh/apache/yunikorn-core/commit/d74e7283b38eb0ce3645e1502e06735dbb202e3e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d74e728) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #430 +/- ##
==========================================
+ Coverage 71.87% 71.88% +0.01%
==========================================
Files 65 65
Lines 9602 9606 +4
==========================================
+ Hits 6901 6905 +4
Misses 2458 2458
Partials 243 243
```
| [Impacted Files](https://codecov.io/gh/apache/yunikorn-core/pull/430?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/scheduler/objects/queue.go](https://codecov.io/gh/apache/yunikorn-core/pull/430/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL3F1ZXVlLmdv) | `70.04% <100.00%> (+0.14%)` | :arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] ycr-oss commented on a diff in pull request #430: [YUNIKORN-1282] Resource metrics are not tracked correctly for all queue types
Posted by GitBox <gi...@apache.org>.
ycr-oss commented on code in PR #430:
URL: https://github.com/apache/yunikorn-core/pull/430#discussion_r948655450
##########
pkg/scheduler/objects/queue.go:
##########
@@ -1017,6 +1019,8 @@ func (sq *Queue) SetMaxResource(max *resources.Resource) {
zap.String("current max", sq.maxResource.String()),
zap.String("new max", max.String()))
sq.maxResource = max.Clone()
+ sq.updateGuaranteedResourceMetrics()
Review Comment:
Good catch. Removed
--
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@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] wilfred-s commented on a diff in pull request #430: [YUNIKORN-1282] Resource metrics are not tracked correctly for all queue types
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on code in PR #430:
URL: https://github.com/apache/yunikorn-core/pull/430#discussion_r948579426
##########
pkg/scheduler/objects/queue.go:
##########
@@ -269,12 +269,14 @@ func (sq *Queue) setResources(resource configs.Resources) error {
} else {
log.Logger().Debug("max resources setting ignored: cannot set zero max resources")
}
+ sq.updateMaxResourceMetrics()
Review Comment:
I think we need this inside the `if` clause with the check for `StrictlyGreaterThanZero`. If there is no max set the object is a non nil pointer with no content. We do not allow 0 values without at least 1 value larger than 0. That means this call would not result in any metrics being generated. The for loop inside the `updateMaxResourceMetrics` never executes. It thus is a call to nowhere.
Same for guaranteed resources.
##########
pkg/scheduler/objects/queue.go:
##########
@@ -1017,6 +1019,8 @@ func (sq *Queue) SetMaxResource(max *resources.Resource) {
zap.String("current max", sq.maxResource.String()),
zap.String("new max", max.String()))
sq.maxResource = max.Clone()
+ sq.updateGuaranteedResourceMetrics()
Review Comment:
We only set max so we should not set the guaranteed metric.
--
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@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] ycr-oss commented on a diff in pull request #430: [YUNIKORN-1282] Resource metrics are not tracked correctly for all queue types
Posted by GitBox <gi...@apache.org>.
ycr-oss commented on code in PR #430:
URL: https://github.com/apache/yunikorn-core/pull/430#discussion_r948655588
##########
pkg/scheduler/objects/queue.go:
##########
@@ -269,12 +269,14 @@ func (sq *Queue) setResources(resource configs.Resources) error {
} else {
log.Logger().Debug("max resources setting ignored: cannot set zero max resources")
}
+ sq.updateMaxResourceMetrics()
Review Comment:
I agree. Updated
--
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@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] wilfred-s closed pull request #430: [YUNIKORN-1282] Resource metrics are not tracked correctly for all queue types
Posted by GitBox <gi...@apache.org>.
wilfred-s closed pull request #430: [YUNIKORN-1282] Resource metrics are not tracked correctly for all queue types
URL: https://github.com/apache/yunikorn-core/pull/430
--
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@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org