Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Counters & Lists: Consolidate priorities sorting #3690

Merged
merged 3 commits into from
Mar 19, 2024
Merged

Counters & Lists: Consolidate priorities sorting #3690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6e1c648
Counters & Lists: Consolidate `priorities` sorting
markmandel Mar 6, 2024
59bc188
Merge branch 'main' into cleanup/priority-sameness
igooch Mar 18, 2024
2db88c7
Merge branch 'main' into cleanup/priority-sameness
agones-bot Mar 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions pkg/apis/agones/v1/gameserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -981,3 +981,87 @@ func MergeRemoveDuplicates(list1 []string, list2 []string) []string {
}
return uniqueList
}

// CompareCountAndListPriorities compares two game servers based on a list of CountsAndLists Priorities using available
// capacity as the comparison.
func (gs *GameServer) CompareCountAndListPriorities(priorities []Priority, other *GameServer) *bool {
for _, priority := range priorities {
res := gs.compareCountAndListPriority(&priority, other)
if res != nil {
// reverse if descending
if priority.Order == GameServerPriorityDescending {
flip := !*res
return &flip
}

return res
}
}

return nil
}

// compareCountAndListPriority compares two game servers based on a CountsAndLists Priority using available
// capacity (Capacity - Count for Counters, and Capacity - len(Values) for Lists) as the comparison.
// Returns true if gs1 < gs2; false if gs1 > gs2; nil if gs1 == gs2; nil if neither gamer server has the Priority.
// If only one game server has the Priority, prefer that server. I.e. nil < gsX when Priority
// Order is Descending (3, 2, 1, 0, nil), and nil > gsX when Order is Ascending (0, 1, 2, 3, nil).
func (gs *GameServer) compareCountAndListPriority(p *Priority, other *GameServer) *bool {
var gs1ok, gs2ok bool
t := true
f := false
switch p.Type {
case GameServerPriorityCounter:
// Check if both game servers contain the Counter.
counter1, ok1 := gs.Status.Counters[p.Key]
counter2, ok2 := other.Status.Counters[p.Key]
// If both game servers have the Counter
if ok1 && ok2 {
availCapacity1 := counter1.Capacity - counter1.Count
availCapacity2 := counter2.Capacity - counter2.Count
if availCapacity1 < availCapacity2 {
return &t
}
if availCapacity1 > availCapacity2 {
return &f
}
if availCapacity1 == availCapacity2 {
return nil
}
}
gs1ok = ok1
gs2ok = ok2
case GameServerPriorityList:
// Check if both game servers contain the List.
list1, ok1 := gs.Status.Lists[p.Key]
list2, ok2 := other.Status.Lists[p.Key]
// If both game servers have the List
if ok1 && ok2 {
availCapacity1 := list1.Capacity - int64(len(list1.Values))
availCapacity2 := list2.Capacity - int64(len(list2.Values))
if availCapacity1 < availCapacity2 {
return &t
}
if availCapacity1 > availCapacity2 {
return &f
}
if availCapacity1 == availCapacity2 {
return nil
}
}
gs1ok = ok1
gs2ok = ok2
}
// If only one game server has the Priority, prefer that server. I.e. nil < gsX when Order is
// Descending (3, 2, 1, 0, nil), and nil > gsX when Order is Ascending (0, 1, 2, 3, nil).
if (gs1ok && p.Order == GameServerPriorityDescending) ||
(gs2ok && p.Order == GameServerPriorityAscending) {
return &f
}
if (gs1ok && p.Order == GameServerPriorityAscending) ||
(gs2ok && p.Order == GameServerPriorityDescending) {
return &t
}
// If neither game server has the Priority
return nil
}
4 changes: 2 additions & 2 deletions pkg/fleetautoscalers/fleetautoscalers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -937,7 +937,7 @@ func TestApplyCounterPolicy(t *testing.T) {
}}}},
},
want: expected{
replicas: 2,
replicas: 1,
limited: false,
wantErr: false,
},
Expand Down Expand Up @@ -1729,7 +1729,7 @@ func TestApplyListPolicy(t *testing.T) {
}}}},
},
want: expected{
replicas: 3,
replicas: 4,
limited: false,
wantErr: false,
},
Expand Down
103 changes: 4 additions & 99 deletions pkg/gameserverallocations/allocation_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,24 +225,8 @@ func (c *AllocationCache) ListSortedGameServers(gsa *allocationv1.GameServerAllo

// if we end up here, then break the tie with Counter or List Priority.
if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) && (gsa != nil) {
for _, priority := range gsa.Spec.Priorities {
res := compareGameServers(&priority, gs1, gs2)
switch priority.Order {
case agonesv1.GameServerPriorityAscending:
if res == -1 {
return true
}
if res == 1 {
return false
}
case agonesv1.GameServerPriorityDescending:
if res == -1 {
return false
}
if res == 1 {
return true
}
}
if res := gs1.CompareCountAndListPriorities(gsa.Spec.Priorities, gs2); res != nil {
return *res
}
}

Expand All @@ -266,24 +250,8 @@ func (c *AllocationCache) ListSortedGameServersPriorities(gsa *allocationv1.Game
gs2 := list[j]

if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) && (gsa != nil) {
for _, priority := range gsa.Spec.Priorities {
res := compareGameServers(&priority, gs1, gs2)
switch priority.Order {
case agonesv1.GameServerPriorityAscending:
if res == -1 {
return true
}
if res == 1 {
return false
}
case agonesv1.GameServerPriorityDescending:
if res == -1 {
return false
}
if res == 1 {
return true
}
}
if res := gs1.CompareCountAndListPriorities(gsa.Spec.Priorities, gs2); res != nil {
return *res
}
}

Expand All @@ -294,69 +262,6 @@ func (c *AllocationCache) ListSortedGameServersPriorities(gsa *allocationv1.Game
return list
}

// compareGameServers compares two game servers based on a CountsAndLists Priority using available
// capacity (Capacity - Count for Counters, and Capacity - len(Values) for Lists) as the comparison.
// Returns -1 if gs1 < gs2; 1 if gs1 > gs2; 0 if gs1 == gs2; 0 if neither gamer server has the Priority.
// If only one game server has the Priority, prefer that server. I.e. nil < gsX when Priority
// Order is Descending (3, 2, 1, 0, nil), and nil > gsX when Order is Ascending (0, 1, 2, 3, nil).
func compareGameServers(p *agonesv1.Priority, gs1, gs2 *agonesv1.GameServer) int {
var gs1ok, gs2ok bool
switch p.Type {
case agonesv1.GameServerPriorityCounter:
// Check if both game servers contain the Counter.
counter1, ok1 := gs1.Status.Counters[p.Key]
counter2, ok2 := gs2.Status.Counters[p.Key]
// If both game servers have the Counter
if ok1 && ok2 {
availCapacity1 := counter1.Capacity - counter1.Count
availCapacity2 := counter2.Capacity - counter2.Count
if availCapacity1 < availCapacity2 {
return -1
}
if availCapacity1 > availCapacity2 {
return 1
}
if availCapacity1 == availCapacity2 {
return 0
}
}
gs1ok = ok1
gs2ok = ok2
case agonesv1.GameServerPriorityList:
// Check if both game servers contain the List.
list1, ok1 := gs1.Status.Lists[p.Key]
list2, ok2 := gs2.Status.Lists[p.Key]
// If both game servers have the List
if ok1 && ok2 {
availCapacity1 := list1.Capacity - int64(len(list1.Values))
availCapacity2 := list2.Capacity - int64(len(list2.Values))
if availCapacity1 < availCapacity2 {
return -1
}
if availCapacity1 > availCapacity2 {
return 1
}
if availCapacity1 == availCapacity2 {
return 0
}
}
gs1ok = ok1
gs2ok = ok2
}
// If only one game server has the Priority, prefer that server. I.e. nil < gsX when Order is
// Descending (3, 2, 1, 0, nil), and nil > gsX when Order is Ascending (0, 1, 2, 3, nil).
if (gs1ok && p.Order == agonesv1.GameServerPriorityDescending) ||
(gs2ok && p.Order == agonesv1.GameServerPriorityAscending) {
return 1
}
if (gs1ok && p.Order == agonesv1.GameServerPriorityAscending) ||
(gs2ok && p.Order == agonesv1.GameServerPriorityDescending) {
return -1
}
// If neither game server has the Priority
return 0
}

// SyncGameServers synchronises the GameServers to Gameserver cache. This is called when a failure
// happened during the allocation. This method will sync and make sure the cache is up to date.
func (c *AllocationCache) SyncGameServers(ctx context.Context, key string) error {
Expand Down
123 changes: 9 additions & 114 deletions pkg/gameserversets/gameserversets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,31 +86,15 @@ func sortGameServersByPackedStrategy(list []*agonesv1.GameServer, count map[stri
return false
}

// if the Nodes have the same count and CountsAndLists flag is enabled, sort based on CountsAndLists Priorities.
if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) {
for _, priority := range priorities {
res := compareGameServerCapacity(&priority, a, b)
switch priority.Order {
case agonesv1.GameServerPriorityAscending:
if res == -1 {
return true
}
if res == 1 {
return false
}
case agonesv1.GameServerPriorityDescending:
if res == -1 {
return false
}
if res == 1 {
return true
}
if a.Status.NodeName == b.Status.NodeName {
// See if Count and List priorities can be used as a tie-breaker within the node
if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) {
if res := a.CompareCountAndListPriorities(priorities, b); res != nil {
return *res
}
}
}

// if the gameservers are on the same node, then sort lexigraphically for a stable sort
if a.Status.NodeName == b.Status.NodeName {
// Sort lexicographically for a stable sort within the node
return a.GetObjectMeta().GetName() < b.GetObjectMeta().GetName()
}
// if both Nodes have the same count, one node is emptied first (packed scheduling behavior)
Expand All @@ -121,31 +105,15 @@ func sortGameServersByPackedStrategy(list []*agonesv1.GameServer, count map[stri
}

// sortGameServersByDistributedStrategy sorts by newest gameservers first.
// If FeatureCountsAndLists is enabled, sort by Priority first, then tie break with newest gameservers.
// If FeatureCountsAndLists is enabled, sort by Priority first, then tie-break with newest gameservers.
func sortGameServersByDistributedStrategy(list []*agonesv1.GameServer, priorities []agonesv1.Priority) []*agonesv1.GameServer {
sort.Slice(list, func(i, j int) bool {
a := list[i]
b := list[j]

if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) {
for _, priority := range priorities {
res := compareGameServerCapacity(&priority, a, b)
switch priority.Order {
case agonesv1.GameServerPriorityAscending:
if res == -1 {
return true
}
if res == 1 {
return false
}
case agonesv1.GameServerPriorityDescending:
if res == -1 {
return false
}
if res == 1 {
return true
}
}
if res := a.CompareCountAndListPriorities(priorities, b); res != nil {
return *res
}
}

Expand All @@ -172,76 +140,3 @@ func ListGameServersByGameServerSetOwner(gameServerLister listerv1.GameServerLis

return result, nil
}

// compareGameServerCapacity compares two game servers based on a CountsAndLists Priority. First
// compares by Capacity, and then compares by Count or length of the List Values if Capacity is equal.
// NOTE: This does NOT sort by available capacity.
// Returns -1 if gs1 < gs2; 1 if gs1 > gs2; 0 if gs1 == gs2; 0 if neither gamer server has the Priority.
// If only one game server has the Priority, prefer that server. I.e. nil < gsX when Priority
// Order is Descending (3, 2, 1, 0, nil), and nil > gsX when Order is Ascending (0, 1, 2, 3, nil).
func compareGameServerCapacity(p *agonesv1.Priority, gs1, gs2 *agonesv1.GameServer) int {
var gs1ok, gs2ok bool
switch p.Type {
case agonesv1.GameServerPriorityCounter:
// Check if both game servers contain the Counter.
counter1, ok1 := gs1.Status.Counters[p.Key]
counter2, ok2 := gs2.Status.Counters[p.Key]
// If both game servers have the Counter
if ok1 && ok2 {
if counter1.Capacity < counter2.Capacity {
return -1
}
if counter1.Capacity > counter2.Capacity {
return 1
}
if counter1.Capacity == counter2.Capacity {
if counter1.Count < counter2.Count {
return -1
}
if counter1.Count > counter2.Count {
return 1
}
return 0
}
}
gs1ok = ok1
gs2ok = ok2
case agonesv1.GameServerPriorityList:
// Check if both game servers contain the List.
list1, ok1 := gs1.Status.Lists[p.Key]
list2, ok2 := gs2.Status.Lists[p.Key]
// If both game servers have the List
if ok1 && ok2 {
if list1.Capacity < list2.Capacity {
return -1
}
if list1.Capacity > list2.Capacity {
return 1
}
if list1.Capacity == list2.Capacity {
if len(list1.Values) < len(list2.Values) {
return -1
}
if len(list1.Values) > len(list2.Values) {
return 1
}
return 0
}
}
gs1ok = ok1
gs2ok = ok2
}
// If only one game server has the Priority, prefer that server. I.e. nil < gsX when Order is
// Descending (3, 2, 1, 0, nil), and nil > gsX when Order is Ascending (0, 1, 2, 3, nil).
// No Order specified "" is the same as Ascending.
if (gs1ok && p.Order == agonesv1.GameServerPriorityDescending) ||
(gs2ok && p.Order == agonesv1.GameServerPriorityAscending) {
return 1
}
if (gs1ok && p.Order == agonesv1.GameServerPriorityAscending) ||
(gs2ok && p.Order == agonesv1.GameServerPriorityDescending) {
return -1
}
// If neither game server has the Priority
return 0
}
Loading
Loading