overcommit plugin AddForMinReq is no need to add other resources. #3521
Conversation
|
Welcome @xieyanker! |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
volcano-sh-bot
added
the
size/M
| if rr.MilliCPU == 0 { | ||
| r.MilliCPU = 0 | ||
| } else { | ||
| r.MilliCPU += rr.MilliCPU | ||
| } | ||
|
|
||
| if rr.Memory == 0 { | ||
| r.Memory = 0 | ||
| } else { | ||
| r.Memory += rr.Memory | ||
| } |
There was a problem hiding this comment.
should we just use this
r.MilliCPU += rr.MilliCPU
r.Memory += rr.Memory
| newScalarResources := map[v1.ResourceName]float64{} | ||
| for rName, rQuant := range rr.ScalarResources { | ||
| if r.ScalarResources == nil { | ||
| r.ScalarResources = map[v1.ResourceName]float64{} | ||
| } | ||
| newScalarResources[rName] = r.ScalarResources[rName] + rQuant | ||
| } | ||
| r.ScalarResources = newScalarResources |
There was a problem hiding this comment.
Is this code OK? It is relatively more readable
if r.ScalarResources == nil {
r.ScalarResources = map[v1.ResourceName]float64{}
}
for rName, rQuant := range rr.ScalarResources {
r.ScalarResources[rName] += rQuant
}
| @@ -144,3 +144,4 @@ func (op *overcommitPlugin) OnSessionClose(ssn *framework.Session) { | |||
| op.idleResource = nil | |||
| op.inqueueResource = nil | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Can this blank line be deleted?
In addition, can some test cases be added to prove this pr?
|
/ok-to-test |
volcano-sh-bot
added
the
ok-to-test
For example, if the minimum request is only 1 CPU core, then there is no need to add memory, GPU and other resources. Signed-off-by: xieyanker <[email protected]>
xieyanker
force-pushed
the
overcommit_add
branch
from
20abaa8
to
3a827bf
Compare
|
/hold change origin value is a bit strange. |
volcano-sh-bot
added
the
do-not-merge/hold
Fix #3520