using set instead of lists for topologyPairsMaps attributes

pkg/scheduler/algorithm/predicates/metadata.go

@@ -51,11 +51,15 @@ type matchingPodAntiAffinityTerm struct {
 	node *v1.Node
 }
 
+type podSet map[*v1.Pod]struct{}
+
+type topologyPairSet map[topologyPair]struct{}
+
 // topologyPairsMaps keeps topologyPairToAntiAffinityPods and antiAffinityPodToTopologyPairs in sync
 // as they are the inverse of each others.
 type topologyPairsMaps struct {
-	topologyPairToPods map[topologyPair][]*v1.Pod
-	podToTopologyPairs map[string][]topologyPair
+	topologyPairToPods map[topologyPair]podSet
+	podToTopologyPairs map[string]topologyPairSet
 }
 
 // NOTE: When new fields are added/removed or logic is changed, please make sure that
@@ -151,44 +155,40 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf
 	return predicateMetadata
 }
 
-func (topologyPairsMaps *topologyPairsMaps) AddTopologyPair(pair topologyPair, pod *v1.Pod) {
-	found := false
-	for _, existingPod := range topologyPairsMaps.topologyPairToPods[pair] {
-		if existingPod == pod {
-			found = true
-			break
-		}
+// returns a pointer to a new topologyPairsMaps
+func newTopologyPairsMaps() *topologyPairsMaps {
+	return &topologyPairsMaps{topologyPairToPods: make(map[topologyPair]podSet),
+		podToTopologyPairs: make(map[string]topologyPairSet)}
+}
+
+func (topologyPairsMaps *topologyPairsMaps) addTopologyPair(pair topologyPair, pod *v1.Pod) {
+	podFullName := schedutil.GetPodFullName(pod)
+	if topologyPairsMaps.topologyPairToPods[pair] == nil {
+		topologyPairsMaps.topologyPairToPods[pair] = make(map[*v1.Pod]struct{})
 	}
-	if !found {
-		topologyPairsMaps.topologyPairToPods[pair] = append(topologyPairsMaps.topologyPairToPods[pair], pod)
-		topologyPairsMaps.podToTopologyPairs[schedutil.GetPodFullName(pod)] = append(topologyPairsMaps.podToTopologyPairs[schedutil.GetPodFullName(pod)], pair)
+	topologyPairsMaps.topologyPairToPods[pair][pod] = struct{}{}
+	if topologyPairsMaps.podToTopologyPairs[podFullName] == nil {
+		topologyPairsMaps.podToTopologyPairs[podFullName] = make(map[topologyPair]struct{})
 	}
+	topologyPairsMaps.podToTopologyPairs[podFullName][pair] = struct{}{}
 }
 
-func (topologyPairsMaps *topologyPairsMaps) RemovePod(podName string) {
-	for _, pair := range topologyPairsMaps.podToTopologyPairs[podName] {
-		for index, pod := range topologyPairsMaps.topologyPairToPods[pair] {
-			if schedutil.GetPodFullName(pod) == podName {
-				podsList := topologyPairsMaps.topologyPairToPods[pair]
-				podsList[index] = podsList[len(podsList)-1]
-				if len(podsList) <= 1 {
-					delete(topologyPairsMaps.topologyPairToPods, pair)
-				} else {
-					topologyPairsMaps.topologyPairToPods[pair] = podsList[:len(podsList)-1]
-				}
-				break
-			}
+func (topologyPairsMaps *topologyPairsMaps) removePod(deletedPod *v1.Pod) {
+	deletedPodFullName := schedutil.GetPodFullName(deletedPod)
+	for pair := range topologyPairsMaps.podToTopologyPairs[deletedPodFullName] {
+		delete(topologyPairsMaps.topologyPairToPods[pair], deletedPod)
+		if len(topologyPairsMaps.topologyPairToPods[pair]) == 0 {
+			delete(topologyPairsMaps.topologyPairToPods, pair)
 		}
 	}
-	delete(topologyPairsMaps.podToTopologyPairs, podName)
+	delete(topologyPairsMaps.podToTopologyPairs, deletedPodFullName)
 }
 
 func (topologyPairsMaps *topologyPairsMaps) appendMaps(toAppend *topologyPairsMaps) {
-	for pod, pairs := range toAppend.podToTopologyPairs {
-		topologyPairsMaps.podToTopologyPairs[pod] = append(topologyPairsMaps.podToTopologyPairs[pod], pairs...)
-	}
-	for pair, pods := range toAppend.topologyPairToPods {
-		topologyPairsMaps.topologyPairToPods[pair] = append(topologyPairsMaps.topologyPairToPods[pair], pods...)
+	for pair := range toAppend.topologyPairToPods {
+		for pod := range toAppend.topologyPairToPods[pair] {
+			topologyPairsMaps.addTopologyPair(pair, pod)
+		}
 	}
 }
 
@@ -199,7 +199,7 @@ func (meta *predicateMetadata) RemovePod(deletedPod *v1.Pod) error {
 	if deletedPodFullName == schedutil.GetPodFullName(meta.pod) {
 		return fmt.Errorf("deletedPod and meta.pod must not be the same")
 	}
-	meta.topologyPairsAntiAffinityPodsMap.RemovePod(deletedPodFullName)
+	meta.topologyPairsAntiAffinityPodsMap.removePod(deletedPod)
 	// Delete pod from the matching affinity or anti-affinity pods if exists.
 	affinity := meta.pod.Spec.Affinity
 	podNodeName := deletedPod.Spec.NodeName
@@ -324,8 +324,7 @@ func (meta *predicateMetadata) ShallowCopy() algorithm.PredicateMetadata {
 	for k, v := range meta.nodeNameToMatchingAntiAffinityPods {
 		newPredMeta.nodeNameToMatchingAntiAffinityPods[k] = append([]*v1.Pod(nil), v...)
 	}
-	newPredMeta.topologyPairsAntiAffinityPodsMap = &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod),
-		podToTopologyPairs: make(map[string][]topologyPair)}
+	newPredMeta.topologyPairsAntiAffinityPodsMap = newTopologyPairsMaps()
 	newPredMeta.topologyPairsAntiAffinityPodsMap.appendMaps(meta.topologyPairsAntiAffinityPodsMap)
 	newPredMeta.serviceAffinityMatchingPodServices = append([]*v1.Service(nil),
 		meta.serviceAffinityMatchingPodServices...)

pkg/scheduler/algorithm/predicates/metadata_test.go

@@ -28,33 +28,6 @@ import (
 	schedulertesting "k8s.io/kubernetes/pkg/scheduler/testing"
 )
 
-// sortableTopologyPairs lets us sort topology pairs
-type sortableTopologyPairs []topologyPair
-
-// Less establishes some ordering between two topologyPairs for sorting.
-func (s sortableTopologyPairs) Less(i, j int) bool {
-	t1, t2 := s[i], s[j]
-	return t1.key < t2.key || (t1.key == t2.key && t1.value < t2.value)
-}
-func (s sortableTopologyPairs) Len() int      { return len(s) }
-func (s sortableTopologyPairs) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
-
-var _ = sort.Interface(sortableTopologyPairs{})
-
-func sortTopologyPairs(pairs map[string][]topologyPair) {
-	for k, v := range pairs {
-		sortableTerms := sortableTopologyPairs(v)
-		sort.Sort(sortableTerms)
-		pairs[k] = sortableTerms
-	}
-}
-func sortTopologyPairPods(np map[topologyPair][]*v1.Pod) {
-	for _, pl := range np {
-		sortablePods := sortablePods(pl)
-		sort.Sort(sortablePods)
-	}
-}
-
 // sortablePods lets us to sort pods.
 type sortablePods []*v1.Pod
 
@@ -114,17 +87,13 @@ func predicateMetadataEquivalent(meta1, meta2 *predicateMetadata) error {
 	if !reflect.DeepEqual(meta1.nodeNameToMatchingAntiAffinityPods, meta2.nodeNameToMatchingAntiAffinityPods) {
 		return fmt.Errorf("nodeNameToMatchingAntiAffinityPods are not euqal")
 	}
-	sortTopologyPairs(meta1.topologyPairsAntiAffinityPodsMap.podToTopologyPairs)
-	sortTopologyPairs(meta2.topologyPairsAntiAffinityPodsMap.podToTopologyPairs)
 	if !reflect.DeepEqual(meta1.topologyPairsAntiAffinityPodsMap.podToTopologyPairs,
 		meta2.topologyPairsAntiAffinityPodsMap.podToTopologyPairs) {
-		return fmt.Errorf("topologyPairsAntiAffinityPodsMap.antiAffinityPodToTopologyPairs are not equal")
+		return fmt.Errorf("topologyPairsAntiAffinityPodsMap.podToTopologyPairs are not equal")
 	}
-	sortTopologyPairPods(meta1.topologyPairsAntiAffinityPodsMap.topologyPairToPods)
-	sortTopologyPairPods(meta2.topologyPairsAntiAffinityPodsMap.topologyPairToPods)
 	if !reflect.DeepEqual(meta1.topologyPairsAntiAffinityPodsMap.topologyPairToPods,
 		meta2.topologyPairsAntiAffinityPodsMap.topologyPairToPods) {
-		return fmt.Errorf("topologyPairsAntiAffinityPodsMap.topologyPairToAntiAffinityPods are not equal")
+		return fmt.Errorf("topologyPairsAntiAffinityPodsMap.topologyPairToPods are not equal")
 	}
 	if meta1.serviceAffinityInUse {
 		sortablePods1 := sortablePods(meta1.serviceAffinityMatchingPodList)
@@ -464,24 +433,24 @@ func TestPredicateMetadata_ShallowCopy(t *testing.T) {
 			},
 		},
 		topologyPairsAntiAffinityPodsMap: &topologyPairsMaps{
-			topologyPairToPods: map[topologyPair][]*v1.Pod{
+			topologyPairToPods: map[topologyPair]podSet{
 				{key: "name", value: "machine1"}: {
 					&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p2", Labels: selector1},
 						Spec: v1.PodSpec{NodeName: "nodeC"},
-					},
+					}: struct{}{},
 				},
 				{key: "name", value: "machine2"}: {
 					&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1},
 						Spec: v1.PodSpec{NodeName: "nodeA"},
-					},
+					}: struct{}{},
 				},
 			},
-			podToTopologyPairs: map[string][]topologyPair{
-				"p2": {
-					topologyPair{key: "name", value: "machine1"},
+			podToTopologyPairs: map[string]topologyPairSet{
+				"p2_": {
+					topologyPair{key: "name", value: "machine1"}: struct{}{},
 				},
-				"p1": {
-					topologyPair{key: "name", value: "machine2"},
+				"p1_": {
+					topologyPair{key: "name", value: "machine2"}: struct{}{},
 				},
 			},
 		},

pkg/scheduler/algorithm/predicates/predicates.go

@@ -1255,8 +1255,7 @@ func getMatchingTopologyPairs(pod *v1.Pod, nodeInfoMap map[string]*schedulercach
 	var lock sync.Mutex
 	var firstError error
 
-	topologyMaps := &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod),
-		podToTopologyPairs: make(map[string][]topologyPair)}
+	topologyMaps := newTopologyPairsMaps()
 
 	appendTopologyPairsMaps := func(toAppend *topologyPairsMaps) {
 		lock.Lock()
@@ -1278,8 +1277,7 @@ func getMatchingTopologyPairs(pod *v1.Pod, nodeInfoMap map[string]*schedulercach
 			catchError(fmt.Errorf("node not found"))
 			return
 		}
-		nodeTopologyMaps := &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod),
-			podToTopologyPairs: make(map[string][]topologyPair)}
+		nodeTopologyMaps := newTopologyPairsMaps()
 		for _, existingPod := range nodeInfo.PodsWithAffinity() {
 			affinity := existingPod.Spec.Affinity
 			if affinity == nil {
@@ -1295,7 +1293,7 @@ func getMatchingTopologyPairs(pod *v1.Pod, nodeInfoMap map[string]*schedulercach
 				if priorityutil.PodMatchesTermsNamespaceAndSelector(pod, namespaces, selector) {
 					if topologyValue, ok := node.Labels[term.TopologyKey]; ok {
 						pair := topologyPair{key: term.TopologyKey, value: topologyValue}
-						nodeTopologyMaps.AddTopologyPair(pair, existingPod)
+						nodeTopologyMaps.addTopologyPair(pair, existingPod)
 					}
 				}
 			}
@@ -1309,8 +1307,7 @@ func getMatchingTopologyPairs(pod *v1.Pod, nodeInfoMap map[string]*schedulercach
 }
 
 func getMatchingTopologyPairsOfExistingPod(newPod *v1.Pod, existingPod *v1.Pod, node *v1.Node) (*topologyPairsMaps, error) {
-	topologyMaps := &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod),
-		podToTopologyPairs: make(map[string][]topologyPair)}
+	topologyMaps := newTopologyPairsMaps()
 	affinity := existingPod.Spec.Affinity
 	if affinity != nil && affinity.PodAntiAffinity != nil {
 		for _, term := range GetPodAntiAffinityTerms(affinity.PodAntiAffinity) {
@@ -1322,7 +1319,7 @@ func getMatchingTopologyPairsOfExistingPod(newPod *v1.Pod, existingPod *v1.Pod,
 			if priorityutil.PodMatchesTermsNamespaceAndSelector(newPod, namespaces, selector) {
 				if topologyValue, ok := node.Labels[term.TopologyKey]; ok {
 					pair := topologyPair{key: term.TopologyKey, value: topologyValue}
-					topologyMaps.AddTopologyPair(pair, existingPod)
+					topologyMaps.addTopologyPair(pair, existingPod)
 				}
 			}
 		}
@@ -1330,8 +1327,7 @@ func getMatchingTopologyPairsOfExistingPod(newPod *v1.Pod, existingPod *v1.Pod,
 	return topologyMaps, nil
 }
 func (c *PodAffinityChecker) getMatchingAntiAffinityTopologyPairs(pod *v1.Pod, allPods []*v1.Pod) (*topologyPairsMaps, error) {
-	topologyMaps := &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod),
-		podToTopologyPairs: make(map[string][]topologyPair)}
+	topologyMaps := newTopologyPairsMaps()
 
 	for _, existingPod := range allPods {
 		affinity := existingPod.Spec.Affinity
@@ -1383,7 +1379,7 @@ func (c *PodAffinityChecker) satisfiesExistingPodsAntiAffinity(pod *v1.Pod, meta
 	// Iterate over topology pairs to get any of the pods being affected by
 	// the scheduled pod anti-affinity rules
 	for topologyKey, topologyValue := range node.Labels {
-		if _, ok := topologyMaps.topologyPairToPods[topologyPair{key: topologyKey, value: topologyValue}]; ok {
+		if topologyMaps.topologyPairToPods[topologyPair{key: topologyKey, value: topologyValue}] != nil {
 			glog.V(10).Infof("Cannot schedule pod %+v onto node %v", podName(pod), node.Name)
 			return ErrExistingPodsAntiAffinityRulesNotMatch, nil
 		}