Skip to content

Commit fac9b6b

Browse files
manirajv06manirajv06
committed
[YUNIKORN-3233] Improve victim selection algorithm (#1072)
Closes: #1072 Signed-off-by: mani <manirajv06@gmail.com>
1 parent 956ade5 commit fac9b6b

5 files changed

Lines changed: 254 additions & 1 deletion

File tree

‎pkg/common/resources/resources.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,3 +1209,48 @@ func (r *Resource) DominantResourceType(capacity *Resource) string {
12091209
}
12101210
return dominant
12111211
}
1212+
1213+
func (r *Resource) TypeMatching(other *Resource) uint64 {
1214+
if r == nil || other == nil {
1215+
return 0
1216+
}
1217+
matchingResTypes := 0
1218+
for k := range other.Resources {
1219+
if _, ok := r.Resources[k]; ok {
1220+
matchingResTypes++
1221+
}
1222+
}
1223+
return uint64(matchingResTypes * 100 / len(r.Resources))
1224+
}
1225+
1226+
// CompUsageRatioSpecificTypes Compare the left and right resources based on the ask resources.
1227+
// Collect resource types of both left and right resources from ask resource perspective and ignore others.
1228+
// Sort the shares for the collected resource types in increasing order and decide the largest resource
1229+
// by comparing each share starting from last index to 0th index.
1230+
func CompUsageRatioSpecificTypes(left, right, total, specificTypes *Resource) int {
1231+
lShareTypeWise := GetSharesTypeWise(left, total)
1232+
rShareTypeWise := GetSharesTypeWise(right, total)
1233+
if specificTypes != nil {
1234+
var lShare []float64
1235+
var rShare []float64
1236+
for k := range specificTypes.Resources {
1237+
lValue, lExists := lShareTypeWise[k]
1238+
rValue, rExists := rShareTypeWise[k]
1239+
if lExists {
1240+
lShare = append(lShare, lValue)
1241+
}
1242+
if rExists {
1243+
rShare = append(rShare, rValue)
1244+
}
1245+
}
1246+
if len(lShare) == 0 && len(rShare) == 0 {
1247+
return 0
1248+
}
1249+
sort.Float64s(lShare)
1250+
sort.Float64s(rShare)
1251+
return compareShares(lShare, rShare)
1252+
} else {
1253+
// fall back to usual way of comparing usage shares.
1254+
return CompUsageRatio(left, right, total)
1255+
}
1256+
}

‎pkg/common/resources/resources_test.go‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2470,3 +2470,72 @@ func TestResource_Prune(t *testing.T) {
24702470
})
24712471
}
24722472
}
2473+
2474+
func TestTypeMatching(t *testing.T) {
2475+
var tests = []struct {
2476+
name string
2477+
base *Resource
2478+
other *Resource
2479+
expected uint64
2480+
}{
2481+
{"nil base resource", nil, NewResourceFromMap(map[string]Quantity{"first": 20}), 0},
2482+
{"nil other resource", NewResourceFromMap(map[string]Quantity{"first": 10}), nil, 0},
2483+
{"both nil resources", nil, nil, 0},
2484+
{"same resources", NewResourceFromMap(map[string]Quantity{"first": 10}), NewResourceFromMap(map[string]Quantity{"first": 20}), 100},
2485+
{"half of the resources matches", NewResourceFromMap(map[string]Quantity{"first": 10, "second": 10}), NewResourceFromMap(map[string]Quantity{"first": 10}), 50},
2486+
{"one third of the resources matches", NewResourceFromMap(map[string]Quantity{"first": 10, "second": 10, "third": 10}), NewResourceFromMap(map[string]Quantity{"first": 10}), 33},
2487+
{"one fourth of the resources matches", NewResourceFromMap(map[string]Quantity{"first": 10, "second": 10, "third": 10, "fourth": 10}), NewResourceFromMap(map[string]Quantity{"first": 10}), 25},
2488+
{"one fourth of the resources matches, extra types in other resource should not affect", NewResourceFromMap(map[string]Quantity{"first": 10, "second": 10, "third": 10, "fourth": 10}), NewResourceFromMap(map[string]Quantity{"first": 10, "fifth": 10, "sixth": 10}), 25},
2489+
}
2490+
for _, tt := range tests {
2491+
t.Run(tt.name, func(t *testing.T) {
2492+
assert.Equal(t, tt.base.TypeMatching(tt.other), tt.expected)
2493+
})
2494+
}
2495+
}
2496+
2497+
func TestCompUsageRatioSpecificTypes(t *testing.T) {
2498+
tests := []struct {
2499+
name string
2500+
left *Resource
2501+
right *Resource
2502+
total *Resource
2503+
ask *Resource
2504+
expected int
2505+
}{
2506+
{"nil resources", nil, nil, nil, NewResourceFromMap(map[string]Quantity{"first": 10}), 0},
2507+
{"empty resource with total nil", NewResource(), NewResource(), nil, NewResourceFromMap(map[string]Quantity{"first": 10}), 0},
2508+
{"empty resource", NewResource(), NewResource(), NewResource(), NewResourceFromMap(map[string]Quantity{"first": 10}), 0},
2509+
{"zero valued resource but extra types differs and not present in ask", NewResourceFromMap(map[string]Quantity{"zero": 0, "extra_a": 1}), NewResourceFromMap(map[string]Quantity{"zero": 0, "extra_a": 2}), nil, NewResourceFromMap(map[string]Quantity{"zero": 10, "first": 10}), 0},
2510+
{"negative valued resource", NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0, "small": -2, "extra_a": 1}), NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0, "small": -5, "extra_a": 2}), nil, NewResourceFromMap(map[string]Quantity{"large": 5, "small": 1}), 1},
2511+
{"negative valued resource on left side, ask is nil", NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0, "small": -5}), NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0}), nil, nil, -1},
2512+
{"negative valued resource on right side, ask is nil", NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0}), NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0, "small": -5}), nil, nil, 1},
2513+
{"negative valued resource but extra types differs and not present in ask", NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0, "small": -5, "extra_a": 1}), NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0, "small": -5, "extra_a": 2}), nil, NewResourceFromMap(map[string]Quantity{"large": 5}), 0},
2514+
{"zero valued resource with total but extra types differs and not present in ask", NewResourceFromMap(map[string]Quantity{"zero": 0, "extra_a": 1}), NewResourceFromMap(map[string]Quantity{"zero": 0, "extra_a": 2}), NewResource(), NewResourceFromMap(map[string]Quantity{"zero": 0}), 0},
2515+
{"same resource and total but extra types differs and not present in ask", NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0, "small": -5, "extra_a": 1}), NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0, "small": -5, "extra_a": 2}), NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0, "small": -5, "extra_a": 10}), NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0, "small": -5}), 0},
2516+
{"left side has more one negative value for type present in ask but not present in right", NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0, "small": -5}), NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0}), NewResourceFromMap(map[string]Quantity{"large": 10, "zero": 10}), NewResourceFromMap(map[string]Quantity{"large": 100, "zero": 100, "small": 5}), -1},
2517+
{"left side has more one negative value but not present in ask", NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0, "small": -5}), NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0}), NewResourceFromMap(map[string]Quantity{"large": 10, "zero": 10}), NewResourceFromMap(map[string]Quantity{"large": 100, "zero": 100}), 0},
2518+
{"right side has more one negative value for type present in ask but not present in left", NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0}), NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0, "small": -5}), NewResourceFromMap(map[string]Quantity{"large": 10, "zero": 10}), NewResourceFromMap(map[string]Quantity{"large": 100, "zero": 100, "small": 5}), 1},
2519+
{"right side has more one negative value but not present in ask", NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0}), NewResourceFromMap(map[string]Quantity{"large": 5, "zero": 0, "small": -5}), NewResourceFromMap(map[string]Quantity{"large": 10, "zero": 10}), NewResourceFromMap(map[string]Quantity{"large": 100, "zero": 100}), 0},
2520+
{"left side first one bigger, last one smaller. right side vice versa. but share wise same", NewResourceFromMap(map[string]Quantity{"first": 10, "second": 5}), NewResourceFromMap(map[string]Quantity{"first": 5, "second": 10}), NewResourceFromMap(map[string]Quantity{"first": 15, "second": 15}), NewResourceFromMap(map[string]Quantity{"first": 1, "second": 1}), 0},
2521+
{"left side first one bigger, last one smaller. right side vice versa. but share wise not same", NewResourceFromMap(map[string]Quantity{"first": 10, "second": 5}), NewResourceFromMap(map[string]Quantity{"first": 5, "second": 10}), NewResourceFromMap(map[string]Quantity{"first": 15}), NewResourceFromMap(map[string]Quantity{"first": 1, "second": 1}), -1},
2522+
{"left side first one bigger, last one smaller. right side vice versa. but share wise not same. none of the ask res types match", NewResourceFromMap(map[string]Quantity{"first": 10, "second": 5}), NewResourceFromMap(map[string]Quantity{"first": 5, "second": 10}), NewResourceFromMap(map[string]Quantity{"first": 15}), NewResourceFromMap(map[string]Quantity{"third": 1, "fourth": 1}), 0},
2523+
{"left side first one smaller, last one bigger. right side vice versa. but share wise not same", NewResourceFromMap(map[string]Quantity{"first": 5, "second": 10}), NewResourceFromMap(map[string]Quantity{"first": 10, "second": 5}), NewResourceFromMap(map[string]Quantity{"first": 15}), NewResourceFromMap(map[string]Quantity{"first": 1, "second": 1}), 1},
2524+
{"left side first one smaller, last one bigger. right side vice versa. but share wise not same. none of the ask res types match", NewResourceFromMap(map[string]Quantity{"first": 5, "second": 10}), NewResourceFromMap(map[string]Quantity{"first": 10, "second": 5}), NewResourceFromMap(map[string]Quantity{"first": 15}), NewResourceFromMap(map[string]Quantity{"third": 1, "fourth": 1}), 0},
2525+
{"left side key order not same as right side which is aligned with ask order ", NewResourceFromMap(map[string]Quantity{"second": 10, "first": 5}), NewResourceFromMap(map[string]Quantity{"first": 10, "second": 5}), NewResourceFromMap(map[string]Quantity{}), NewResourceFromMap(map[string]Quantity{"first": 1, "second": 1}), 0},
2526+
{"right side key order not same as left side which is aligned with ask order", NewResourceFromMap(map[string]Quantity{"second": 10, "first": 5}), NewResourceFromMap(map[string]Quantity{"first": 10, "second": 5}), NewResourceFromMap(map[string]Quantity{}), NewResourceFromMap(map[string]Quantity{"second": 1, "first": 1}), 0},
2527+
}
2528+
2529+
for _, tc := range tests {
2530+
t.Run(tc.name, func(t *testing.T) {
2531+
ratio := CompUsageRatioSpecificTypes(tc.left, tc.right, tc.total, tc.ask)
2532+
if ratio != tc.expected {
2533+
t.Errorf("incorrect ratio, expected %v got: %v", tc.expected, ratio)
2534+
}
2535+
if tc.ask == nil {
2536+
assert.Equal(t, ratio, CompUsageRatio(tc.left, tc.right, tc.total))
2537+
assert.Equal(t, ratio, tc.expected)
2538+
}
2539+
})
2540+
}
2541+
}

‎pkg/scheduler/objects/preemption_utilities.go‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ package objects
2020

2121
import (
2222
"sort"
23+
"time"
2324

2425
"github.com/apache/yunikorn-core/pkg/common/resources"
2526
)
@@ -79,3 +80,60 @@ func SortAllocations(allocations []*Allocation) {
7980
return true
8081
})
8182
}
83+
84+
func SortAllocationsBasedOnAsk(allocations []*Allocation, total, ask *resources.Resource) {
85+
sort.SliceStable(allocations, func(i, j int) bool {
86+
l := allocations[i]
87+
r := allocations[j]
88+
89+
scoreLeft := scoreAllocationBasedOnAsk(l, ask)
90+
scoreRight := scoreAllocationBasedOnAsk(r, ask)
91+
if scoreLeft != scoreRight {
92+
return scoreLeft > scoreRight
93+
}
94+
95+
// sort based on the priority
96+
lPriority := l.GetPriority()
97+
rPriority := r.GetPriority()
98+
if lPriority < rPriority {
99+
return true
100+
}
101+
if lPriority > rPriority {
102+
return false
103+
}
104+
105+
// sort based on the age (limiting the boundary to hour max)
106+
lHour := l.GetCreateTime().Truncate(time.Hour)
107+
rHour := r.GetCreateTime().Truncate(time.Hour)
108+
if !lHour.Equal(rHour) {
109+
return lHour.After(rHour)
110+
}
111+
112+
// sort based on the allocated resource
113+
lResource := l.GetAllocatedResource()
114+
rResource := r.GetAllocatedResource()
115+
comp := resources.CompUsageRatioSpecificTypes(lResource, rResource, total, ask)
116+
if comp == -1 {
117+
return true
118+
}
119+
if comp == 1 {
120+
return false
121+
}
122+
return true
123+
})
124+
}
125+
126+
// scoreAllocation generates a relative score for an allocation. Lower-scored allocations are considered more likely
127+
// preemption candidates. Tasks which have opted into preemption are considered first, then tasks which are not
128+
// application originators.
129+
func scoreAllocationBasedOnAsk(allocation *Allocation, ask *resources.Resource) uint64 {
130+
var score uint64 = 0
131+
if allocation.IsOriginator() {
132+
score |= scoreOriginator
133+
}
134+
if !allocation.IsAllowPreemptSelf() {
135+
score |= scoreNoPreempt
136+
}
137+
score += allocation.GetAllocatedResource().TypeMatching(ask)
138+
return score
139+
}

‎pkg/scheduler/objects/preemption_utilities_test.go‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,3 +211,84 @@ func TestSortAllocations(t *testing.T) {
211211

212212
removeAllocationAsks(node, asks)
213213
}
214+
215+
func TestSortAllocationsBasedOnAsk(t *testing.T) {
216+
node := NewNode(&si.NodeInfo{
217+
NodeID: "node",
218+
Attributes: nil,
219+
SchedulableResource: &si.Resource{
220+
Resources: map[string]*si.Quantity{"first": {Value: 100}, "second": {Value: 100}, "third": {Value: 100}, "fourth": {Value: 100},
221+
"extra_a": {Value: 100}, "extra_b": {Value: 100}, "extra_c": {Value: 100}, "extra_d": {Value: 100}, "extra_e": {Value: 100},
222+
"extra_f": {Value: 100}, "extra_g": {Value: 100}, "extra_h": {Value: 100}, "extra_i": {Value: 100}, "extra_j": {Value: 100}},
223+
},
224+
})
225+
226+
type vStruct struct {
227+
key string
228+
allocated *resources.Resource
229+
}
230+
231+
// Victims for res types matching comparison
232+
res1 := vStruct{"ask1", resources.NewResourceFromMap(map[string]resources.Quantity{"first": 13})}
233+
res2 := vStruct{"ask2", resources.NewResourceFromMap(map[string]resources.Quantity{"first": 12, "second": 10})}
234+
res3 := vStruct{"ask3", resources.NewResourceFromMap(map[string]resources.Quantity{"first": 11, "second": 10, "third": 10})}
235+
res4 := vStruct{"ask4", resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10, "second": 10, "third": 10, "fourth": 10})}
236+
askWithExtraRes := resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10, "extra_a": 10, "extra_b": 10})
237+
238+
// Victims for allocated resources comparison
239+
res5 := vStruct{"ask4", resources.NewResourceFromMap(map[string]resources.Quantity{"first": 13, "extra_c": 10, "extra_d": 10})}
240+
res6 := vStruct{"ask3", resources.NewResourceFromMap(map[string]resources.Quantity{"first": 12, "extra_e": 10, "extra_f": 10})}
241+
res7 := vStruct{"ask2", resources.NewResourceFromMap(map[string]resources.Quantity{"first": 11, "extra_g": 10, "extra_h": 10})}
242+
res8 := vStruct{"ask1", resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10, "extra_i": 10, "extra_j": 10})}
243+
res9 := vStruct{"ask4", resources.NewResourceFromMap(map[string]resources.Quantity{"first": -13, "extra_c": 10, "extra_d": 10})}
244+
245+
res10 := vStruct{"ask2", resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5, "second": 10})}
246+
res11 := vStruct{"ask1", resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10, "second": 5})}
247+
res12 := vStruct{"ask1", resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5})}
248+
res13 := vStruct{"ask2", resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10, "second": 10})}
249+
askWithExtraRes1 := resources.NewResourceFromMap(map[string]resources.Quantity{"first": 1, "second": 1})
250+
251+
total := resources.NewResourceFromMap(map[string]resources.Quantity{"first": 100, "second": 100, "third": 100, "fourth": 100})
252+
253+
testCases := []struct {
254+
name string
255+
victims []vStruct
256+
total *resources.Resource
257+
ask *resources.Resource
258+
expectedVictims []string
259+
}{
260+
{"100% matching victims, final order should be based on matching % in decreasing order. ask extra res should not create any impact",
261+
[]vStruct{res1}, total, askWithExtraRes, []string{"ask1"}},
262+
{"50% matching victims, final order should be based on matching % in decreasing order. ask extra res should not create any impact",
263+
[]vStruct{res2, res1}, total, askWithExtraRes, []string{"ask1", "ask2"}},
264+
{"33% matching victims, final order should be based on matching % in decreasing order. ask extra res should not create any impact",
265+
[]vStruct{res3, res2, res1}, total, askWithExtraRes, []string{"ask1", "ask2", "ask3"}},
266+
{"25% matching victims, final order should be based on matching % in decreasing order. ask extra res should not create any impact",
267+
[]vStruct{res4, res3, res2, res1}, total, askWithExtraRes, []string{"ask1", "ask2", "ask3", "ask4"}},
268+
{"ordering based on matching res types, non matching res types does not matter",
269+
[]vStruct{res5, res6, res7, res8}, total, askWithExtraRes, []string{"ask1", "ask2", "ask3", "ask4"}},
270+
{"negative value resource, ordering based on matching res types, non matching res types does not matter",
271+
[]vStruct{res6, res7, res8, res9}, total, askWithExtraRes, []string{"ask4", "ask1", "ask2", "ask3"}},
272+
{"first victim one res type bigger and another smaller and vice versa on second victim",
273+
[]vStruct{res10, res11}, total, askWithExtraRes1, []string{"ask1", "ask2"}},
274+
{"first victim one res type bigger and another smaller and vice versa on second victim",
275+
[]vStruct{res13, res12}, total, askWithExtraRes1, []string{"ask1", "ask2"}},
276+
}
277+
for _, tc := range testCases {
278+
t.Run(tc.name, func(t *testing.T) {
279+
var victimAllocations []*Allocation
280+
for _, vRes := range tc.victims {
281+
// Except victim used resources and ask resources, have all other criteria same for all victims
282+
victim := createAllocation(vRes.key, "app1", node.NodeID, true, false, 10, false,
283+
vRes.allocated)
284+
assert.Assert(t, node.TryAddAllocation(victim))
285+
victimAllocations = append(victimAllocations, victim)
286+
}
287+
SortAllocationsBasedOnAsk(victimAllocations, tc.total, tc.ask)
288+
for index, sortedAsk := range victimAllocations {
289+
assert.Equal(t, sortedAsk.GetAllocationKey(), tc.expectedVictims[index])
290+
}
291+
removeAllocationAsks(node, victimAllocations)
292+
})
293+
}
294+
}

‎pkg/scheduler/objects/quota_preemptor.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ func (qpc *QuotaPreemptionContext) filterAllocations() {
231231
// sortAllocations Sort the allocations running in the queue
232232
func (qpc *QuotaPreemptionContext) sortAllocations() {
233233
if len(qpc.allocations) > 0 {
234-
SortAllocations(qpc.allocations)
234+
SortAllocationsBasedOnAsk(qpc.allocations, qpc.maxResource, qpc.preemptableResource)
235235
}
236236
}
237237

0 commit comments

Comments
 (0)