Minor selectHost optimization in scheduler

plugin/pkg/scheduler/generic_scheduler.go

@@ -20,7 +20,6 @@ import (
 	"bytes"
 	"fmt"
 	"math/rand"
-	"sort"
 	"strings"
 	"sync"
 
@@ -101,20 +100,31 @@ func (g *genericScheduler) Schedule(pod *api.Pod, nodeLister algorithm.NodeListe
 	return g.selectHost(priorityList)
 }
 
-// This method takes a prioritized list of nodes and sorts them in reverse order based on scores
-// and then picks one randomly from the nodes that had the highest score
+// selectHost takes a prioritized list of nodes and then picks one
+// randomly from the nodes that had the highest score.
 func (g *genericScheduler) selectHost(priorityList schedulerapi.HostPriorityList) (string, error) {
 	if len(priorityList) == 0 {
 		return "", fmt.Errorf("empty priorityList")
 	}
-	sort.Sort(sort.Reverse(priorityList))
 
-	hosts := getBestHosts(priorityList)
+	maxScore := priorityList[0].Score
+	// idx contains indices of elements with score == maxScore.
+	idx := []int{}
+
+	for i, entry := range priorityList {
+		if entry.Score > maxScore {
+			maxScore = entry.Score
+			idx = []int{i}
+		} else if entry.Score == maxScore {
+			idx = append(idx, i)
+		}
+	}
+
 	g.randomLock.Lock()
-	defer g.randomLock.Unlock()
+	ix := g.random.Int() % len(idx)
+	g.randomLock.Unlock()
 
-	ix := g.random.Int() % len(hosts)
-	return hosts[ix], nil
+	return priorityList[idx[ix]].Host, nil
 }
 
 // Filters the nodes to find the ones that fit based on the given predicate functions
@@ -258,18 +268,6 @@ func PrioritizeNodes(pod *api.Pod, machinesToPods map[string][]*api.Pod, podList
 	return result, nil
 }
 
-func getBestHosts(list schedulerapi.HostPriorityList) []string {
-	result := []string{}
-	for _, hostEntry := range list {
-		if hostEntry.Score == list[0].Score {
-			result = append(result, hostEntry.Host)
-		} else {
-			break
-		}
-	}
-	return result
-}
-
 // EqualPriority is a prioritizer function that gives an equal weight of one to all nodes
 func EqualPriority(_ *api.Pod, machinesToPods map[string][]*api.Pod, podLister algorithm.PodLister, nodeLister algorithm.NodeLister) (schedulerapi.HostPriorityList, error) {
 	nodes, err := nodeLister.List()

plugin/pkg/scheduler/generic_scheduler_test.go

@@ -166,14 +166,14 @@ func TestSelectHost(t *testing.T) {
 
 func TestGenericScheduler(t *testing.T) {
 	tests := []struct {
-		name         string
-		predicates   map[string]algorithm.FitPredicate
-		prioritizers []algorithm.PriorityConfig
-		nodes        []string
-		pod          *api.Pod
-		pods         []*api.Pod
-		expectedHost string
-		expectsErr   bool
+		name          string
+		predicates    map[string]algorithm.FitPredicate
+		prioritizers  []algorithm.PriorityConfig
+		nodes         []string
+		pod           *api.Pod
+		pods          []*api.Pod
+		expectedHosts sets.String
+		expectsErr    bool
 	}{
 		{
 			predicates:   map[string]algorithm.FitPredicate{"false": falsePredicate},
@@ -183,44 +183,43 @@ func TestGenericScheduler(t *testing.T) {
 			name:         "test 1",
 		},
 		{
-			predicates:   map[string]algorithm.FitPredicate{"true": truePredicate},
-			prioritizers: []algorithm.PriorityConfig{{Function: EqualPriority, Weight: 1}},
-			nodes:        []string{"machine1", "machine2"},
-			// Random choice between both, the rand seeded above with zero, chooses "machine1"
-			expectedHost: "machine1",
-			name:         "test 2",
+			predicates:    map[string]algorithm.FitPredicate{"true": truePredicate},
+			prioritizers:  []algorithm.PriorityConfig{{Function: EqualPriority, Weight: 1}},
+			nodes:         []string{"machine1", "machine2"},
+			expectedHosts: sets.NewString("machine1", "machine2"),
+			name:          "test 2",
 		},
 		{
 			// Fits on a machine where the pod ID matches the machine name
-			predicates:   map[string]algorithm.FitPredicate{"matches": matchesPredicate},
-			prioritizers: []algorithm.PriorityConfig{{Function: EqualPriority, Weight: 1}},
-			nodes:        []string{"machine1", "machine2"},
-			pod:          &api.Pod{ObjectMeta: api.ObjectMeta{Name: "machine2"}},
-			expectedHost: "machine2",
-			name:         "test 3",
+			predicates:    map[string]algorithm.FitPredicate{"matches": matchesPredicate},
+			prioritizers:  []algorithm.PriorityConfig{{Function: EqualPriority, Weight: 1}},
+			nodes:         []string{"machine1", "machine2"},
+			pod:           &api.Pod{ObjectMeta: api.ObjectMeta{Name: "machine2"}},
+			expectedHosts: sets.NewString("machine2"),
+			name:          "test 3",
 		},
 		{
-			predicates:   map[string]algorithm.FitPredicate{"true": truePredicate},
-			prioritizers: []algorithm.PriorityConfig{{Function: numericPriority, Weight: 1}},
-			nodes:        []string{"3", "2", "1"},
-			expectedHost: "3",
-			name:         "test 4",
+			predicates:    map[string]algorithm.FitPredicate{"true": truePredicate},
+			prioritizers:  []algorithm.PriorityConfig{{Function: numericPriority, Weight: 1}},
+			nodes:         []string{"3", "2", "1"},
+			expectedHosts: sets.NewString("3"),
+			name:          "test 4",
 		},
 		{
-			predicates:   map[string]algorithm.FitPredicate{"matches": matchesPredicate},
-			prioritizers: []algorithm.PriorityConfig{{Function: numericPriority, Weight: 1}},
-			nodes:        []string{"3", "2", "1"},
-			pod:          &api.Pod{ObjectMeta: api.ObjectMeta{Name: "2"}},
-			expectedHost: "2",
-			name:         "test 5",
+			predicates:    map[string]algorithm.FitPredicate{"matches": matchesPredicate},
+			prioritizers:  []algorithm.PriorityConfig{{Function: numericPriority, Weight: 1}},
+			nodes:         []string{"3", "2", "1"},
+			pod:           &api.Pod{ObjectMeta: api.ObjectMeta{Name: "2"}},
+			expectedHosts: sets.NewString("2"),
+			name:          "test 5",
 		},
 		{
-			predicates:   map[string]algorithm.FitPredicate{"true": truePredicate},
-			prioritizers: []algorithm.PriorityConfig{{Function: numericPriority, Weight: 1}, {Function: reverseNumericPriority, Weight: 2}},
-			nodes:        []string{"3", "2", "1"},
-			pod:          &api.Pod{ObjectMeta: api.ObjectMeta{Name: "2"}},
-			expectedHost: "1",
-			name:         "test 6",
+			predicates:    map[string]algorithm.FitPredicate{"true": truePredicate},
+			prioritizers:  []algorithm.PriorityConfig{{Function: numericPriority, Weight: 1}, {Function: reverseNumericPriority, Weight: 2}},
+			nodes:         []string{"3", "2", "1"},
+			pod:           &api.Pod{ObjectMeta: api.ObjectMeta{Name: "2"}},
+			expectedHosts: sets.NewString("1"),
+			name:          "test 6",
 		},
 		{
 			predicates:   map[string]algorithm.FitPredicate{"true": truePredicate, "false": falsePredicate},
@@ -266,8 +265,8 @@ func TestGenericScheduler(t *testing.T) {
 			if err != nil {
 				t.Errorf("Unexpected error: %v", err)
 			}
-			if test.expectedHost != machine {
-				t.Errorf("Failed : %s, Expected: %s, Saw: %s", test.name, test.expectedHost, machine)
+			if !test.expectedHosts.Has(machine) {
+				t.Errorf("Failed : %s, Expected: %s, Saw: %s", test.name, test.expectedHosts, machine)
 			}
 		}
 	}