Merge pull request #500 from PiotrProkop/add-numa-distance

Make LeastNUMANodes scoringStrategy consider distance between zones

Author: k8s-ci-robot

pkg/noderesourcetopology/least_numa.go

@@ -31,21 +31,37 @@ import (
 	"sigs.k8s.io/scheduler-plugins/pkg/util"
 )
 
+const (
+	// 255 is max value as defined by ACPI SLIT(System Locality Information Tables), which means unknown/undefined
+	maxDistanceValue = 255
+)
+
 func leastNUMAContainerScopeScore(pod *v1.Pod, zones topologyv1alpha2.ZoneList) (int64, *framework.Status) {
 	nodes := createNUMANodeList(zones)
 	qos := v1qos.GetPodQOS(pod)
 
 	maxNUMANodesCount := 0
+	allContainersMinAvgDistance := true
 	// the order how TopologyManager asks for hint is important so doing it in the same order
 	// https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/cm/topologymanager/scope_container.go#L52
 	for _, container := range append(pod.Spec.InitContainers, pod.Spec.Containers...) {
+		// if a container requests only non NUMA just continue
+		if onlyNonNUMAResources(nodes, container.Resources.Requests) {
+			continue
+		}
 		identifier := fmt.Sprintf("%s/%s/%s", pod.Namespace, pod.Name, container.Name)
-		numaNodes := numaNodesRequired(identifier, qos, nodes, container.Resources.Requests)
+		numaNodes, isMinAvgDistance := numaNodesRequired(identifier, qos, nodes, container.Resources.Requests)
 		// container's resources can't fit onto node, return MinNodeScore for whole pod
 		if numaNodes == nil {
+			// score plugin should be running after resource filter plugin so we should always find sufficient amount of NUMA nodes
+			klog.Warningf("cannot calculate how many NUMA nodes are required for: %s", identifier)
 			return framework.MinNodeScore, nil
 		}
 
+		if !isMinAvgDistance {
+			allContainersMinAvgDistance = false
+		}
+
 		if numaNodes.Count() > maxNUMANodesCount {
 			maxNUMANodesCount = numaNodes.Count()
 		}
@@ -55,7 +71,11 @@ func leastNUMAContainerScopeScore(pod *v1.Pod, zones topologyv1alpha2.ZoneList)
 		subtractFromNUMAs(container.Resources.Requests, nodes, numaNodes.GetBits()...)
 	}
 
-	return normalizeScore(maxNUMANodesCount), nil
+	if maxNUMANodesCount == 0 {
+		return framework.MaxNodeScore, nil
+	}
+
+	return normalizeScore(maxNUMANodesCount, allContainersMinAvgDistance), nil
 }
 
 func leastNUMAPodScopeScore(pod *v1.Pod, zones topologyv1alpha2.ZoneList) (int64, *framework.Status) {
@@ -65,77 +85,69 @@ func leastNUMAPodScopeScore(pod *v1.Pod, zones topologyv1alpha2.ZoneList) (int64
 	identifier := fmt.Sprintf("%s/%s", pod.Namespace, pod.Name)
 
 	resources := util.GetPodEffectiveRequest(pod)
+	// if a pod requests only non NUMA resources return max score
+	if onlyNonNUMAResources(nodes, resources) {
+		return framework.MaxNodeScore, nil
+	}
 
-	numaNodes := numaNodesRequired(identifier, qos, nodes, resources)
+	numaNodes, isMinAvgDistance := numaNodesRequired(identifier, qos, nodes, resources)
 	// pod's resources can't fit onto node, return MinNodeScore
 	if numaNodes == nil {
+		// score plugin should be running after resource filter plugin so we should always find sufficient amount of NUMA nodes
+		klog.Warningf("cannot calculate how many NUMA nodes are required for: %s", identifier)
 		return framework.MinNodeScore, nil
 	}
 
-	return normalizeScore(numaNodes.Count()), nil
+	return normalizeScore(numaNodes.Count(), isMinAvgDistance), nil
 }
 
-func normalizeScore(numaNodes int) int64 {
-	numaScore := framework.MaxNodeScore / highestNUMAID
-	return framework.MaxNodeScore - int64(numaNodes)*numaScore
+func normalizeScore(numaNodesCount int, isMinAvgDistance bool) int64 {
+	numaNodeScore := framework.MaxNodeScore / highestNUMAID
+	score := framework.MaxNodeScore - int64(numaNodesCount)*numaNodeScore
+	if isMinAvgDistance {
+		// if distance between NUMA domains is optimal add half of numaNodeScore to make this node more favorable
+		return score + numaNodeScore/2
+	}
+
+	return score
 }
 
-// numaNodesRequired returns bitmask with minimal NUMA nodes required to run given resources
-// or nil when resources can't be fitted onto the node
-func numaNodesRequired(identifier string, qos v1.PodQOSClass, numaNodes NUMANodeList, resources v1.ResourceList) bitmask.BitMask {
-	combinationBitmask := bitmask.NewEmptyBitMask()
-	// we will generate combination of numa nodes from len = 1 to the number of numa nodes present on the machine
-	for i := 1; i <= len(numaNodes); i++ {
-		// generate combinations of len i
-		numaNodesCombination := combin.Combinations(len(numaNodes), i)
-		// iterate over combinations for given i
-		for _, combination := range numaNodesCombination {
-			// accumulate resources for given combination
-			combinationResources := combineResources(numaNodes, combination)
-
-			resourcesFit := true
-			onlyNonNUMAResources := true
-			for resource, quantity := range resources {
-				if quantity.IsZero() {
-					// why bother? everything's fine from the perspective of this resource
-					klog.V(4).InfoS("ignoring zero-qty resource request", "identifier", identifier, "resource", resource)
-					continue
-				}
-
-				combinationQuantity, ok := combinationResources[resource]
-				if !ok {
-					// non NUMA resource continue
-					continue
-				}
-
-				// there can be a situation where container/pod requests only non NUMA resources
-				onlyNonNUMAResources = false
-
-				if !isResourceSetSuitable(qos, resource, quantity, combinationQuantity) {
-					resourcesFit = false
-					break
-				}
+func minAvgDistanceInCombinations(numaNodes NUMANodeList, numaNodesCombination [][]int) float32 {
+	// max distance for NUMA node
+	var minDistance float32 = maxDistanceValue
 
+	for _, combination := range numaNodesCombination {
+		avgDistance := nodesAvgDistance(numaNodes, combination...)
+		if avgDistance < minDistance {
+			minDistance = avgDistance
+		}
+	}
+
+	return minDistance
+}
+
+func nodesAvgDistance(numaNodes NUMANodeList, nodes ...int) float32 {
+	if len(nodes) == 0 {
+		return maxDistanceValue
+	}
+
+	var (
+		accu int
+	)
+
+	for _, node1 := range nodes {
+		for _, node2 := range nodes {
+			cost, ok := numaNodes[node1].Costs[node2]
+			// we couldn't read Costs assign maxDistanceValue
+			if !ok {
+				klog.Warningf("cannot retrieve Costs information for node : %s", node1)
+				cost = maxDistanceValue
 			}
-			// if resources can be fit on given combination, just return the number of numa nodes requires to fit them
-			// according to TopologyManager if both masks are the same size pick the one that has less bits set
-			// https://github.com/kubernetes/kubernetes/blob/3e26e104bdf9d0dc3c4046d6350b93557c67f3f4/pkg/kubelet/cm/topologymanager/bitmask/bitmask.go#L146
-			// combin.Combinations is generating combinations in an order from the smallest to highest value
-			if resourcesFit {
-				// if a container/pod requests only non NUMA resources return empty bitmask and score of 0
-				if onlyNonNUMAResources {
-					return combinationBitmask
-				}
-
-				combinationBitmask.Add(combination...)
-				return combinationBitmask
-			}
+			accu += cost
 		}
 	}
 
-	// score plugin should be running after resource filter plugin so we should always find sufficient amount of NUMA nodes
-	klog.Warningf("cannot calculate how many NUMA nodes are required for: %s", identifier)
-	return nil
+	return float32(accu) / float32(len(nodes)*len(nodes))
 }
 
 func combineResources(numaNodes NUMANodeList, combination []int) v1.ResourceList {
@@ -153,3 +165,66 @@ func combineResources(numaNodes NUMANodeList, combination []int) v1.ResourceList
 
 	return resources
 }
+
+// numaNodesRequired returns bitmask with minimal NUMA nodes required to run given resources
+// or nil when resources can't be fitted onto the worker node
+// second value returned is a boolean indicating if bitmask is optimal from distance perspective
+func numaNodesRequired(identifier string, qos v1.PodQOSClass, numaNodes NUMANodeList, resources v1.ResourceList) (bitmask.BitMask, bool) {
+	for bitmaskLen := 1; bitmaskLen <= len(numaNodes); bitmaskLen++ {
+		numaNodesCombination := combin.Combinations(len(numaNodes), bitmaskLen)
+		suitableCombination, isMinDistance := findSuitableCombination(identifier, qos, numaNodes, resources, numaNodesCombination)
+		// we have found suitable combination for given bitmaskLen
+		if suitableCombination != nil {
+			bm := bitmask.NewEmptyBitMask()
+			bm.Add(suitableCombination...)
+			return bm, isMinDistance
+		}
+	}
+
+	return nil, false
+}
+
+// findSuitableCombination returns combination from numaNodesCombination that can fit resources, otherwise return nil
+// second value returned is a boolean indicating if returned combination is optimal from distance perspective
+// this function will always return combination that provides minimal average distance between nodes in combination
+func findSuitableCombination(identifier string, qos v1.PodQOSClass, numaNodes NUMANodeList, resources v1.ResourceList, numaNodesCombination [][]int) ([]int, bool) {
+	minAvgDistance := minAvgDistanceInCombinations(numaNodes, numaNodesCombination)
+	var (
+		minDistanceCombination []int
+		// init as max distance
+		minDistance float32 = 256
+	)
+	for _, combination := range numaNodesCombination {
+		combinationResources := combineResources(numaNodes, combination)
+		resourcesFit := checkResourcesFit(identifier, qos, resources, combinationResources)
+
+		if resourcesFit {
+			distance := nodesAvgDistance(numaNodes, combination...)
+			if distance == minAvgDistance {
+				// return early if we can fit resources into combination and provide minDistance
+				return combination, true
+			}
+			// we don't have to check which combination bitmask has lower value since we are generating them from lowest value
+			if distance < minDistance {
+				minDistance = distance
+				minDistanceCombination = combination
+			}
+		}
+	}
+
+	return minDistanceCombination, false
+}
+
+func checkResourcesFit(identifier string, qos v1.PodQOSClass, resources v1.ResourceList, combinationResources v1.ResourceList) bool {
+	for resource, quantity := range resources {
+		if quantity.IsZero() {
+			klog.V(4).InfoS("ignoring zero-qty resource request", "identifier", identifier, "resource", resource)
+			continue
+		}
+		if combinationQuantity := combinationResources[resource]; !isResourceSetSuitable(qos, resource, quantity, combinationQuantity) {
+			return false
+		}
+	}
+
+	return true
+}