You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@yunikorn.apache.org by "Wilfred Spiegelenburg (Jira)" <ji...@apache.org> on 2021/02/01 04:27:00 UTC
[jira] [Commented] (YUNIKORN-352) when child queue capacity greater
than parent, the configmap update is rejected but not notified to end user
[ https://issues.apache.org/jira/browse/YUNIKORN-352?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17276047#comment-17276047 ]
Wilfred Spiegelenburg commented on YUNIKORN-352:
------------------------------------------------
Tracking for later why I reopened this a while ago:
{quote}I had not looked at the change before but I see now that I needed to fix the panic in the resources code that we have an oversight in our checks and that we have introduced a behavioural change.
The check for resources does not take into account the fact that we can skip a level at the queues. If I create the following queue setup it will pass the max check and it will change the guaranteed setting for parent2:
\{{ root}}
\{{ – parent max == 100, guaranteed == 90}}
{{ – parent1 max == -, guaranteed = 40}}
{{ – parent2 max == -, guaranteed == -}}
{{ – child1 max == 75, guaranteed == 25}}
{{ – child2 max == 150, guaranteed == 25}}
First the guaranteed change:
The original code create a shadow queue structure to perform all checks on and then threw the structure away. It never modified the original config. So the parent2 queue would be created without a guarantee set. The configuration change will however result in a real parent queue change. When sorting queues the guaranteed resource is used. That means we have now have a queue with a guarantee and have changed the sorting of queues. Not sure if that is what we want or not.
Second max resource check:
The check is just the parent, it is the missing the recursive check. That has been an issue from the start and not introduced by this change. I think we need to plug this gap when we look at how we handle this behavioural change.
{quote}
And the second part of the exchange:
{quote}The root queue should never have a guaranteed resource set. It does not make sense. The root is the whole cluster and is a single queue at that level. The guaranteed resource is only used to sort queues. Root can never be sorted with itself.
Similar as to why the max resource should not be set from the config for the root the guaranteed resource should not be set for root. That is already documented.
For the check we need to set some rules and implement the rule so we can document them too:
* the root queue cannot have a maximum or guaranteed resource set
* a queue may not have a guaranteed resource larger than the maximum for that queue
* a child may not have a maximum that is larger than the maximum of its parent queue
* the sum of guaranteed resources of all child queues of a queue may not be larger than the guaranteed resources of that queue
NOTE: guaranteed resources for a queue can never be larger than maximum of that queue, this thus also implies that the sum of the child guaranteed resources can never be larger than the maximum. If a queue has no maximum set, the maximum of its direct parent is used in all checks, this applies recursively to the root.
For this last one I don't think we cover that at all and we need to revamp the check. This one is also not correct and not caught as far as I can tell:
{{root}}
{{ – parent max == 100, guaranteed == -}}
{{ – parent1 max == 80, guaranteed == -}}
{{ – child1 max == -, guaranteed == 50}}
{{ – child2 max == -, guaranteed == 40}}
{{ – parent2 max == -, guaranteed == -}}
{{ – child3 max == -, guaranteed == 125}}
This should fail for two reasons:
# child1 + child2 guaranteed is larger than parent1 max.
This fails the sum of all children guaranteed against guaranteed of the parent which is the max (40+50 > 80).
# child3 guaranteed is larger than parent2 max as it is inherited from parent (125 > 100).
In the resources code we have multiple functions that help us do the resource comparison. We should not need the function getLastValidMaxMap().
Starting at the root we have nil resources for max and guaranteed. Each level we go down we update the max and guaranteed we pass down the level. Guaranteed can be smaller than max so we need both passed down. Each time we go further down we adjust the max and guaranteed based on the queue we are working on. Max to be passed down is smallest of parent max and current queue max. Guaranteed to be passed down is the smallest of the max to be passed down and the current queue guaranteed resources.
If max and guaranteed are both nil passed in and the current queue has a max set we should before we do the child guaranteed check also set the guaranteed of that queue
{quote}
> when child queue capacity greater than parent, the configmap update is rejected but not notified to end user
> ------------------------------------------------------------------------------------------------------------
>
> Key: YUNIKORN-352
> URL: https://issues.apache.org/jira/browse/YUNIKORN-352
> Project: Apache YuniKorn
> Issue Type: Bug
> Components: core - scheduler
> Reporter: Ayub Pathan
> Assignee: Kinga Marton
> Priority: Major
> Labels: pull-request-available
> Fix For: 0.10
>
>
> Create a nested static queue like below.
> {noformat}
> partitions:
> -
> name: default
> placementrules:
> - name: tag
> value: namespace
> create: true
> queues:
> - name: root
> submitacl: '*'
> queues:
> - name: queue2
> resources:
> guaranteed:
> memory: 300
> cpu: 300
> max:
> memory: 1000
> cpu: 1000
> queues:
> - name: queue3
> resources:
> guaranteed:
> memory: 300
> cpu: 300
> max:
> memory: 2000
> cpu: 2000
> {noformat}
> Validate the same through rest API /queues - queues3 is not even shown in the response.
> {noformat}
> {
> "capacity": {
> "capacity": "map[attachable-volumes-aws-ebs:75 ephemeral-storage:94992122100 hugepages-1Gi:0 hugepages-2Mi:0 memory:18966 pods:87 vcore:4875]",
> "usedcapacity": "0"
> },
> "nodes": null,
> "partitionName": "[mycluster]default",
> "queues": {
> "capacities": {
> "absusedcapacity": "[memory:0 vcore:2]",
> "capacity": "[]",
> "maxcapacity": "[attachable-volumes-aws-ebs:75 ephemeral-storage:94992122100 hugepages-1Gi:0 hugepages-2Mi:0 memory:18966 pods:87 vcore:4875]",
> "usedcapacity": "[memory:1 vcore:110]"
> },
> "properties": {},
> "queuename": "root",
> "queues": [
> {
> "capacities": {
> "absusedcapacity": "[]",
> "capacity": "[]",
> "maxcapacity": "[]",
> "usedcapacity": "[memory:1]"
> },
> "properties": {},
> "queuename": "monitoring",
> "queues": null,
> "status": "Active"
> },
> {
> "capacities": {
> "absusedcapacity": "[]",
> "capacity": "[]",
> "maxcapacity": "[]",
> "usedcapacity": "[vcore:110]"
> },
> "properties": {},
> "queuename": "kube-system",
> "queues": null,
> "status": "Active"
> },
> {
> "capacities": {
> "absusedcapacity": "[]",
> "capacity": "[cpu:300 memory:300]",
> "maxcapacity": "[cpu:1000 memory:1000]",
> "usedcapacity": "[]"
> },
> "properties": {},
> "queuename": "queue2",
> "queues": null,
> "status": "Active"
> }
> ],
> "status": "Active"
> }
> }
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: issues-help@yunikorn.apache.org