You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@yunikorn.apache.org by ma...@apache.org on 2026/03/30 10:54:41 UTC

(yunikorn-core) branch master updated: [YUNIKORN-3240] Ensure leaf queues doesn't violate parent queue's guaranteed settings (#1074)

This is an automated email from the ASF dual-hosted git repository.

mani pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/yunikorn-core.git


The following commit(s) were added to refs/heads/master by this push:
     new e32624f4 [YUNIKORN-3240] Ensure leaf queues doesn't violate parent queue's guaranteed settings (#1074)
e32624f4 is described below

commit e32624f4856aa876e40313c1b614d324811e0252
Author: mani <ma...@gmail.com>
AuthorDate: Mon Mar 30 16:24:27 2026 +0530

    [YUNIKORN-3240] Ensure leaf queues doesn't violate parent queue's guaranteed settings (#1074)
    
    Improved Preemptable resources calculation to consider parent's and/or ancestor preemptable resources based on guaranteed as well to ensure preemptable resources is set correctly upfront for the queue being worked upon. Since preemptable resource has been defined upfront based on the above method, checks to ensure usage doesn't go below guaranteed at any level being carried out at much later stage are avoided
    
    Closes: #1074
    
    Signed-off-by: mani <ma...@gmail.com>
---
 pkg/scheduler/objects/queue.go                |  19 ++++
 pkg/scheduler/objects/quota_preemptor.go      |  65 +++++++-----
 pkg/scheduler/objects/quota_preemptor_test.go | 143 +++++++++++++++-----------
 3 files changed, 142 insertions(+), 85 deletions(-)

diff --git a/pkg/scheduler/objects/queue.go b/pkg/scheduler/objects/queue.go
index 45ce26db..37792fb2 100644
--- a/pkg/scheduler/objects/queue.go
+++ b/pkg/scheduler/objects/queue.go
@@ -816,6 +816,25 @@ func (sq *Queue) GetActualGuaranteedResource() *resources.Resource {
 	return resources.ComponentWiseMin(sq.guaranteedResource, parentGuaranteed)
 }
 
+// GetPreemptableResource returns the actual (including parent) preemptable resources for the queue.
+// Preemptable resource is nothing but the resources used above the guaranteed quota.
+func (sq *Queue) GetPreemptableResource() *resources.Resource {
+	if sq == nil {
+		return resources.NewResource()
+	}
+	parentPreemptable := sq.parent.GetPreemptableResource()
+	sq.RLock()
+	defer sq.RUnlock()
+
+	var preemptable *resources.Resource
+	if sq.guaranteedResource == nil {
+		preemptable = sq.allocatedResource.Clone()
+	} else {
+		preemptable = resources.SubOnlyExisting(sq.guaranteedResource, sq.allocatedResource.Clone())
+	}
+	return resources.ComponentWiseMin(preemptable, parentPreemptable)
+}
+
 func (sq *Queue) GetPreemptionDelay() time.Duration {
 	sq.RLock()
 	defer sq.RUnlock()
diff --git a/pkg/scheduler/objects/quota_preemptor.go b/pkg/scheduler/objects/quota_preemptor.go
index 108c2ce4..1eedf827 100644
--- a/pkg/scheduler/objects/quota_preemptor.go
+++ b/pkg/scheduler/objects/quota_preemptor.go
@@ -39,7 +39,7 @@ type QuotaPreemptionContext struct {
 }
 
 func NewQuotaPreemptor(queue *Queue) *QuotaPreemptionContext {
-	preemptor := &QuotaPreemptionContext{
+	return &QuotaPreemptionContext{
 		queue:               queue,
 		maxResource:         queue.cloneMaxResource(),
 		guaranteedResource:  queue.GetGuaranteedResource(),
@@ -48,8 +48,6 @@ func NewQuotaPreemptor(queue *Queue) *QuotaPreemptionContext {
 		preemptableResource: nil,
 		allocations:         make([]*Allocation, 0),
 	}
-
-	return preemptor
 }
 
 func (qpc *QuotaPreemptionContext) tryPreemption() {
@@ -60,7 +58,7 @@ func (qpc *QuotaPreemptionContext) tryPreemption() {
 		qpc.tryPreemptionInternal()
 		return
 	}
-	leafQueues := make(map[*Queue]*resources.Resource)
+	leafQueues := make(map[*Queue]*QuotaPreemptionContext)
 	getChildQueuesPreemptableResource(qpc.queue, qpc.preemptableResource, leafQueues)
 
 	log.Log(log.SchedQuotaChangePreemption).Info("Triggering quota change preemption for parent queue",
@@ -69,10 +67,8 @@ func (qpc *QuotaPreemptionContext) tryPreemption() {
 		zap.Int("no. of leaf queues with potential victims", len(leafQueues)),
 	)
 
-	for leaf, leafPreemptableResource := range leafQueues {
-		leafQueueQCPC := NewQuotaPreemptor(leaf)
-		leafQueueQCPC.preemptableResource = leafPreemptableResource
-		leafQueueQCPC.tryPreemptionInternal()
+	for _, leafContext := range leafQueues {
+		leafContext.tryPreemptionInternal()
 	}
 }
 
@@ -106,7 +102,7 @@ func (qpc *QuotaPreemptionContext) tryPreemptionInternal() {
 // In order to achieve a fair distribution of parent's preemptable resource among its children,
 // Higher (relatively) the usage is, higher the preemptable resource would be resulted in.
 // Usage above guaranteed (if set) is only considered to derive the preemptable resource.
-func getChildQueuesPreemptableResource(queue *Queue, parentPreemptableResource *resources.Resource, childQueues map[*Queue]*resources.Resource) {
+func getChildQueuesPreemptableResource(queue *Queue, parentPreemptableResource *resources.Resource, childQueues map[*Queue]*QuotaPreemptionContext) {
 	children := queue.GetCopyOfChildren()
 	if len(children) == 0 {
 		return
@@ -154,12 +150,14 @@ func getChildQueuesPreemptableResource(queue *Queue, parentPreemptableResource *
 		per := resources.GetSharesTypeWise(pRes, totalPreemptableResource)
 		for k := range pRes.Resources {
 			if _, ok := parentPreemptableResource.Resources[k]; ok {
-				value := math.RoundToEven(per[k] * float64(parentPreemptableResource.Resources[k]))
+				value := int(math.Floor(per[k] * float64(parentPreemptableResource.Resources[k])))
 				childPreemptableResource.Resources[k] = resources.Quantity(value)
 			}
 		}
 		if c.IsLeafQueue() {
-			childQueues[c] = childPreemptableResource
+			leafContext := NewQuotaPreemptor(c)
+			leafContext.preemptableResource = childPreemptableResource
+			childQueues[c] = leafContext
 		} else {
 			getChildQueuesPreemptableResource(c, childPreemptableResource, childQueues)
 		}
@@ -169,19 +167,41 @@ func getChildQueuesPreemptableResource(queue *Queue, parentPreemptableResource *
 // setPreemptableResources Get the preemptable resources for the queue
 // Subtracting the usage from the max resource gives the preemptable resources.
 // It could contain both positive and negative values. Only negative values are preemptable.
+// In addition, derive the minimum preemptable resources over parents based on guaranteed resources recursively
+// and do the minimum of this with earlier preemptable resources to derive the net preemptable resources
 func (qpc *QuotaPreemptionContext) setPreemptableResources() {
 	used := resources.SubOnlyExisting(qpc.allocatedResource, qpc.preemptingResource)
 	if qpc.maxResource.IsEmpty() || used.IsEmpty() {
 		return
 	}
 	actual := resources.SubOnlyExisting(qpc.maxResource, used)
-	preemptableResource := resources.NewResource()
+
+	netPreemptableResource := resources.NewResource()
 	// Keep only the resource type which needs to be preempted
 	for k, v := range actual.Resources {
 		if v < 0 {
-			preemptableResource.Resources[k] = resources.Quantity(math.Abs(float64(v)))
+			netPreemptableResource.Resources[k] = resources.Quantity(math.Abs(float64(v)))
+		}
+	}
+
+	parentPreemptableResource := qpc.queue.GetPreemptableResource()
+
+	netParentPreemptableResource := resources.NewResource()
+	// Keep only the resource type which needs to be preempted
+	for k, v := range parentPreemptableResource.Resources {
+		if v < 0 {
+			netParentPreemptableResource.Resources[k] = resources.Quantity(math.Abs(float64(v)))
 		}
 	}
+
+	// Min (current queue preemptable resources, Min(preemptable resources across parents based on guaranteed resources) )
+	preemptableResource := resources.ComponentWiseMinOnlyExisting(netPreemptableResource, netParentPreemptableResource)
+
+	log.Log(log.SchedQuotaChangePreemption).Debug("Preemptable resources calculated",
+		zap.String("queue", qpc.queue.GetQueuePath()),
+		zap.String("actual preemptable resources", netPreemptableResource.String()),
+		zap.String("preemptable resources based on guaranteed", netParentPreemptableResource.String()),
+		zap.String("net preemptable resources", preemptableResource.String()))
 	if preemptableResource.IsEmpty() {
 		return
 	}
@@ -269,22 +289,13 @@ func (qpc *QuotaPreemptionContext) preemptVictims() {
 			continue
 		}
 
-		// Keep collecting the victims until preemptable resource reaches and subtract the usage
-		if qpc.preemptableResource.StrictlyGreaterThanOnlyExisting(victimsTotalResource) {
+		// Keep collecting the victims until preemptable resource reaches
+		victimsTotalResource.AddTo(victimAlloc)
+		if qpc.preemptableResource.StrictlyGreaterThanOrEqualsOnlyExisting(victimsTotalResource) {
 			apps[application] = append(apps[application], victim)
-			qpc.allocatedResource.SubFrom(victimAlloc)
-		}
-
-		// Has usage gone below the guaranteed resources?
-		// If yes, revert the recently added victim steps completely and try next victim.
-		if !qpc.guaranteedResource.IsEmpty() && qpc.guaranteedResource.StrictlyGreaterThanOnlyExisting(qpc.allocatedResource) {
-			victims := apps[application]
-			exceptRecentlyAddedVictims := victims[:len(victims)-1]
-			apps[application] = exceptRecentlyAddedVictims
-			qpc.allocatedResource.AddTo(victimAlloc)
-			victimsTotalResource.SubFrom(victimAlloc)
 		} else {
-			victimsTotalResource.AddTo(victimAlloc)
+			// reverse to try next victim
+			victimsTotalResource.SubFrom(victimAlloc)
 		}
 	}
 
diff --git a/pkg/scheduler/objects/quota_preemptor_test.go b/pkg/scheduler/objects/quota_preemptor_test.go
index 04b923ec..d3dff3ed 100644
--- a/pkg/scheduler/objects/quota_preemptor_test.go
+++ b/pkg/scheduler/objects/quota_preemptor_test.go
@@ -30,33 +30,49 @@ import (
 )
 
 func TestQuotaChangeGetPreemptableResource(t *testing.T) {
+	parent, err := NewConfiguredQueue(configs.QueueConfig{
+		Name:   "parent",
+		Parent: true,
+	}, nil, false, nil)
+	assert.NilError(t, err)
+
 	leaf, err := NewConfiguredQueue(configs.QueueConfig{
 		Name: "leaf",
-	}, nil, false, nil)
+	}, parent, false, nil)
 	assert.NilError(t, err)
 
 	testCases := []struct {
-		name         string
-		queue        *Queue
-		maxResource  *resources.Resource
-		usedResource *resources.Resource
-		preemptable  *resources.Resource
+		name             string
+		queue            *Queue
+		parentGuaranteed *resources.Resource
+		maxResource      *resources.Resource
+		usedResource     *resources.Resource
+		preemptable      *resources.Resource
 	}{
-		{"nil max and nil used", leaf, nil, nil, nil},
-		{"nil max", leaf, nil, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000}), nil},
-		{"nil used", leaf, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000}), nil, nil},
-		{"used below max", leaf, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 500}), nil},
-		{"used above max", leaf, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1500}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 500})},
-		{"used above max, below max, equals max in specific res type and also with extra res types", leaf, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000, "cpu": 10, "gpu": 10}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1500, "cpu": 10, "gpu": 9}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 500})},
-		{"used res type but max undefined", leaf, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000}), resources.NewResourceFromMap(map[string]resources.Quantity{"cpu": 150}), nil},
+		{"nil max and nil used", leaf, nil, nil, nil, nil},
+		{"nil max", leaf, nil, nil, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000}), nil},
+		{"nil used", leaf, nil, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000}), nil, nil},
+		{"used below max", leaf, nil, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 500}), nil},
+		{"used above max", leaf, nil, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1500}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 500})},
+		{"used above max, below max, equals max in specific res type and also with extra res types", leaf, nil, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000, "cpu": 10, "gpu": 10}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1500, "cpu": 10, "gpu": 9}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 500})},
+		{"used res type but max undefined", leaf, nil, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000}), resources.NewResourceFromMap(map[string]resources.Quantity{"cpu": 150}), nil},
+		{"parent guaranteed set, lower than leaf's preemptable resources", leaf, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1200}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1500}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 300})},
+		{"parent guaranteed set, lower than leaf's preemptable resources - extra res types ", leaf, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1200}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1500, "cpu": 10, "gpu": 9}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 300})},
+		{"parent guaranteed set, higher than leaf's preemptable resources", leaf, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 900}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1500}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 500})},
+		{"parent guaranteed set, higher than leaf's preemptable resources - extra res types", leaf, resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 900}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1000}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 1500, "cpu": 10, "gpu": 9}), resources.NewResourceFromMap(map[string]resources.Quantity{"memory": 500})},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
+			tc.queue.parent.guaranteedResource = tc.parentGuaranteed
 			tc.queue.maxResource = tc.maxResource
-			tc.queue.allocatedResource = tc.usedResource
+			tc.queue.IncAllocatedResource(tc.usedResource)
 			preemptor := NewQuotaPreemptor(tc.queue)
 			preemptor.setPreemptableResources()
 			assert.Equal(t, resources.Equals(preemptor.preemptableResource, tc.preemptable), true)
+
+			// reset
+			err = tc.queue.DecAllocatedResource(tc.usedResource)
+			assert.NilError(t, err)
 		})
 	}
 }
@@ -172,10 +188,10 @@ func TestQuotaChangeTryPreemption(t *testing.T) {
 		{"no victims available", leaf, oldMax, newMax, nil, preemptable, []*Allocation{}, 0, 0},
 		{"suitable victims available", leaf, oldMax, newMax, nil, preemptable, suitableVictims, 2, 1},
 		{"skip over sized victims", leaf, oldMax, newMax, nil, preemptable, oversizedVictims, 2, 1},
-		{"guaranteed not set, victims total resource might go over the requirement a bit", leaf, oldMax, newMax, nil, preemptable, overflowVictims, 3, 2},
-		{"guaranteed set but lower than max, victims total resource might go over the requirement a bit", leaf, oldMax, newMax, lowerGuaranteed, preemptable, overflowVictims, 3, 2},
-		{"best effort - guaranteed set and equals max, victims total resource might fall below the requirement a bit", leaf, oldMax, newMax, guaranteed, preemptable, shortfallVictims, 4, 2},
-		{"best effort - guaranteed set, max not set earlier but now, victims total resource might fall below the requirement a bit", leaf, nil, newMax, guaranteed, preemptable, shortfallVictims, 4, 2},
+		{"guaranteed not set", leaf, oldMax, newMax, nil, preemptable, overflowVictims, 3, 1},
+		{"guaranteed set but lower than max", leaf, oldMax, newMax, lowerGuaranteed, preemptable, overflowVictims, 3, 1},
+		{"best effort - guaranteed set and equals max", leaf, oldMax, newMax, guaranteed, preemptable, shortfallVictims, 4, 2},
+		{"best effort - guaranteed set, max not set earlier but now", leaf, nil, newMax, guaranteed, preemptable, shortfallVictims, 4, 2},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -283,7 +299,7 @@ func TestQuotaChangeTryPreemptionWithDifferentResTypes(t *testing.T) {
 		},
 		{"overflow victims available with extra resource types other than defined in guaranteed and vice versa", leaf, oldMax, newMax, lowerGuaranteedWithNewResTypes,
 			[]test{
-				{overflowVictims, 3, 2},
+				{overflowVictims, 3, 1},
 			},
 		},
 	}
@@ -356,25 +372,25 @@ func TestQuotaChangeGetChildQueuesPreemptableResource(t *testing.T) {
 	}{
 		{"normal preemptable resources  - normal distribution", parent, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 100}),
 			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 22}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 11}),
-			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 42}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 25})},
+			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 41}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 24})},
 
 		{"twice the preemptable resources - twice the normal distribution", parent, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 200}),
-			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 45}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 22}),
-			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 83}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 50})},
+			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 44}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 22}),
+			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 83}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 49})},
 
 		{"half the preemptable resources - half the normal distribution", parent, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 50}),
-			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 11}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 6}),
-			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 21}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 12})},
+			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5}),
+			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 20}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 12})},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			childQueues := make(map[*Queue]*resources.Resource)
+			childQueues := make(map[*Queue]*QuotaPreemptionContext)
 			getChildQueuesPreemptableResource(tc.parentQueue, tc.parentPreemptable, childQueues)
 			assert.Equal(t, len(childQueues), 4)
-			assert.Equal(t, resources.Equals(childQueues[leaf111], tc.leaf111PRes), true)
-			assert.Equal(t, resources.Equals(childQueues[leaf12], tc.leaf12PRes), true)
-			assert.Equal(t, resources.Equals(childQueues[leaf211], tc.leaf211PRes), true)
-			assert.Equal(t, resources.Equals(childQueues[leaf22], tc.leaf22PRes), true)
+			assert.Equal(t, resources.Equals(childQueues[leaf111].preemptableResource, tc.leaf111PRes), true)
+			assert.Equal(t, resources.Equals(childQueues[leaf12].preemptableResource, tc.leaf12PRes), true)
+			assert.Equal(t, resources.Equals(childQueues[leaf211].preemptableResource, tc.leaf211PRes), true)
+			assert.Equal(t, resources.Equals(childQueues[leaf22].preemptableResource, tc.leaf22PRes), true)
 			if _, ok := childQueues[parent.GetChildQueue("leaf3")]; ok {
 				t.Fatal("leaf 3 queue exists")
 			}
@@ -427,25 +443,25 @@ func TestQuotaChangeGetChildQueuesPreemptableResourceWithDifferentResTypes(t *te
 	}{
 		{"normal preemptable resources  - normal distribution", parent, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 100, "fourth": 100}),
 			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 22}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 11}),
-			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 42}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 25})},
+			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 41}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 24})},
 
 		{"twice the preemptable resources - twice the normal distribution", parent, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 200, "fourth": 100}),
-			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 45}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 22}),
-			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 83}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 50})},
+			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 44}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 22}),
+			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 83}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 49})},
 
 		{"half the preemptable resources - half the normal distribution", parent, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 50, "fourth": 100}),
-			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 11}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 6}),
-			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 21}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 12})},
+			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5}),
+			resources.NewResourceFromMap(map[string]resources.Quantity{"first": 20}), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 12})},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			childQueues := make(map[*Queue]*resources.Resource)
+			childQueues := make(map[*Queue]*QuotaPreemptionContext)
 			getChildQueuesPreemptableResource(tc.parentQueue, tc.parentPreemptable, childQueues)
 			assert.Equal(t, len(childQueues), 4)
-			assert.Equal(t, resources.Equals(childQueues[leaf111], tc.leaf111PRes), true)
-			assert.Equal(t, resources.Equals(childQueues[leaf12], tc.leaf12PRes), true)
-			assert.Equal(t, resources.Equals(childQueues[leaf211], tc.leaf211PRes), true)
-			assert.Equal(t, resources.Equals(childQueues[leaf22], tc.leaf22PRes), true)
+			assert.Equal(t, resources.Equals(childQueues[leaf111].preemptableResource, tc.leaf111PRes), true)
+			assert.Equal(t, resources.Equals(childQueues[leaf12].preemptableResource, tc.leaf12PRes), true)
+			assert.Equal(t, resources.Equals(childQueues[leaf211].preemptableResource, tc.leaf211PRes), true)
+			assert.Equal(t, resources.Equals(childQueues[leaf22].preemptableResource, tc.leaf22PRes), true)
 			if _, ok := childQueues[parent.GetChildQueue("leaf3")]; ok {
 				t.Fatal("leaf 3 queue exists")
 			}
@@ -461,7 +477,7 @@ func TestQuotaChangeTryPreemptionForParentQueue(t *testing.T) {
 		NodeID:     "node",
 		Attributes: nil,
 		SchedulableResource: &si.Resource{
-			Resources: map[string]*si.Quantity{"first": {Value: 200}},
+			Resources: map[string]*si.Quantity{"first": {Value: 500}},
 		},
 	})
 
@@ -473,9 +489,15 @@ func TestQuotaChangeTryPreemptionForParentQueue(t *testing.T) {
 	parent1, err := NewConfiguredQueue(parentConfig1, nil, false, nil)
 	assert.NilError(t, err)
 
+	parentConfig2 := configs.QueueConfig{Name: "parent2", Parent: true}
+	parent2, err := NewConfiguredQueue(parentConfig2, nil, false, nil)
+	assert.NilError(t, err)
+
 	leaf111G, leaf12G, leaf211G, leaf22G, leaf4G := createQueueSetups(t, parent, configs.Resources{Guaranteed: map[string]string{"first": "10"}}, configs.Resources{})
 	leaf111, leaf12, leaf211, leaf22, leaf4 := createQueueSetups(t, parent1, configs.Resources{}, configs.Resources{})
 
+	leaf111WithParentG, leaf12WithParentG, leaf211WithParentG, leaf22WithParentG, leaf4WithParentG := createQueueSetups(t, parent2, configs.Resources{}, configs.Resources{})
+
 	suitableVictims := make([]*Allocation, 0)
 	suitableVictims = append(suitableVictims, createVictim(t, "ask1", node, 5, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10})))
 	suitableVictims = append(suitableVictims, createVictim(t, "ask2", node, 4, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10})))
@@ -505,7 +527,6 @@ func TestQuotaChangeTryPreemptionForParentQueue(t *testing.T) {
 	suitableVictims3 = append(suitableVictims3, createVictim(t, "ask11", node, 5, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10})))
 	suitableVictims3 = append(suitableVictims3, createVictim(t, "ask12", node, 4, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10})))
 	suitableVictims3 = append(suitableVictims3, createVictim(t, "ask13", node, 4, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10})))
-
 	leafGVictims[leaf22G] = suitableVictims3
 	leafVictims[leaf22] = suitableVictims3
 
@@ -513,20 +534,23 @@ func TestQuotaChangeTryPreemptionForParentQueue(t *testing.T) {
 	leafGVictims[leaf4G] = []*Allocation{v}
 	leafVictims[leaf4] = []*Allocation{v}
 
-	expectedGVictims := make(map[*Queue]int)
-	expectedGVictims[leaf111G] = 2
-	expectedGVictims[leaf12G] = 1
-	expectedGVictims[leaf211G] = 5
-	expectedGVictims[leaf22G] = 3
-
-	expectedVictims := make(map[*Queue]int)
-	expectedVictims[leaf111] = 3
-	expectedVictims[leaf12] = 2
-	expectedVictims[leaf211] = 5
-	expectedVictims[leaf22] = 3
+	leafVictimsWithParentG := make(map[*Queue][]*Allocation)
+	leafVictimsWithParentG[leaf111WithParentG] = []*Allocation{createVictim(t, "ask15", node, 4, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 13})),
+		createVictim(t, "ask16", node, 4, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 12}))}
+	leafVictimsWithParentG[leaf12WithParentG] = []*Allocation{createVictim(t, "ask17", node, 4, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 13})),
+		createVictim(t, "ask18", node, 4, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 12}))}
+	leafVictimsWithParentG[leaf211WithParentG] = []*Allocation{createVictim(t, "ask19", node, 4, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 13})),
+		createVictim(t, "ask20", node, 4, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 12}))}
+	leafVictimsWithParentG[leaf22WithParentG] = []*Allocation{createVictim(t, "ask21", node, 4, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 13})),
+		createVictim(t, "ask22", node, 4, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 12}))}
+	leafVictimsWithParentG[leaf4WithParentG] = []*Allocation{createVictim(t, "ask23", node, 4, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 13})),
+		createVictim(t, "ask24", node, 4, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 12}))}
 
 	oldMax := resources.NewResourceFromMap(map[string]resources.Quantity{"first": 130})
 	newMax := resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10})
+	oldMax1 := resources.NewResourceFromMap(map[string]resources.Quantity{"first": 150})
+	newMax1 := resources.NewResourceFromMap(map[string]resources.Quantity{"first": 20})
+	newMax2 := resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5})
 
 	testCases := []struct {
 		name            string
@@ -534,10 +558,12 @@ func TestQuotaChangeTryPreemptionForParentQueue(t *testing.T) {
 		oldMax          *resources.Resource
 		newMax          *resources.Resource
 		victims         map[*Queue][]*Allocation
-		expectedVictims map[*Queue]int
+		expectedVictims int
 	}{
-		{"Guaranteed set on one side of queue hierarchy - suitable victims available", parent, oldMax, newMax, leafGVictims, expectedGVictims},
-		{"Guaranteed set not set on any queue - suitable victims available", parent1, oldMax, newMax, leafVictims, expectedVictims},
+		{"Guaranteed set on one side of queue hierarchy - suitable victims available", parent, oldMax, newMax, leafGVictims, 11},
+		{"Guaranteed set not set on any queue - suitable victims available", parent1, oldMax, newMax, leafVictims, 9},
+		{"Guaranteed set only on parent queue but not on any child queues underneath - suitable victims available", parent2, oldMax1, newMax1, leafVictimsWithParentG, 5},
+		{"Guaranteed set only on parent queue but not on any child queues underneath - suitable victims available", parent2, oldMax1, newMax2, leafVictimsWithParentG, 5},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -546,17 +572,18 @@ func TestQuotaChangeTryPreemptionForParentQueue(t *testing.T) {
 				assignAllocationsToQueue(v, q)
 			}
 			tc.queue.maxResource = tc.newMax
+			tc.queue.guaranteedResource = tc.newMax
 			preemptor := NewQuotaPreemptor(tc.queue)
 			preemptor.tryPreemption()
-			for q, asks := range tc.victims {
-				var victimsCount int
+			victimsCount := 0
+			for _, asks := range tc.victims {
 				for _, a := range asks {
 					if a.IsPreempted() {
 						victimsCount++
 					}
 				}
-				assert.Equal(t, victimsCount, tc.expectedVictims[q])
 			}
+			assert.Equal(t, victimsCount, tc.expectedVictims)
 			for _, v := range tc.victims {
 				removeAllocationAsks(node, v)
 			}


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: issues-help@yunikorn.apache.org