From b4c7d190cd9a6770ea39c6b99a799e45aed1c5ce Mon Sep 17 00:00:00 2001 From: Ahmad Diaa Date: Fri, 17 Aug 2018 21:30:33 +0200 Subject: [PATCH] using set instead of lists for topologyPairsMaps attributes --- .../algorithm/predicates/metadata.go | 71 +++++++++---------- .../algorithm/predicates/metadata_test.go | 51 +++---------- .../algorithm/predicates/predicates.go | 18 ++--- 3 files changed, 52 insertions(+), 88 deletions(-) diff --git a/pkg/scheduler/algorithm/predicates/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index 0a87a17a689..163ad90ba2c 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/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 - } - } - if !found { - topologyPairsMaps.topologyPairToPods[pair] = append(topologyPairsMaps.topologyPairToPods[pair], pod) - topologyPairsMaps.podToTopologyPairs[schedutil.GetPodFullName(pod)] = append(topologyPairsMaps.podToTopologyPairs[schedutil.GetPodFullName(pod)], pair) - } +// 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) 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) addTopologyPair(pair topologyPair, pod *v1.Pod) { + podFullName := schedutil.GetPodFullName(pod) + if topologyPairsMaps.topologyPairToPods[pair] == nil { + topologyPairsMaps.topologyPairToPods[pair] = make(map[*v1.Pod]struct{}) + } + 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(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...) diff --git a/pkg/scheduler/algorithm/predicates/metadata_test.go b/pkg/scheduler/algorithm/predicates/metadata_test.go index 0fc116e8a1c..04da8ed5c0c 100644 --- a/pkg/scheduler/algorithm/predicates/metadata_test.go +++ b/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{}{}, }, }, }, diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index 103d5ea7f55..c919282c837 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/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 }