Skip to content
Permalink
Browse files

Replaced sortable list with native golang slice.

  • Loading branch information
hprateek43 committed Oct 11, 2019
1 parent e62ed95 commit 3b1d75e0f6ac75b58caab938ab96e4e692d84bd3
@@ -19,6 +19,7 @@ package core
import (
"fmt"
"reflect"
"sort"
"testing"
"time"

@@ -202,7 +203,7 @@ func (f *FakeExtender) selectVictimsOnNodeByExtender(
// and get cached node info by given node name.
nodeInfoCopy := f.cachedNodeNameToInfo[node.GetName()].Clone()

potentialVictims := util.SortableList{CompFunc: util.MoreImportantPod}
var potentialVictims []*v1.Pod

removePod := func(rp *v1.Pod) {
nodeInfoCopy.RemovePod(rp)
@@ -215,11 +216,11 @@ func (f *FakeExtender) selectVictimsOnNodeByExtender(
podPriority := podutil.GetPodPriority(pod)
for _, p := range nodeInfoCopy.Pods() {
if podutil.GetPodPriority(p) < podPriority {
potentialVictims.Items = append(potentialVictims.Items, p)
potentialVictims = append(potentialVictims, p)
removePod(p)
}
}
potentialVictims.Sort()
sort.Slice(potentialVictims, func(i, j int) bool { return util.MoreImportantPod(potentialVictims[i], potentialVictims[j]) })

// If the new pod does not fit after removing all the lower priority pods,
// we are almost done and this node is not suitable for preemption.
@@ -248,8 +249,8 @@ func (f *FakeExtender) selectVictimsOnNodeByExtender(

// For now, assume all potential victims to be non-violating.
// Now we try to reprieve non-violating victims.
for _, p := range potentialVictims.Items {
reprievePod(p.(*v1.Pod))
for _, p := range potentialVictims {
reprievePod(p)
}

return victims, numViolatingVictim, true, nil
@@ -1105,7 +1105,7 @@ func (g *genericScheduler) selectVictimsOnNode(
queue internalqueue.SchedulingQueue,
pdbs []*policy.PodDisruptionBudget,
) ([]*v1.Pod, int, bool) {
potentialVictims := util.SortableList{CompFunc: util.MoreImportantPod}
var potentialVictims []*v1.Pod

removePod := func(rp *v1.Pod) error {
if err := nodeInfo.RemovePod(rp); err != nil {
@@ -1140,7 +1140,7 @@ func (g *genericScheduler) selectVictimsOnNode(
podPriority := podutil.GetPodPriority(pod)
for _, p := range nodeInfo.Pods() {
if podutil.GetPodPriority(p) < podPriority {
potentialVictims.Items = append(potentialVictims.Items, p)
potentialVictims = append(potentialVictims, p)
if err := removePod(p); err != nil {
return nil, 0, false
}
@@ -1161,11 +1161,11 @@ func (g *genericScheduler) selectVictimsOnNode(
}
var victims []*v1.Pod
numViolatingVictim := 0
potentialVictims.Sort()
sort.Slice(potentialVictims, func(i, j int) bool { return util.MoreImportantPod(potentialVictims[i], potentialVictims[j]) })
// Try to reprieve as many pods as possible. We first try to reprieve the PDB
// violating victims and then other non-violating ones. In both cases, we start
// from the highest priority victims.
violatingVictims, nonViolatingVictims := filterPodsWithPDBViolation(potentialVictims.Items, pdbs)
violatingVictims, nonViolatingVictims := filterPodsWithPDBViolation(potentialVictims, pdbs)
reprievePod := func(p *v1.Pod) (bool, error) {
if err := addPod(p); err != nil {
return false, err
@@ -17,7 +17,6 @@ limitations under the License.
package util

import (
"sort"
"time"

v1 "k8s.io/api/core/v1"
@@ -92,39 +91,15 @@ func GetEarliestPodStartTime(victims *extenderv1.Victims) *metav1.Time {
return earliestPodStartTime
}

// SortableList is a list that implements sort.Interface.
type SortableList struct {
Items []interface{}
CompFunc lessFunc
}

var _ = sort.Interface(&SortableList{})

func (l *SortableList) Len() int { return len(l.Items) }

func (l *SortableList) Less(i, j int) bool {
return l.CompFunc(l.Items[i], l.Items[j])
}

func (l *SortableList) Swap(i, j int) {
l.Items[i], l.Items[j] = l.Items[j], l.Items[i]
}

// Sort sorts the items in the list using the given CompFunc. Item1 is placed
// before Item2 when CompFunc(Item1, Item2) returns true.
func (l *SortableList) Sort() {
sort.Sort(l)
}

// MoreImportantPod return true when priority of the first pod is higher than
// the second one. If two pods' priorities are equal, compare their StartTime.
// It takes arguments of the type "interface{}" to be used with SortableList,
// but expects those arguments to be *v1.Pod.
func MoreImportantPod(pod1, pod2 interface{}) bool {
p1 := podutil.GetPodPriority(pod1.(*v1.Pod))
p2 := podutil.GetPodPriority(pod2.(*v1.Pod))
func MoreImportantPod(pod1, pod2 *v1.Pod) bool {
p1 := podutil.GetPodPriority(pod1)
p2 := podutil.GetPodPriority(pod2)
if p1 != p2 {
return p1 > p2
}
return GetPodStartTime(pod1.(*v1.Pod)).Before(GetPodStartTime(pod2.(*v1.Pod)))
return GetPodStartTime(pod1).Before(GetPodStartTime(pod2))
}
@@ -19,6 +19,7 @@ package util
import (
"fmt"
"reflect"
"sort"
"testing"
"time"

@@ -32,10 +33,8 @@ import (
// TestSortableList tests SortableList by storing pods in the list and sorting
// them by their priority.
func TestSortableList(t *testing.T) {
higherPriority := func(pod1, pod2 interface{}) bool {
return pod.GetPodPriority(pod1.(*v1.Pod)) > pod.GetPodPriority(pod2.(*v1.Pod))
}
podList := SortableList{CompFunc: higherPriority}

var podList []*v1.Pod
// Add a few Pods with different priorities from lowest to highest priority.
for i := 0; i < 10; i++ {
var p = int32(i)
@@ -50,16 +49,19 @@ func TestSortableList(t *testing.T) {
Priority: &p,
},
}
podList.Items = append(podList.Items, pod)
podList = append(podList, pod)
}
podList.Sort()
if len(podList.Items) != 10 {
t.Errorf("expected length of list was 10, got: %v", len(podList.Items))
//sort.Slice(podList, func(i, j int) bool { return higherPriority(podList[i], podList[j]) })
sort.Slice(podList, func(i, j int) bool { return pod.GetPodPriority(podList[i]) > pod.GetPodPriority(podList[j]) })


if len(podList) != 10 {
t.Errorf("expected length of list was 10, got: %v", len(podList))
}
var prevPriority = int32(10)
for _, p := range podList.Items {
if *p.(*v1.Pod).Spec.Priority >= prevPriority {
t.Errorf("Pods are not soreted. Current pod pririty is %v, while previous one was %v.", *p.(*v1.Pod).Spec.Priority, prevPriority)
for _, p := range podList {
if *p.Spec.Priority >= prevPriority {
t.Errorf("Pods are not soreted. Current pod pririty is %v, while previous one was %v.", *p.Spec.Priority, prevPriority)
}
}
}

0 comments on commit 3b1d75e

Please sign in to comment.
You can’t perform that action at this time.