Revert "Faster scheduler"

This commit is contained in:
ahg-g 2019-06-06 22:34:18 -04:00 committed by GitHub
parent 333081e79c
commit ece3e3cdba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 44 additions and 67 deletions

View File

@ -167,7 +167,6 @@ type genericScheduler struct {
pvcLister corelisters.PersistentVolumeClaimLister pvcLister corelisters.PersistentVolumeClaimLister
pdbLister algorithm.PDBLister pdbLister algorithm.PDBLister
disablePreemption bool disablePreemption bool
lastIndex int
percentageOfNodesToScore int32 percentageOfNodesToScore int32
enableNonPreempting bool enableNonPreempting bool
} }
@ -462,8 +461,8 @@ func (g *genericScheduler) findNodesThatFit(pod *v1.Pod, nodes []*v1.Node) ([]*v
if len(g.predicates) == 0 { if len(g.predicates) == 0 {
filtered = nodes filtered = nodes
} else { } else {
allNodes := g.cache.NodeTree().AllNodes() allNodes := int32(g.cache.NodeTree().NumNodes())
numNodesToFind := g.numFeasibleNodesToFind(int32(len(allNodes))) numNodesToFind := g.numFeasibleNodesToFind(allNodes)
// Create filtered list with enough space to avoid growing it // Create filtered list with enough space to avoid growing it
// and allow assigning. // and allow assigning.
@ -479,12 +478,8 @@ func (g *genericScheduler) findNodesThatFit(pod *v1.Pod, nodes []*v1.Node) ([]*v
// We can use the same metadata producer for all nodes. // We can use the same metadata producer for all nodes.
meta := g.predicateMetaProducer(pod, g.nodeInfoSnapshot.NodeInfoMap) meta := g.predicateMetaProducer(pod, g.nodeInfoSnapshot.NodeInfoMap)
processedNodes := int32(0)
checkNode := func(i int) { checkNode := func(i int) {
// We check the nodes starting from where we left off in the previous scheduling cycle, nodeName := g.cache.NodeTree().Next()
// this is to make sure all nodes have the same chance of being examined across pods.
atomic.AddInt32(&processedNodes, 1)
nodeName := allNodes[(g.lastIndex+i)%len(allNodes)]
fits, failedPredicates, err := podFitsOnNode( fits, failedPredicates, err := podFitsOnNode(
pod, pod,
meta, meta,
@ -516,8 +511,7 @@ func (g *genericScheduler) findNodesThatFit(pod *v1.Pod, nodes []*v1.Node) ([]*v
// Stops searching for more nodes once the configured number of feasible nodes // Stops searching for more nodes once the configured number of feasible nodes
// are found. // are found.
workqueue.ParallelizeUntil(ctx, 16, len(allNodes), checkNode) workqueue.ParallelizeUntil(ctx, 16, int(allNodes), checkNode)
g.lastIndex = (g.lastIndex + int(processedNodes)) % len(allNodes)
filtered = filtered[:filteredLen] filtered = filtered[:filteredLen]
if len(errs) > 0 { if len(errs) > 0 {

View File

@ -1066,7 +1066,7 @@ func TestNodeOperators(t *testing.T) {
if !found { if !found {
t.Errorf("Failed to find node %v in internalcache.", node.Name) t.Errorf("Failed to find node %v in internalcache.", node.Name)
} }
if cache.nodeTree.NumNodes() != 1 || cache.nodeTree.next() != node.Name { if cache.nodeTree.NumNodes() != 1 || cache.nodeTree.Next() != node.Name {
t.Errorf("cache.nodeTree is not updated correctly after adding node: %v", node.Name) t.Errorf("cache.nodeTree is not updated correctly after adding node: %v", node.Name)
} }
@ -1109,7 +1109,7 @@ func TestNodeOperators(t *testing.T) {
t.Errorf("Failed to update node in schedulernodeinfo:\n got: %+v \nexpected: %+v", got, expected) t.Errorf("Failed to update node in schedulernodeinfo:\n got: %+v \nexpected: %+v", got, expected)
} }
// Check nodeTree after update // Check nodeTree after update
if cache.nodeTree.NumNodes() != 1 || cache.nodeTree.next() != node.Name { if cache.nodeTree.NumNodes() != 1 || cache.nodeTree.Next() != node.Name {
t.Errorf("unexpected cache.nodeTree after updating node: %v", node.Name) t.Errorf("unexpected cache.nodeTree after updating node: %v", node.Name)
} }
@ -1120,7 +1120,7 @@ func TestNodeOperators(t *testing.T) {
} }
// Check nodeTree after remove. The node should be removed from the nodeTree even if there are // Check nodeTree after remove. The node should be removed from the nodeTree even if there are
// still pods on it. // still pods on it.
if cache.nodeTree.NumNodes() != 0 || cache.nodeTree.next() != "" { if cache.nodeTree.NumNodes() != 0 || cache.nodeTree.Next() != "" {
t.Errorf("unexpected cache.nodeTree after removing node: %v", node.Name) t.Errorf("unexpected cache.nodeTree after removing node: %v", node.Name)
} }
} }

View File

@ -32,7 +32,6 @@ type NodeTree struct {
tree map[string]*nodeArray // a map from zone (region-zone) to an array of nodes in the zone. tree map[string]*nodeArray // a map from zone (region-zone) to an array of nodes in the zone.
zones []string // a list of all the zones in the tree (keys) zones []string // a list of all the zones in the tree (keys)
zoneIndex int zoneIndex int
allNodes []string
numNodes int numNodes int
mu sync.RWMutex mu sync.RWMutex
} }
@ -93,7 +92,6 @@ func (nt *NodeTree) addNode(n *v1.Node) {
} }
klog.V(5).Infof("Added node %v in group %v to NodeTree", n.Name, zone) klog.V(5).Infof("Added node %v in group %v to NodeTree", n.Name, zone)
nt.numNodes++ nt.numNodes++
nt.recomputeAllNodes()
} }
// RemoveNode removes a node from the NodeTree. // RemoveNode removes a node from the NodeTree.
@ -114,7 +112,6 @@ func (nt *NodeTree) removeNode(n *v1.Node) error {
} }
klog.V(5).Infof("Removed node %v in group %v from NodeTree", n.Name, zone) klog.V(5).Infof("Removed node %v in group %v from NodeTree", n.Name, zone)
nt.numNodes-- nt.numNodes--
nt.recomputeAllNodes()
return nil return nil
} }
} }
@ -162,7 +159,9 @@ func (nt *NodeTree) resetExhausted() {
// Next returns the name of the next node. NodeTree iterates over zones and in each zone iterates // Next returns the name of the next node. NodeTree iterates over zones and in each zone iterates
// over nodes in a round robin fashion. // over nodes in a round robin fashion.
func (nt *NodeTree) next() string { func (nt *NodeTree) Next() string {
nt.mu.Lock()
defer nt.mu.Unlock()
if len(nt.zones) == 0 { if len(nt.zones) == 0 {
return "" return ""
} }
@ -187,22 +186,6 @@ func (nt *NodeTree) next() string {
} }
} }
func (nt *NodeTree) recomputeAllNodes() {
nt.allNodes = make([]string, 0, nt.numNodes)
nt.resetExhausted()
for i := 0; i < nt.numNodes; i++ {
nt.allNodes = append(nt.allNodes, nt.next())
}
}
// AllNodes returns the list of nodes as they would be iterated by
// Next() method.
func (nt *NodeTree) AllNodes() []string {
nt.mu.RLock()
defer nt.mu.RUnlock()
return nt.allNodes
}
// NumNodes returns the number of nodes. // NumNodes returns the number of nodes.
func (nt *NodeTree) NumNodes() int { func (nt *NodeTree) NumNodes() int {
nt.mu.RLock() nt.mu.RLock()

View File

@ -140,28 +140,28 @@ func TestNodeTree_AddNode(t *testing.T) {
{ {
name: "single node no labels", name: "single node no labels",
nodesToAdd: allNodes[:1], nodesToAdd: allNodes[:1],
expectedTree: map[string]*nodeArray{"": {[]string{"node-0"}, 1}}, expectedTree: map[string]*nodeArray{"": {[]string{"node-0"}, 0}},
}, },
{ {
name: "mix of nodes with and without proper labels", name: "mix of nodes with and without proper labels",
nodesToAdd: allNodes[:4], nodesToAdd: allNodes[:4],
expectedTree: map[string]*nodeArray{ expectedTree: map[string]*nodeArray{
"": {[]string{"node-0"}, 1}, "": {[]string{"node-0"}, 0},
"region-1:\x00:": {[]string{"node-1"}, 1}, "region-1:\x00:": {[]string{"node-1"}, 0},
":\x00:zone-2": {[]string{"node-2"}, 1}, ":\x00:zone-2": {[]string{"node-2"}, 0},
"region-1:\x00:zone-2": {[]string{"node-3"}, 1}, "region-1:\x00:zone-2": {[]string{"node-3"}, 0},
}, },
}, },
{ {
name: "mix of nodes with and without proper labels and some zones with multiple nodes", name: "mix of nodes with and without proper labels and some zones with multiple nodes",
nodesToAdd: allNodes[:7], nodesToAdd: allNodes[:7],
expectedTree: map[string]*nodeArray{ expectedTree: map[string]*nodeArray{
"": {[]string{"node-0"}, 1}, "": {[]string{"node-0"}, 0},
"region-1:\x00:": {[]string{"node-1"}, 1}, "region-1:\x00:": {[]string{"node-1"}, 0},
":\x00:zone-2": {[]string{"node-2"}, 1}, ":\x00:zone-2": {[]string{"node-2"}, 0},
"region-1:\x00:zone-2": {[]string{"node-3", "node-4"}, 2}, "region-1:\x00:zone-2": {[]string{"node-3", "node-4"}, 0},
"region-1:\x00:zone-3": {[]string{"node-5"}, 1}, "region-1:\x00:zone-3": {[]string{"node-5"}, 0},
"region-2:\x00:zone-2": {[]string{"node-6"}, 1}, "region-2:\x00:zone-2": {[]string{"node-6"}, 0},
}, },
}, },
} }
@ -190,11 +190,11 @@ func TestNodeTree_RemoveNode(t *testing.T) {
existingNodes: allNodes[:7], existingNodes: allNodes[:7],
nodesToRemove: allNodes[:1], nodesToRemove: allNodes[:1],
expectedTree: map[string]*nodeArray{ expectedTree: map[string]*nodeArray{
"region-1:\x00:": {[]string{"node-1"}, 1}, "region-1:\x00:": {[]string{"node-1"}, 0},
":\x00:zone-2": {[]string{"node-2"}, 1}, ":\x00:zone-2": {[]string{"node-2"}, 0},
"region-1:\x00:zone-2": {[]string{"node-3", "node-4"}, 2}, "region-1:\x00:zone-2": {[]string{"node-3", "node-4"}, 0},
"region-1:\x00:zone-3": {[]string{"node-5"}, 1}, "region-1:\x00:zone-3": {[]string{"node-5"}, 0},
"region-2:\x00:zone-2": {[]string{"node-6"}, 1}, "region-2:\x00:zone-2": {[]string{"node-6"}, 0},
}, },
}, },
{ {
@ -202,10 +202,10 @@ func TestNodeTree_RemoveNode(t *testing.T) {
existingNodes: allNodes[:7], existingNodes: allNodes[:7],
nodesToRemove: allNodes[1:4], nodesToRemove: allNodes[1:4],
expectedTree: map[string]*nodeArray{ expectedTree: map[string]*nodeArray{
"": {[]string{"node-0"}, 1}, "": {[]string{"node-0"}, 0},
"region-1:\x00:zone-2": {[]string{"node-4"}, 1}, "region-1:\x00:zone-2": {[]string{"node-4"}, 0},
"region-1:\x00:zone-3": {[]string{"node-5"}, 1}, "region-1:\x00:zone-3": {[]string{"node-5"}, 0},
"region-2:\x00:zone-2": {[]string{"node-6"}, 1}, "region-2:\x00:zone-2": {[]string{"node-6"}, 0},
}, },
}, },
{ {
@ -257,11 +257,11 @@ func TestNodeTree_UpdateNode(t *testing.T) {
}, },
}, },
expectedTree: map[string]*nodeArray{ expectedTree: map[string]*nodeArray{
"region-1:\x00:": {[]string{"node-1"}, 1}, "region-1:\x00:": {[]string{"node-1"}, 0},
":\x00:zone-2": {[]string{"node-2"}, 1}, ":\x00:zone-2": {[]string{"node-2"}, 0},
"region-1:\x00:zone-2": {[]string{"node-3", "node-4", "node-0"}, 3}, "region-1:\x00:zone-2": {[]string{"node-3", "node-4", "node-0"}, 0},
"region-1:\x00:zone-3": {[]string{"node-5"}, 1}, "region-1:\x00:zone-3": {[]string{"node-5"}, 0},
"region-2:\x00:zone-2": {[]string{"node-6"}, 1}, "region-2:\x00:zone-2": {[]string{"node-6"}, 0},
}, },
}, },
{ {
@ -277,7 +277,7 @@ func TestNodeTree_UpdateNode(t *testing.T) {
}, },
}, },
expectedTree: map[string]*nodeArray{ expectedTree: map[string]*nodeArray{
"region-1:\x00:zone-2": {[]string{"node-0"}, 1}, "region-1:\x00:zone-2": {[]string{"node-0"}, 0},
}, },
}, },
{ {
@ -293,8 +293,8 @@ func TestNodeTree_UpdateNode(t *testing.T) {
}, },
}, },
expectedTree: map[string]*nodeArray{ expectedTree: map[string]*nodeArray{
"": {[]string{"node-0"}, 1}, "": {[]string{"node-0"}, 0},
"region-1:\x00:zone-2": {[]string{"node-new"}, 1}, "region-1:\x00:zone-2": {[]string{"node-new"}, 0},
}, },
}, },
} }
@ -322,7 +322,7 @@ func TestNodeTree_Next(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
nodesToAdd []*v1.Node nodesToAdd []*v1.Node
numRuns int // number of times to run next() numRuns int // number of times to run Next()
expectedOutput []string expectedOutput []string
}{ }{
{ {
@ -357,7 +357,7 @@ func TestNodeTree_Next(t *testing.T) {
var output []string var output []string
for i := 0; i < test.numRuns; i++ { for i := 0; i < test.numRuns; i++ {
output = append(output, nt.next()) output = append(output, nt.Next())
} }
if !reflect.DeepEqual(output, test.expectedOutput) { if !reflect.DeepEqual(output, test.expectedOutput) {
t.Errorf("unexpected output. Expected: %v, Got: %v", test.expectedOutput, output) t.Errorf("unexpected output. Expected: %v, Got: %v", test.expectedOutput, output)
@ -399,15 +399,15 @@ func TestNodeTreeMultiOperations(t *testing.T) {
name: "add more nodes to an exhausted zone", name: "add more nodes to an exhausted zone",
nodesToAdd: append(allNodes[4:9], allNodes[3]), nodesToAdd: append(allNodes[4:9], allNodes[3]),
nodesToRemove: nil, nodesToRemove: nil,
operations: []string{"add", "add", "add", "add", "add", "next", "next", "next", "next", "next", "next", "next"}, operations: []string{"add", "add", "add", "add", "add", "next", "next", "next", "next", "add", "next", "next", "next"},
expectedOutput: []string{"node-4", "node-5", "node-6", "node-7", "node-8", "node-4", "node-5"}, expectedOutput: []string{"node-4", "node-5", "node-6", "node-7", "node-3", "node-8", "node-4"},
}, },
{ {
name: "remove zone and add new to ensure exhausted is reset correctly", name: "remove zone and add new to ensure exhausted is reset correctly",
nodesToAdd: append(allNodes[3:5], allNodes[6:8]...), nodesToAdd: append(allNodes[3:5], allNodes[6:8]...),
nodesToRemove: allNodes[3:5], nodesToRemove: allNodes[3:5],
operations: []string{"add", "add", "next", "next", "remove", "add", "add", "next", "next", "remove", "next", "next"}, operations: []string{"add", "add", "next", "next", "remove", "add", "add", "next", "next", "remove", "next", "next"},
expectedOutput: []string{"node-3", "node-4", "node-4", "node-6", "node-6", "node-7"}, expectedOutput: []string{"node-3", "node-4", "node-6", "node-7", "node-6", "node-7"},
}, },
} }
@ -434,7 +434,7 @@ func TestNodeTreeMultiOperations(t *testing.T) {
removeIndex++ removeIndex++
} }
case "next": case "next":
output = append(output, nt.next()) output = append(output, nt.Next())
default: default:
t.Errorf("unknow operation: %v", op) t.Errorf("unknow operation: %v", op)
} }