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 2021/08/09 19:07:19 UTC
[GitHub] [incubator-yunikorn-core] chia7712 opened a new pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
chia7712 opened a new pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296
…properties
### What is this PR for?
The parent queue can not provide a template for auto generated child queues.
This means that there is no way for a parent to specify an application sorting policy or a maximum resource setting on a child queue.
application sort policy
max resources
We should allow a child template to be set on the managed parent queue (in the config) which can define the behaviour of leaf queues below it
### What type of PR is it?
* [ ] - Bug Fix
* [ ] - Improvement
* [x] - Feature
* [ ] - Documentation
* [ ] - Hot Fix
* [ ] - Refactoring
### Todos
* [ ] - Task
### What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-193
### How should this be tested?
this PR includes a lot of new unit tests
### 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] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (26b695f) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.96%`.
> The diff coverage is `65.09%`.
> :exclamation: Current head 26b695f differs from pull request most recent head fc5181c. Consider uploading reports for the commit fc5181c to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.42% +1.96%
==========================================
Files 60 61 +1
Lines 5220 5762 +542
==========================================
+ Hits 3313 3770 +457
- Misses 1747 1796 +49
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `50.00% <50.00%> (ø)` | |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [eaa6afb...fc5181c](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] chia7712 commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r686604805
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
+ if sq == nil {
+ return nil
}
- // for a leaf queue pull out all values from the template and set each of them
- // See YUNIKORN-193: for now just copy one attr from parent
- if sq.isLeaf {
- if parent[configs.ApplicationSortPolicy] != "" {
- sq.properties[configs.ApplicationSortPolicy] = parent[configs.ApplicationSortPolicy]
- }
+ if current := sq.template; current != nil {
+ return current
}
- // for a parent queue we just copy the template from its parent (no need to be recursive)
- // this stops at the first managed queue
- // See YUNIKORN-193
+ return lookupTemplate(sq.parent)
}
-func (sq *Queue) SetQueueConfig(conf configs.QueueConfig) error {
+func (sq *Queue) ApplyConf(conf configs.QueueConfig) error {
sq.Lock()
defer sq.Unlock()
- return sq.setQueueConfig(conf)
+ return sq.applyConf(conf)
+}
+
+// set inner resource only if input resource is valid
+func (sq *Queue) checkAndSetResources(maxResource, guaranteedResource *resources.Resource) {
+ isInvalidResource := func(resource *resources.Resource) bool {
+ return resource == nil || len(resource.Resources) == 0 || resources.IsZero(resource)
+ }
+
+ if isInvalidResource(maxResource) {
+ log.Logger().Debug("max resources setting ignored: cannot set zero max resources")
+ } else {
+ sq.maxResource = maxResource
+ }
+
+ if isInvalidResource(guaranteedResource) {
+ log.Logger().Debug("guaranteed resources setting ignored: cannot set zero max resources")
+ } else {
+ sq.guaranteedResource = guaranteedResource
+ }
+}
+
+func (sq *Queue) setResources(resource configs.Resources) error {
Review comment:
`checkAndSetResources` is used by `applyTemplate` also. the resources from template may be invalid also.
--
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] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fc5181c) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.92%`.
> The diff coverage is `65.62%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.39% +1.92%
==========================================
Files 60 62 +2
Lines 5220 5762 +542
==========================================
+ Hits 3313 3768 +455
- Misses 1747 1798 +51
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `50.00% <50.00%> (ø)` | |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| ... and [19 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [eaa6afb...fc5181c](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] chia7712 commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r686623121
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
+ if sq == nil {
+ return nil
}
- // for a leaf queue pull out all values from the template and set each of them
- // See YUNIKORN-193: for now just copy one attr from parent
- if sq.isLeaf {
- if parent[configs.ApplicationSortPolicy] != "" {
- sq.properties[configs.ApplicationSortPolicy] = parent[configs.ApplicationSortPolicy]
- }
Review comment:
I will change it to be compatible to old behavior.
--
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] [incubator-yunikorn-core] wilfred-s commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r688209990
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -142,15 +144,26 @@ func NewDynamicQueue(name string, leaf bool, parent *Queue) (*Queue, error) {
if err != nil {
return nil, fmt.Errorf("dynamic queue creation failed: %s", err)
}
- // pull the properties from the parent that should be set on the child
- sq.setTemplateProperties(parent.getProperties())
+
+ sq.applyTemplate(lookupTemplate(parent))
sq.UpdateSortType()
log.Logger().Info("dynamic queue added to scheduler",
zap.String("queueName", sq.QueuePath))
return sq, nil
}
+// use input template to initialize properties, maxResource, and guaranteedResource
+// only leaf queue can use template as the values from template is meaningful to leaf only.
Review comment:
included in addChildQueue 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@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] chia7712 commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r686674211
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -142,15 +144,26 @@ func NewDynamicQueue(name string, leaf bool, parent *Queue) (*Queue, error) {
if err != nil {
return nil, fmt.Errorf("dynamic queue creation failed: %s", err)
}
- // pull the properties from the parent that should be set on the child
- sq.setTemplateProperties(parent.getProperties())
+
+ sq.applyTemplate(lookupTemplate(parent))
sq.UpdateSortType()
log.Logger().Info("dynamic queue added to scheduler",
zap.String("queueName", sq.QueuePath))
return sq, nil
}
+// use input template to initialize properties, maxResource, and guaranteedResource
+// only leaf queue can use template as the values from template is meaningful to leaf only.
Review comment:
copy the response from JIRA
> do we inherit the whole template parent to parent?
That makes sense to me due to following reason.
1. we allow users to create dynamic queue in another dynamic queue. Also, the dynamic queue can't have template. Hence, the dynamic queue which is in another dynamic queue must look up the template from parent to parent.
2. we allow configured queue to inherit properties from parent to parent. for consistence, the template should have similar behavior also.
--
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] [incubator-yunikorn-core] wilfred-s closed pull request #296: [YUNIKORN-193] Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
wilfred-s closed pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296
--
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] [incubator-yunikorn-core] chia7712 commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r688266458
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
Review comment:
> The REST info is not affected and we do not have to recursively look for the template with the inheritance implemented as proposed in the addChildQueue comments.
fair enough. will set the template when we add the queue
--
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] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71effe8) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `2.01%`.
> The diff coverage is `66.16%`.
> :exclamation: Current head 71effe8 differs from pull request most recent head 9effe59. Consider uploading reports for the commit 9effe59 to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.47% +2.01%
==========================================
Files 60 62 +2
Lines 5220 5776 +556
==========================================
+ Hits 3313 3782 +469
- Misses 1747 1798 +51
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `64.10% <64.10%> (ø)` | |
| ... and [19 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [eaaf8b6...9effe59](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] chia7712 commented on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-896478250
@wilfred-s thanks for all suggestions. As you have left some design issue in JIRA, I will go back to here after we reach consensus on those design choices.
--
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] [incubator-yunikorn-core] wilfred-s commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r688209830
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -142,15 +144,26 @@ func NewDynamicQueue(name string, leaf bool, parent *Queue) (*Queue, error) {
if err != nil {
return nil, fmt.Errorf("dynamic queue creation failed: %s", err)
}
- // pull the properties from the parent that should be set on the child
- sq.setTemplateProperties(parent.getProperties())
+
+ sq.applyTemplate(lookupTemplate(parent))
Review comment:
At that point we go through `addChildQueue()` we know all about the type: parent or leaf, managed or dynamic.
Until we get back from the `addChildQueue()` call the child is unreachable and we do not need a lock on it. As soon as we're back we *should*. We currently do not do that as setting the template properties as it is now was supposed to be a real short term solution...
The parent queue is locked in `addChildQueue()` and we can handle all cases in one go:
* if parent queue: inherit the template from parent to parent.
* if leaf and dynamic: set the properties as needed
The inheritance should be as simple as this inside the `addChildQueue()` before we add the child to the map as the last action:
```
if child.isLeaf {
if !child.isManaged {
child.applyTemplate(sq.template)
}
} else {
child.template = sq.template
}
```
--
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] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4e72eea) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `2.01%`.
> The diff coverage is `66.16%`.
> :exclamation: Current head 4e72eea differs from pull request most recent head 71effe8. Consider uploading reports for the commit 71effe8 to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.47% +2.01%
==========================================
Files 60 62 +2
Lines 5220 5776 +556
==========================================
+ Hits 3313 3782 +469
- Misses 1747 1798 +51
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `64.10% <64.10%> (ø)` | |
| ... and [19 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [eaa6afb...71effe8](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71effe8) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `2.01%`.
> The diff coverage is `66.16%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.47% +2.01%
==========================================
Files 60 62 +2
Lines 5220 5776 +556
==========================================
+ Hits 3313 3782 +469
- Misses 1747 1798 +51
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `64.10% <64.10%> (ø)` | |
| ... and [19 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [eaa6afb...71effe8](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (271024b) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.96%`.
> The diff coverage is `65.09%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.42% +1.96%
==========================================
Files 60 61 +1
Lines 5220 5762 +542
==========================================
+ Hits 3313 3770 +457
- Misses 1747 1796 +49
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `50.00% <50.00%> (ø)` | |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [2bcfb6d...271024b](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] wilfred-s commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r688211720
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
Review comment:
The REST info is not affected and we do not have to recursively look for the template with the inheritance implemented as proposed in the addChildQueue 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@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (da6e265) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.96%`.
> The diff coverage is `65.09%`.
> :exclamation: Current head da6e265 differs from pull request most recent head 271024b. Consider uploading reports for the commit 271024b to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.42% +1.96%
==========================================
Files 60 61 +1
Lines 5220 5762 +542
==========================================
+ Hits 3313 3770 +457
- Misses 1747 1796 +49
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `50.00% <50.00%> (ø)` | |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [2bcfb6d...271024b](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] wilfred-s commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r686417588
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
Review comment:
We access the parent unlocked which will trigger race conditions. This also seems to indicate implicit inheritance. (see comment above).
A cheaper solution would be to set the template when we add the queue (set the pointer to the same template as the parent has)
NIT: typo in the comment
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -142,15 +144,26 @@ func NewDynamicQueue(name string, leaf bool, parent *Queue) (*Queue, error) {
if err != nil {
return nil, fmt.Errorf("dynamic queue creation failed: %s", err)
}
- // pull the properties from the parent that should be set on the child
- sq.setTemplateProperties(parent.getProperties())
+
+ sq.applyTemplate(lookupTemplate(parent))
sq.UpdateSortType()
log.Logger().Info("dynamic queue added to scheduler",
zap.String("queueName", sq.QueuePath))
return sq, nil
}
+// use input template to initialize properties, maxResource, and guaranteedResource
+// only leaf queue can use template as the values from template is meaningful to leaf only.
Review comment:
Design question: do we inherit the whole template parent to parent?
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
+ if sq == nil {
+ return nil
}
- // for a leaf queue pull out all values from the template and set each of them
- // See YUNIKORN-193: for now just copy one attr from parent
- if sq.isLeaf {
- if parent[configs.ApplicationSortPolicy] != "" {
- sq.properties[configs.ApplicationSortPolicy] = parent[configs.ApplicationSortPolicy]
- }
+ if current := sq.template; current != nil {
+ return current
}
- // for a parent queue we just copy the template from its parent (no need to be recursive)
- // this stops at the first managed queue
- // See YUNIKORN-193
+ return lookupTemplate(sq.parent)
}
-func (sq *Queue) SetQueueConfig(conf configs.QueueConfig) error {
+func (sq *Queue) ApplyConf(conf configs.QueueConfig) error {
sq.Lock()
defer sq.Unlock()
- return sq.setQueueConfig(conf)
+ return sq.applyConf(conf)
+}
Review comment:
Layout comment: keep the exported locked version with the unlocked version
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -142,15 +144,26 @@ func NewDynamicQueue(name string, leaf bool, parent *Queue) (*Queue, error) {
if err != nil {
return nil, fmt.Errorf("dynamic queue creation failed: %s", err)
}
- // pull the properties from the parent that should be set on the child
- sq.setTemplateProperties(parent.getProperties())
+
+ sq.applyTemplate(lookupTemplate(parent))
Review comment:
This should be processed as part of the `addChildQueue()` call that we do just before this point now that we have a proper template to set. Removes taking a lock and copying data.
##########
File path: pkg/scheduler/objects/queue_test.go
##########
@@ -1166,8 +1169,7 @@ func TestQueueProps(t *testing.T) {
leaf, err = createDynamicQueue(parent, "leaf", false)
assert.NilError(t, err, "failed to create leaf queue")
assert.Assert(t, leaf.isLeaf && !leaf.isManaged, "leaf queue is not marked as unmanaged leaf")
- assert.Equal(t, len(leaf.properties), 1, "leaf queue properties size incorrect")
- assert.Equal(t, leaf.properties[configs.ApplicationSortPolicy], "stateaware", "leaf queue property value not as expected")
+ assert.Equal(t, len(leaf.properties), 0, "leaf queue properties size incorrect")
Review comment:
The fact that this test breaks means we have regressed somewhere. This test should not break by default unless we specifically want it to break.
##########
File path: pkg/scheduler/objects/queue_test.go
##########
@@ -1338,10 +1340,203 @@ func TestSupportTaskGroup(t *testing.T) {
assert.Assert(t, !leaf.SupportTaskGroup(), "leaf queue (FAIR policy) should not support task group")
}
-func TestGetPartitionQueues(t *testing.T) {
+func TestGetPartitionQueueDAOInfo(t *testing.T) {
root, err := createRootQueue(nil)
assert.NilError(t, err, "failed to create basic root queue: %v", err)
- root.properties = make(map[string]string)
- root.properties["key"] = "value"
- assert.Equal(t, "value", root.GetPartitionQueues().Properties["key"])
+
+ // test properties
+ root.properties = getProperties()
+ assert.Assert(t, reflect.DeepEqual(root.properties, root.GetPartitionQueueDAOInfo().Properties))
+
+ // test template
+ root.template = template.NewTemplate(getProperties(), getResource(t), getResource(t))
+ assert.Assert(t, reflect.DeepEqual(root.template.GetProperties(), root.GetPartitionQueueDAOInfo().TemplateInfo.Properties))
+ assert.Equal(t, root.template.GetMaxResource().DAOString(), root.GetPartitionQueueDAOInfo().TemplateInfo.MaxResource)
Review comment:
This is dangerous: resources are a map you cannot never rely on the fact that the resources will be returned in a fixed order. Two calls to DAOString() on the same resource might return different string values.
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
+ if sq == nil {
+ return nil
}
- // for a leaf queue pull out all values from the template and set each of them
- // See YUNIKORN-193: for now just copy one attr from parent
- if sq.isLeaf {
- if parent[configs.ApplicationSortPolicy] != "" {
- sq.properties[configs.ApplicationSortPolicy] = parent[configs.ApplicationSortPolicy]
- }
Review comment:
This will regress existing setups. We need to handle this gracefully. I do not see this attribute in the template either. This is an important one to add
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
+ if sq == nil {
+ return nil
}
- // for a leaf queue pull out all values from the template and set each of them
- // See YUNIKORN-193: for now just copy one attr from parent
- if sq.isLeaf {
- if parent[configs.ApplicationSortPolicy] != "" {
- sq.properties[configs.ApplicationSortPolicy] = parent[configs.ApplicationSortPolicy]
- }
+ if current := sq.template; current != nil {
+ return current
}
- // for a parent queue we just copy the template from its parent (no need to be recursive)
- // this stops at the first managed queue
- // See YUNIKORN-193
+ return lookupTemplate(sq.parent)
}
-func (sq *Queue) SetQueueConfig(conf configs.QueueConfig) error {
+func (sq *Queue) ApplyConf(conf configs.QueueConfig) error {
sq.Lock()
defer sq.Unlock()
- return sq.setQueueConfig(conf)
+ return sq.applyConf(conf)
+}
+
+// set inner resource only if input resource is valid
+func (sq *Queue) checkAndSetResources(maxResource, guaranteedResource *resources.Resource) {
+ isInvalidResource := func(resource *resources.Resource) bool {
+ return resource == nil || len(resource.Resources) == 0 || resources.IsZero(resource)
Review comment:
This is already available as a call to `StrictlyGreaterThanZero(res)` using that call would be the proper solution and would fix a possible lingering bug in setting max and guaranteed resources.
##########
File path: pkg/common/configs/config.go
##########
@@ -74,10 +74,16 @@ type QueueConfig struct {
Properties map[string]string `yaml:",omitempty" json:",omitempty"`
AdminACL string `yaml:",omitempty" json:",omitempty"`
SubmitACL string `yaml:",omitempty" json:",omitempty"`
+ ChildTemplate ChildTemplate `yaml:",omitempty" json:",omitempty"`
Queues []QueueConfig `yaml:",omitempty" json:",omitempty"`
Limits []Limit `yaml:",omitempty" json:",omitempty"`
}
+type ChildTemplate struct {
+ Properties map[string]string `yaml:",omitempty" json:",omitempty"`
Review comment:
We should also add the MaxApplications in this structure
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
+ if sq == nil {
+ return nil
}
- // for a leaf queue pull out all values from the template and set each of them
- // See YUNIKORN-193: for now just copy one attr from parent
- if sq.isLeaf {
- if parent[configs.ApplicationSortPolicy] != "" {
- sq.properties[configs.ApplicationSortPolicy] = parent[configs.ApplicationSortPolicy]
- }
+ if current := sq.template; current != nil {
+ return current
}
- // for a parent queue we just copy the template from its parent (no need to be recursive)
- // this stops at the first managed queue
- // See YUNIKORN-193
+ return lookupTemplate(sq.parent)
}
-func (sq *Queue) SetQueueConfig(conf configs.QueueConfig) error {
+func (sq *Queue) ApplyConf(conf configs.QueueConfig) error {
sq.Lock()
defer sq.Unlock()
- return sq.setQueueConfig(conf)
+ return sq.applyConf(conf)
+}
+
+// set inner resource only if input resource is valid
+func (sq *Queue) checkAndSetResources(maxResource, guaranteedResource *resources.Resource) {
+ isInvalidResource := func(resource *resources.Resource) bool {
+ return resource == nil || len(resource.Resources) == 0 || resources.IsZero(resource)
+ }
+
+ if isInvalidResource(maxResource) {
+ log.Logger().Debug("max resources setting ignored: cannot set zero max resources")
+ } else {
+ sq.maxResource = maxResource
+ }
+
+ if isInvalidResource(guaranteedResource) {
+ log.Logger().Debug("guaranteed resources setting ignored: cannot set zero max resources")
+ } else {
+ sq.guaranteedResource = guaranteedResource
+ }
+}
+
+func (sq *Queue) setResources(resource configs.Resources) error {
Review comment:
Fold `setResources()` and `checkAndSetResources()` into one function. the root queue check should have happened already (see comment later)
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
+ if sq == nil {
+ return nil
}
- // for a leaf queue pull out all values from the template and set each of them
- // See YUNIKORN-193: for now just copy one attr from parent
- if sq.isLeaf {
- if parent[configs.ApplicationSortPolicy] != "" {
- sq.properties[configs.ApplicationSortPolicy] = parent[configs.ApplicationSortPolicy]
- }
+ if current := sq.template; current != nil {
+ return current
}
- // for a parent queue we just copy the template from its parent (no need to be recursive)
- // this stops at the first managed queue
- // See YUNIKORN-193
+ return lookupTemplate(sq.parent)
}
-func (sq *Queue) SetQueueConfig(conf configs.QueueConfig) error {
+func (sq *Queue) ApplyConf(conf configs.QueueConfig) error {
sq.Lock()
defer sq.Unlock()
- return sq.setQueueConfig(conf)
+ return sq.applyConf(conf)
+}
+
+// set inner resource only if input resource is valid
+func (sq *Queue) checkAndSetResources(maxResource, guaranteedResource *resources.Resource) {
+ isInvalidResource := func(resource *resources.Resource) bool {
+ return resource == nil || len(resource.Resources) == 0 || resources.IsZero(resource)
+ }
+
+ if isInvalidResource(maxResource) {
+ log.Logger().Debug("max resources setting ignored: cannot set zero max resources")
+ } else {
+ sq.maxResource = maxResource
+ }
+
+ if isInvalidResource(guaranteedResource) {
+ log.Logger().Debug("guaranteed resources setting ignored: cannot set zero max resources")
+ } else {
+ sq.guaranteedResource = guaranteedResource
+ }
+}
+
+func (sq *Queue) setResources(resource configs.Resources) error {
+ // Load the max & guaranteed resources for all but the root queue
+ if sq.Name != configs.RootQueue {
+ maxResource, err := resources.NewResourceFromConf(resource.Max)
+ if err != nil {
+ log.Logger().Error("parsing failed on max resources this should not happen",
+ zap.Error(err))
+ return err
+ }
+
+ guaranteedResource, err := resources.NewResourceFromConf(resource.Guaranteed)
+ if err != nil {
+ log.Logger().Error("parsing failed on guaranteed resources this should not happen",
+ zap.Error(err))
+ return err
+ }
+ sq.checkAndSetResources(maxResource, guaranteedResource)
+ }
+ return nil
+}
+
+func (sq *Queue) setTemplate(conf configs.ChildTemplate) error {
+ // Empty returns true if all elements in this template are "0". otherwise, it returns false.
+ // This method is used to check whether there is template in yaml file.
+ configIsNotEmpty := func() bool {
+ return len(conf.Properties) != 0 || len(conf.Resources.Guaranteed) != 0 || len(conf.Resources.Max) != 0
Review comment:
Can we fold this into the `FromConf()` call? A non empty list of empty property values is also empty etc. The `FromConf()` code should do the exact same checks including the resource parsing.
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -239,30 +302,12 @@ func (sq *Queue) setQueueConfig(conf configs.QueueConfig) error {
sq.isLeaf = false
}
- // Load the max & guaranteed resources for all but the root queue
- if sq.Name != configs.RootQueue {
- sq.maxResource, err = resources.NewResourceFromConf(conf.Resources.Max)
- if err != nil {
- log.Logger().Error("parsing failed on max resources this should not happen",
- zap.Error(err))
- return err
- }
- if len(sq.maxResource.Resources) == 0 || resources.IsZero(sq.maxResource) {
- log.Logger().Debug("max resources config setting ignored: cannot set zero max resources")
- sq.maxResource = nil
- }
+ if err = sq.setTemplate(conf.ChildTemplate); err != nil {
Review comment:
We have established the type of queue. We should not set a template if we have a leaf queue. The check should be here instead of in the `setTemplate()` code.
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -239,30 +302,12 @@ func (sq *Queue) setQueueConfig(conf configs.QueueConfig) error {
sq.isLeaf = false
}
- // Load the max & guaranteed resources for all but the root queue
- if sq.Name != configs.RootQueue {
- sq.maxResource, err = resources.NewResourceFromConf(conf.Resources.Max)
- if err != nil {
- log.Logger().Error("parsing failed on max resources this should not happen",
- zap.Error(err))
- return err
- }
- if len(sq.maxResource.Resources) == 0 || resources.IsZero(sq.maxResource) {
- log.Logger().Debug("max resources config setting ignored: cannot set zero max resources")
- sq.maxResource = nil
- }
+ if err = sq.setTemplate(conf.ChildTemplate); err != nil {
+ return nil
+ }
- // Load the guaranteed resources
- sq.guaranteedResource, err = resources.NewResourceFromConf(conf.Resources.Guaranteed)
- if err != nil {
- log.Logger().Error("parsing failed on guaranteed resources this should not happen",
- zap.Error(err))
- return err
- }
- if len(sq.guaranteedResource.Resources) == 0 || resources.IsZero(sq.guaranteedResource) {
- log.Logger().Debug("guaranteed resources config setting ignored: guaranteed must be non-zero to take effect")
- sq.guaranteedResource = nil
- }
+ if err = sq.setResources(conf.Resources); err != nil {
+ return err
Review comment:
Same comment as above `setResources()` is a nop call for the root queue: check here before calling.
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -409,22 +454,25 @@ func (sq *Queue) GetQueueInfos() dao.QueueDAOInfo {
return queueInfo
}
-func (sq *Queue) GetPartitionQueues() dao.PartitionQueueDAOInfo {
+func (sq *Queue) GetPartitionQueueDAOInfo() dao.PartitionQueueDAOInfo {
queueInfo := dao.PartitionQueueDAOInfo{}
if len(sq.children) > 0 {
for _, child := range sq.GetCopyOfChildren() {
- queueInfo.Children = append(queueInfo.Children, child.GetPartitionQueues())
+ queueInfo.Children = append(queueInfo.Children, child.GetPartitionQueueDAOInfo())
}
}
sq.RLock()
defer sq.RUnlock()
- queueInfo.QueueName = sq.GetQueuePath()
+
+ // we have held the read lock so following method should not take lock again.
Review comment:
We should not lock anything while holding the lock. Which means that we must not call `sq.parent.GetQueuePath()` to get the path as it might deadlock. Derive the parent path via:
`queueInfo.Parent = sq.QueuePath[:strings.LastIndex(sq.QueuePath, configs.DOT)]`
if parent is not nil
NIT: comment before taking the lock.
--
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] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (271024b) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.96%`.
> The diff coverage is `65.09%`.
> :exclamation: Current head 271024b differs from pull request most recent head 26b695f. Consider uploading reports for the commit 26b695f to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.42% +1.96%
==========================================
Files 60 61 +1
Lines 5220 5762 +542
==========================================
+ Hits 3313 3770 +457
- Misses 1747 1796 +49
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `50.00% <50.00%> (ø)` | |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [2bcfb6d...26b695f](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9effe59) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `2.01%`.
> The diff coverage is `66.03%`.
> :exclamation: Current head 9effe59 differs from pull request most recent head aa70774. Consider uploading reports for the commit aa70774 to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.47% +2.01%
==========================================
Files 60 62 +2
Lines 5220 5776 +556
==========================================
+ Hits 3313 3782 +469
- Misses 1747 1798 +51
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `64.10% <64.10%> (ø)` | |
| ... and [19 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [eaaf8b6...aa70774](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (26b695f) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.96%`.
> The diff coverage is `65.09%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.42% +1.96%
==========================================
Files 60 61 +1
Lines 5220 5762 +542
==========================================
+ Hits 3313 3770 +457
- Misses 1747 1796 +49
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `50.00% <50.00%> (ø)` | |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [2bcfb6d...26b695f](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] chia7712 commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r686816892
##########
File path: pkg/common/configs/config.go
##########
@@ -74,10 +74,16 @@ type QueueConfig struct {
Properties map[string]string `yaml:",omitempty" json:",omitempty"`
AdminACL string `yaml:",omitempty" json:",omitempty"`
SubmitACL string `yaml:",omitempty" json:",omitempty"`
+ ChildTemplate ChildTemplate `yaml:",omitempty" json:",omitempty"`
Queues []QueueConfig `yaml:",omitempty" json:",omitempty"`
Limits []Limit `yaml:",omitempty" json:",omitempty"`
}
+type ChildTemplate struct {
+ Properties map[string]string `yaml:",omitempty" json:",omitempty"`
Review comment:
`MaxApplications` is added to this PR and it will work by follow-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.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] chia7712 commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r688266253
##########
File path: pkg/scheduler/objects/queue_test.go
##########
@@ -1338,10 +1340,203 @@ func TestSupportTaskGroup(t *testing.T) {
assert.Assert(t, !leaf.SupportTaskGroup(), "leaf queue (FAIR policy) should not support task group")
}
-func TestGetPartitionQueues(t *testing.T) {
+func TestGetPartitionQueueDAOInfo(t *testing.T) {
root, err := createRootQueue(nil)
assert.NilError(t, err, "failed to create basic root queue: %v", err)
- root.properties = make(map[string]string)
- root.properties["key"] = "value"
- assert.Equal(t, "value", root.GetPartitionQueues().Properties["key"])
+
+ // test properties
+ root.properties = getProperties()
+ assert.Assert(t, reflect.DeepEqual(root.properties, root.GetPartitionQueueDAOInfo().Properties))
+
+ // test template
+ root.template = template.NewTemplate(getProperties(), getResource(t), getResource(t))
+ assert.Assert(t, reflect.DeepEqual(root.template.GetProperties(), root.GetPartitionQueueDAOInfo().TemplateInfo.Properties))
+ assert.Equal(t, root.template.GetMaxResource().DAOString(), root.GetPartitionQueueDAOInfo().TemplateInfo.MaxResource)
Review comment:
I created a ticket for existent code (https://issues.apache.org/jira/browse/YUNIKORN-797)
in this PR, we test `DAOString()` only if it uses single value.
--
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] [incubator-yunikorn-core] chia7712 commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r686654827
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
Review comment:
> A cheaper solution would be to set the template when we add the queue (set the pointer to the same template as the parent has)
It is hard to know whether it does have template when reading response of restful apis.
I use the roadblock to get template from parents. It should be fine as `NewDynamicQueue` does not hold any lock.
--
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] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9effe59) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `2.01%`.
> The diff coverage is `66.03%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.47% +2.01%
==========================================
Files 60 62 +2
Lines 5220 5776 +556
==========================================
+ Hits 3313 3782 +469
- Misses 1747 1798 +51
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `64.10% <64.10%> (ø)` | |
| ... and [19 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [eaaf8b6...9effe59](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (aa70774) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.97%`.
> The diff coverage is `65.83%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.44% +1.97%
==========================================
Files 60 62 +2
Lines 5220 5770 +550
==========================================
+ Hits 3313 3776 +463
- Misses 1747 1798 +51
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `64.10% <64.10%> (ø)` | |
| ... and [19 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [eaaf8b6...aa70774](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fc5181c) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.92%`.
> The diff coverage is `65.62%`.
> :exclamation: Current head fc5181c differs from pull request most recent head 9b34cad. Consider uploading reports for the commit 9b34cad to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.39% +1.92%
==========================================
Files 60 62 +2
Lines 5220 5762 +542
==========================================
+ Hits 3313 3768 +455
- Misses 1747 1798 +51
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `50.00% <50.00%> (ø)` | |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| ... and [19 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [eaa6afb...9b34cad](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] chia7712 commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r686616800
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -142,15 +144,26 @@ func NewDynamicQueue(name string, leaf bool, parent *Queue) (*Queue, error) {
if err != nil {
return nil, fmt.Errorf("dynamic queue creation failed: %s", err)
}
- // pull the properties from the parent that should be set on the child
- sq.setTemplateProperties(parent.getProperties())
+
+ sq.applyTemplate(lookupTemplate(parent))
Review comment:
`addChildQueue` is used to add both configured queue and dynamic queue. However, the configured queue should not use template to initialize.
--
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] [incubator-yunikorn-core] chia7712 commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r686650510
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
+ if sq == nil {
+ return nil
}
- // for a leaf queue pull out all values from the template and set each of them
- // See YUNIKORN-193: for now just copy one attr from parent
- if sq.isLeaf {
- if parent[configs.ApplicationSortPolicy] != "" {
- sq.properties[configs.ApplicationSortPolicy] = parent[configs.ApplicationSortPolicy]
- }
+ if current := sq.template; current != nil {
+ return current
}
- // for a parent queue we just copy the template from its parent (no need to be recursive)
- // this stops at the first managed queue
- // See YUNIKORN-193
+ return lookupTemplate(sq.parent)
}
-func (sq *Queue) SetQueueConfig(conf configs.QueueConfig) error {
+func (sq *Queue) ApplyConf(conf configs.QueueConfig) error {
sq.Lock()
defer sq.Unlock()
- return sq.setQueueConfig(conf)
+ return sq.applyConf(conf)
+}
+
+// set inner resource only if input resource is valid
+func (sq *Queue) checkAndSetResources(maxResource, guaranteedResource *resources.Resource) {
+ isInvalidResource := func(resource *resources.Resource) bool {
+ return resource == nil || len(resource.Resources) == 0 || resources.IsZero(resource)
+ }
+
+ if isInvalidResource(maxResource) {
+ log.Logger().Debug("max resources setting ignored: cannot set zero max resources")
+ } else {
+ sq.maxResource = maxResource
+ }
+
+ if isInvalidResource(guaranteedResource) {
+ log.Logger().Debug("guaranteed resources setting ignored: cannot set zero max resources")
+ } else {
+ sq.guaranteedResource = guaranteedResource
+ }
+}
+
+func (sq *Queue) setResources(resource configs.Resources) error {
Review comment:
ok. I refactor the code and then address your comment.
--
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] [incubator-yunikorn-core] codecov[bot] commented on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0db0250) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.96%`.
> The diff coverage is `65.09%`.
> :exclamation: Current head 0db0250 differs from pull request most recent head 271024b. Consider uploading reports for the commit 271024b to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.42% +1.96%
==========================================
Files 60 61 +1
Lines 5220 5762 +542
==========================================
+ Hits 3313 3770 +457
- Misses 1747 1796 +49
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `50.00% <50.00%> (ø)` | |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [2bcfb6d...271024b](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] wilfred-s commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r688200846
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -142,15 +144,26 @@ func NewDynamicQueue(name string, leaf bool, parent *Queue) (*Queue, error) {
if err != nil {
return nil, fmt.Errorf("dynamic queue creation failed: %s", err)
}
- // pull the properties from the parent that should be set on the child
- sq.setTemplateProperties(parent.getProperties())
+
+ sq.applyTemplate(lookupTemplate(parent))
sq.UpdateSortType()
log.Logger().Info("dynamic queue added to scheduler",
zap.String("queueName", sq.QueuePath))
return sq, nil
}
+// use input template to initialize properties, maxResource, and guaranteedResource
+// only leaf queue can use template as the values from template is meaningful to leaf only.
Review comment:
I agree please implement that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fc5181c) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.92%`.
> The diff coverage is `65.62%`.
> :exclamation: Current head fc5181c differs from pull request most recent head 4e72eea. Consider uploading reports for the commit 4e72eea to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.39% +1.92%
==========================================
Files 60 62 +2
Lines 5220 5762 +542
==========================================
+ Hits 3313 3768 +455
- Misses 1747 1798 +51
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `50.00% <50.00%> (ø)` | |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| ... and [19 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [eaa6afb...4e72eea](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] chia7712 commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r686962970
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -409,30 +456,33 @@ func (sq *Queue) GetQueueInfos() dao.QueueDAOInfo {
return queueInfo
}
-func (sq *Queue) GetPartitionQueues() dao.PartitionQueueDAOInfo {
+func (sq *Queue) GetPartitionQueueDAOInfo() dao.PartitionQueueDAOInfo {
queueInfo := dao.PartitionQueueDAOInfo{}
if len(sq.children) > 0 {
for _, child := range sq.GetCopyOfChildren() {
- queueInfo.Children = append(queueInfo.Children, child.GetPartitionQueues())
+ queueInfo.Children = append(queueInfo.Children, child.GetPartitionQueueDAOInfo())
}
}
+ // we have held the read lock so following method should not take lock again.
Review comment:
I filed another PR to resolve the deadlock (#299) as I guess this PR needs more time to be reviewed :)
--
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] [incubator-yunikorn-core] wilfred-s commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r687426472
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -409,30 +456,33 @@ func (sq *Queue) GetQueueInfos() dao.QueueDAOInfo {
return queueInfo
}
-func (sq *Queue) GetPartitionQueues() dao.PartitionQueueDAOInfo {
+func (sq *Queue) GetPartitionQueueDAOInfo() dao.PartitionQueueDAOInfo {
queueInfo := dao.PartitionQueueDAOInfo{}
if len(sq.children) > 0 {
for _, child := range sq.GetCopyOfChildren() {
- queueInfo.Children = append(queueInfo.Children, child.GetPartitionQueues())
+ queueInfo.Children = append(queueInfo.Children, child.GetPartitionQueueDAOInfo())
}
}
+ // we have held the read lock so following method should not take lock again.
Review comment:
yes, committed the other 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@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ba42522) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.96%`.
> The diff coverage is `65.09%`.
> :exclamation: Current head ba42522 differs from pull request most recent head 26b695f. Consider uploading reports for the commit 26b695f to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.42% +1.96%
==========================================
Files 60 61 +1
Lines 5220 5762 +542
==========================================
+ Hits 3313 3770 +457
- Misses 1747 1796 +49
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `50.00% <50.00%> (ø)` | |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [2bcfb6d...26b695f](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3a7741a) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.96%`.
> The diff coverage is `65.09%`.
> :exclamation: Current head 3a7741a differs from pull request most recent head 271024b. Consider uploading reports for the commit 271024b to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.42% +1.96%
==========================================
Files 60 61 +1
Lines 5220 5762 +542
==========================================
+ Hits 3313 3770 +457
- Misses 1747 1796 +49
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `50.00% <50.00%> (ø)` | |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [2bcfb6d...271024b](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] wilfred-s commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r688198722
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
+ if sq == nil {
+ return nil
}
- // for a leaf queue pull out all values from the template and set each of them
- // See YUNIKORN-193: for now just copy one attr from parent
- if sq.isLeaf {
- if parent[configs.ApplicationSortPolicy] != "" {
- sq.properties[configs.ApplicationSortPolicy] = parent[configs.ApplicationSortPolicy]
- }
+ if current := sq.template; current != nil {
+ return current
}
- // for a parent queue we just copy the template from its parent (no need to be recursive)
- // this stops at the first managed queue
- // See YUNIKORN-193
+ return lookupTemplate(sq.parent)
}
-func (sq *Queue) SetQueueConfig(conf configs.QueueConfig) error {
+func (sq *Queue) ApplyConf(conf configs.QueueConfig) error {
sq.Lock()
defer sq.Unlock()
- return sq.setQueueConfig(conf)
+ return sq.applyConf(conf)
+}
Review comment:
No: you have an `applyConf()` which is unlocked and an `ApplyConf()` that is locked keep them together not 100s of lines apart
--
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] [incubator-yunikorn-core] chia7712 commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r686476266
##########
File path: pkg/scheduler/objects/queue_test.go
##########
@@ -1338,10 +1340,203 @@ func TestSupportTaskGroup(t *testing.T) {
assert.Assert(t, !leaf.SupportTaskGroup(), "leaf queue (FAIR policy) should not support task group")
}
-func TestGetPartitionQueues(t *testing.T) {
+func TestGetPartitionQueueDAOInfo(t *testing.T) {
root, err := createRootQueue(nil)
assert.NilError(t, err, "failed to create basic root queue: %v", err)
- root.properties = make(map[string]string)
- root.properties["key"] = "value"
- assert.Equal(t, "value", root.GetPartitionQueues().Properties["key"])
+
+ // test properties
+ root.properties = getProperties()
+ assert.Assert(t, reflect.DeepEqual(root.properties, root.GetPartitionQueueDAOInfo().Properties))
+
+ // test template
+ root.template = template.NewTemplate(getProperties(), getResource(t), getResource(t))
+ assert.Assert(t, reflect.DeepEqual(root.template.GetProperties(), root.GetPartitionQueueDAOInfo().TemplateInfo.Properties))
+ assert.Equal(t, root.template.GetMaxResource().DAOString(), root.GetPartitionQueueDAOInfo().TemplateInfo.MaxResource)
Review comment:
you are right.
There is a similar issue in code base (https://github.com/apache/incubator-yunikorn-core/blob/master/pkg/common/resources/resources_test.go#L1550). Maybe we should find out if it contains the expected string one by one. Or we can make DAOString() have consistent order. The benefit is the restful response get more readable. WDTY? I will file a PR to fix existent code.
--
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] [incubator-yunikorn-core] chia7712 commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r688261801
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
+ if sq == nil {
+ return nil
}
- // for a leaf queue pull out all values from the template and set each of them
- // See YUNIKORN-193: for now just copy one attr from parent
- if sq.isLeaf {
- if parent[configs.ApplicationSortPolicy] != "" {
- sq.properties[configs.ApplicationSortPolicy] = parent[configs.ApplicationSortPolicy]
- }
+ if current := sq.template; current != nil {
+ return current
}
- // for a parent queue we just copy the template from its parent (no need to be recursive)
- // this stops at the first managed queue
- // See YUNIKORN-193
+ return lookupTemplate(sq.parent)
}
-func (sq *Queue) SetQueueConfig(conf configs.QueueConfig) error {
+func (sq *Queue) ApplyConf(conf configs.QueueConfig) error {
sq.Lock()
defer sq.Unlock()
- return sq.setQueueConfig(conf)
+ return sq.applyConf(conf)
+}
Review comment:
will copy that!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] wilfred-s commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r688204389
##########
File path: pkg/scheduler/objects/queue_test.go
##########
@@ -1338,10 +1340,203 @@ func TestSupportTaskGroup(t *testing.T) {
assert.Assert(t, !leaf.SupportTaskGroup(), "leaf queue (FAIR policy) should not support task group")
}
-func TestGetPartitionQueues(t *testing.T) {
+func TestGetPartitionQueueDAOInfo(t *testing.T) {
root, err := createRootQueue(nil)
assert.NilError(t, err, "failed to create basic root queue: %v", err)
- root.properties = make(map[string]string)
- root.properties["key"] = "value"
- assert.Equal(t, "value", root.GetPartitionQueues().Properties["key"])
+
+ // test properties
+ root.properties = getProperties()
+ assert.Assert(t, reflect.DeepEqual(root.properties, root.GetPartitionQueueDAOInfo().Properties))
+
+ // test template
+ root.template = template.NewTemplate(getProperties(), getResource(t), getResource(t))
+ assert.Assert(t, reflect.DeepEqual(root.template.GetProperties(), root.GetPartitionQueueDAOInfo().TemplateInfo.Properties))
+ assert.Equal(t, root.template.GetMaxResource().DAOString(), root.GetPartitionQueueDAOInfo().TemplateInfo.MaxResource)
Review comment:
We need a follow up PR. I would suggest that calls that do not directly test `DAOString()` we use single resource values.
In the test case for for `DAOString()` only test the nil, empty and single value cases for full equality and a multi valued resource for the layout markers. This might need to be done for more 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@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4e72eea) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `2.01%`.
> The diff coverage is `66.16%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.47% +2.01%
==========================================
Files 60 62 +2
Lines 5220 5776 +556
==========================================
+ Hits 3313 3782 +469
- Misses 1747 1798 +51
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `64.10% <64.10%> (ø)` | |
| ... and [19 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [eaa6afb...4e72eea](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] [incubator-yunikorn-core] chia7712 commented on a change in pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#discussion_r686621174
##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -182,36 +195,86 @@ func (sq *Queue) mergeProperties(parent, config map[string]string) {
}
}
-// Set the properties that the dynamic child queue inherits from the parent
-// The properties list for the parent must be retrieved using getProperties()
-// This currently only sets the sort policy as it is set on the parent
-// Further implementation is part of YUNIKORN-193
-// lock free call
-func (sq *Queue) setTemplateProperties(parent map[string]string) {
- if len(parent) == 0 {
- return
+// try to find a template from closet parent
+func lookupTemplate(sq *Queue) *template.Template {
+ if sq == nil {
+ return nil
}
- // for a leaf queue pull out all values from the template and set each of them
- // See YUNIKORN-193: for now just copy one attr from parent
- if sq.isLeaf {
- if parent[configs.ApplicationSortPolicy] != "" {
- sq.properties[configs.ApplicationSortPolicy] = parent[configs.ApplicationSortPolicy]
- }
+ if current := sq.template; current != nil {
+ return current
}
- // for a parent queue we just copy the template from its parent (no need to be recursive)
- // this stops at the first managed queue
- // See YUNIKORN-193
+ return lookupTemplate(sq.parent)
}
-func (sq *Queue) SetQueueConfig(conf configs.QueueConfig) error {
+func (sq *Queue) ApplyConf(conf configs.QueueConfig) error {
sq.Lock()
defer sq.Unlock()
- return sq.setQueueConfig(conf)
+ return sq.applyConf(conf)
+}
Review comment:
pardon me. Did you mean we should keep origin naming? (`setQueueConfig`)
--
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] [incubator-yunikorn-core] chia7712 commented on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-898225501
@wilfred-s thanks for all reviews. all comments are addressed. please take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #296: YUNIKORN-193 Add option to allow parent to set unmanaged child queue …
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #296:
URL: https://github.com/apache/incubator-yunikorn-core/pull/296#issuecomment-895471024
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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 [#296](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (271024b) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.96%`.
> The diff coverage is `65.09%`.
> :exclamation: Current head 271024b differs from pull request most recent head ba42522. Consider uploading reports for the commit ba42522 to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 63.46% 65.42% +1.96%
==========================================
Files 60 61 +1
Lines 5220 5762 +542
==========================================
+ Hits 3313 3770 +457
- Misses 1747 1796 +49
- Partials 160 196 +36
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?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/common/utils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL2NvbW1vbi91dGlscy5nbw==) | `77.50% <0.00%> (ø)` | |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.68% <0.00%> (-0.58%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `64.51% <45.00%> (-9.40%)` | :arrow_down: |
| [pkg/scheduler/objects/template/template.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9vYmplY3RzL3RlbXBsYXRlL3RlbXBsYXRlLmdv) | `50.00% <50.00%> (ø)` | |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/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-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `60.79% <55.00%> (-0.79%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [2bcfb6d...ba42522](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/296?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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