From c7cba7370f7b7d67b888a1066fc1fc021b2d9f56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Orive?= Date: Mon, 12 Nov 2018 10:46:09 +0100 Subject: [PATCH] Scheduler internal NodeTree thread-safe NumNodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Adrián Orive --- pkg/scheduler/core/generic_scheduler.go | 2 +- pkg/scheduler/internal/cache/cache_test.go | 6 +++--- pkg/scheduler/internal/cache/node_tree.go | 13 ++++++++++--- pkg/scheduler/internal/cache/node_tree_test.go | 4 ++-- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/pkg/scheduler/core/generic_scheduler.go b/pkg/scheduler/core/generic_scheduler.go index 40effb6fd3..fedd43826c 100644 --- a/pkg/scheduler/core/generic_scheduler.go +++ b/pkg/scheduler/core/generic_scheduler.go @@ -390,7 +390,7 @@ func (g *genericScheduler) findNodesThatFit(pod *v1.Pod, nodes []*v1.Node) ([]*v if len(g.predicates) == 0 { filtered = nodes } else { - allNodes := int32(g.cache.NodeTree().NumNodes) + allNodes := int32(g.cache.NodeTree().NumNodes()) numNodesToFind := g.numFeasibleNodesToFind(allNodes) // Create filtered list with enough space to avoid growing it diff --git a/pkg/scheduler/internal/cache/cache_test.go b/pkg/scheduler/internal/cache/cache_test.go index 8186784968..72ba7eb82f 100644 --- a/pkg/scheduler/internal/cache/cache_test.go +++ b/pkg/scheduler/internal/cache/cache_test.go @@ -1056,7 +1056,7 @@ func TestNodeOperators(t *testing.T) { if !found { t.Errorf("Failed to find node %v in schedulerinternalcache.", 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) } @@ -1099,7 +1099,7 @@ func TestNodeOperators(t *testing.T) { t.Errorf("Failed to update node in schedulercache:\n got: %+v \nexpected: %+v", got, expected) } // 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) } @@ -1110,7 +1110,7 @@ func TestNodeOperators(t *testing.T) { } // Check nodeTree after remove. The node should be removed from the nodeTree even if there are // 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) } } diff --git a/pkg/scheduler/internal/cache/node_tree.go b/pkg/scheduler/internal/cache/node_tree.go index 80ce6d195f..f29024d0ed 100644 --- a/pkg/scheduler/internal/cache/node_tree.go +++ b/pkg/scheduler/internal/cache/node_tree.go @@ -32,7 +32,7 @@ type NodeTree struct { 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) zoneIndex int - NumNodes int + numNodes int mu sync.RWMutex } @@ -91,7 +91,7 @@ func (nt *NodeTree) addNode(n *v1.Node) { nt.tree[zone] = &nodeArray{nodes: []string{n.Name}, lastIndex: 0} } klog.V(5).Infof("Added node %v in group %v to NodeTree", n.Name, zone) - nt.NumNodes++ + nt.numNodes++ } // RemoveNode removes a node from the NodeTree. @@ -111,7 +111,7 @@ func (nt *NodeTree) removeNode(n *v1.Node) error { nt.removeZone(zone) } klog.V(5).Infof("Removed node %v in group %v from NodeTree", n.Name, zone) - nt.NumNodes-- + nt.numNodes-- return nil } } @@ -184,3 +184,10 @@ func (nt *NodeTree) Next() string { } } } + +// NumNodes returns the number of nodes. +func (nt *NodeTree) NumNodes() int { + nt.mu.RLock() + defer nt.mu.RUnlock() + return nt.numNodes +} diff --git a/pkg/scheduler/internal/cache/node_tree_test.go b/pkg/scheduler/internal/cache/node_tree_test.go index ee489870dd..9b4371589f 100644 --- a/pkg/scheduler/internal/cache/node_tree_test.go +++ b/pkg/scheduler/internal/cache/node_tree_test.go @@ -116,8 +116,8 @@ func verifyNodeTree(t *testing.T, nt *NodeTree, expectedTree map[string]*nodeArr for _, na := range expectedTree { expectedNumNodes += len(na.nodes) } - if nt.NumNodes != expectedNumNodes { - t.Errorf("unexpected NodeTree.numNodes. Expected: %v, Got: %v", expectedNumNodes, nt.NumNodes) + if numNodes := nt.NumNodes(); numNodes != expectedNumNodes { + t.Errorf("unexpected NodeTree.numNodes. Expected: %v, Got: %v", expectedNumNodes, numNodes) } if !reflect.DeepEqual(nt.tree, expectedTree) { t.Errorf("The node tree is not the same as expected. Expected: %v, Got: %v", expectedTree, nt.tree)