scheduler: fixed a bug where the bandwidth of reserved cores were not taken into account

Author: mvegter

.changelog/26768.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+scheduler: fixed a bug where the bandwidth of reserved cores were not taken into account
+```

client/client.go

@@ -3323,8 +3323,8 @@ func (c *Client) setGaugeForDiskStats(hStats *hoststats.HostStats, baseLabels []
 // setGaugeForAllocationStats proxies metrics for allocation specific statistics
 func (c *Client) setGaugeForAllocationStats(baseLabels []metrics.Label) {
 	node := c.GetConfig().Node
-	total := node.NodeResources
-	res := node.ReservedResources
+
+	available := node.Comparable()
 	allocated := c.getAllocatedResources(node)
 
 	// Emit allocated
@@ -3342,20 +3342,15 @@ func (c *Client) setGaugeForAllocationStats(baseLabels []metrics.Label) {
 	}
 
 	// Emit unallocated
-	unallocatedMem := total.Memory.MemoryMB - res.Memory.MemoryMB - allocated.Flattened.Memory.MemoryMB
-	unallocatedDisk := total.Disk.DiskMB - res.Disk.DiskMB - allocated.Shared.DiskMB
-
-	// The UsableCompute function call already subtracts and accounts for any
-	// reserved CPU within the client configuration. Therefore, we do not need
-	// to subtract that here.
-	unallocatedCpu := int64(total.Processors.Topology.UsableCompute()) - allocated.Flattened.Cpu.CpuShares
+	unallocatedMem := available.Flattened.Memory.MemoryMB - allocated.Flattened.Memory.MemoryMB
+	unallocatedDisk := available.Shared.DiskMB - allocated.Shared.DiskMB
+	unallocatedCpu := available.Flattened.Cpu.CpuShares - allocated.Flattened.Cpu.CpuShares
 
 	metrics.SetGaugeWithLabels([]string{"client", "unallocated", "memory"}, float32(unallocatedMem), baseLabels)
 	metrics.SetGaugeWithLabels([]string{"client", "unallocated", "disk"}, float32(unallocatedDisk), baseLabels)
 	metrics.SetGaugeWithLabels([]string{"client", "unallocated", "cpu"}, float32(unallocatedCpu), baseLabels)
 
-	totalComparable := total.Comparable()
-	for _, n := range totalComparable.Flattened.Networks {
+	for _, n := range available.Flattened.Networks {
 		// Determined the used resources
 		var usedMbits int
 		totalIdx := allocated.Flattened.Networks.NetIndex(n)

client/lib/numalib/topology.go

@@ -282,7 +282,17 @@ func (st *Topology) NumECores() int {
 	return total
 }
 
-// UsableCores returns the number of logical cores usable by the Nomad client
+// AllCores returns the set of logical cores detected. The UsableCores will
+// be equal to or less than this value.
+func (st *Topology) AllCores() *idset.Set[hw.CoreID] {
+	result := idset.Empty[hw.CoreID]()
+	for _, cpu := range st.Cores {
+		result.Insert(cpu.ID)
+	}
+	return result
+}
+
+// UsableCores returns the set of logical cores usable by the Nomad client
 // for running tasks. Nomad must subtract off any reserved cores (reserved.cores)
 // and/or must mask the cpuset to the one set in config (config.reservable_cores).
 func (st *Topology) UsableCores() *idset.Set[hw.CoreID] {

nomad/structs/funcs.go

@@ -189,10 +189,8 @@ func AllocsFit(node *Node, allocs []*Allocation, netIdx *NetworkIndex, checkDevi
 		return false, "cores", used, nil
 	}
 
-	// Check that the node resources (after subtracting reserved) are a
-	// super set of those that are being allocated
-	available := node.NodeResources.Comparable()
-	available.Subtract(node.ReservedResources.Comparable())
+	// Check that the node resources are a super set of those that are being allocated
+	available := node.Comparable()
 	if superset, dimension := available.Superset(used); !superset {
 		return false, dimension, used, nil
 	}
@@ -232,20 +230,12 @@ func AllocsFit(node *Node, allocs []*Allocation, netIdx *NetworkIndex, checkDevi
 }
 
 func computeFreePercentage(node *Node, util *ComparableResources) (freePctCpu, freePctRam float64) {
-	reserved := node.ReservedResources.Comparable()
-	res := node.NodeResources.Comparable()
-
 	// Determine the node availability
-	nodeCpu := float64(res.Flattened.Cpu.CpuShares)
-	nodeMem := float64(res.Flattened.Memory.MemoryMB)
-	if reserved != nil {
-		nodeCpu -= float64(reserved.Flattened.Cpu.CpuShares)
-		nodeMem -= float64(reserved.Flattened.Memory.MemoryMB)
-	}
+	available := node.Comparable()
 
 	// Compute the free percentage
-	freePctCpu = 1 - (float64(util.Flattened.Cpu.CpuShares) / nodeCpu)
-	freePctRam = 1 - (float64(util.Flattened.Memory.MemoryMB) / nodeMem)
+	freePctCpu = 1 - (float64(util.Flattened.Cpu.CpuShares) / float64(available.Flattened.Cpu.CpuShares))
+	freePctRam = 1 - (float64(util.Flattened.Memory.MemoryMB) / float64(available.Flattened.Memory.MemoryMB))
 	return freePctCpu, freePctRam
 }
 

nomad/structs/funcs_test.go

@@ -106,6 +106,16 @@ func node2k() *Node {
 						ID:        1,
 						Grade:     numalib.Performance,
 						BaseSpeed: 1000,
+					}, {
+						ID:        2,
+						Grade:     numalib.Performance,
+						BaseSpeed: 1000,
+						Disable:   true,
+					}, {
+						ID:        3,
+						Grade:     numalib.Performance,
+						BaseSpeed: 1000,
+						Disable:   true,
 					}},
 					OverrideWitholdCompute: 1000, // set by client reserved field
 				},
@@ -137,7 +147,8 @@ func node2k() *Node {
 		},
 		ReservedResources: &NodeReservedResources{
 			Cpu: NodeReservedCpuResources{
-				CpuShares: 1000,
+				CpuShares:        1000,
+				ReservedCpuCores: []uint16{2, 3},
 			},
 			Memory: NodeReservedMemoryResources{
 				MemoryMB: 1024,
@@ -201,9 +212,10 @@ func TestAllocsFit(t *testing.T) {
 	must.Eq(t, 1024, used.Flattened.Memory.MemoryMB)
 
 	// Should not fit second allocation
-	fit, _, used, err = AllocsFit(n, []*Allocation{a1, a1}, nil, false)
+	fit, dim, used, err = AllocsFit(n, []*Allocation{a1, a1}, nil, false)
 	must.NoError(t, err)
 	must.False(t, fit)
+	must.Eq(t, "cpu", dim)
 	must.Eq(t, 2000, used.Flattened.Cpu.CpuShares)
 	must.Eq(t, 2048, used.Flattened.Memory.MemoryMB)
 
@@ -649,8 +661,23 @@ func TestScoreFitBinPack(t *testing.T) {
 				Cores: []numalib.Core{{
 					ID:        0,
 					Grade:     numalib.Performance,
-					BaseSpeed: 4096,
+					BaseSpeed: 2048,
+				}, {
+					ID:        1,
+					Grade:     numalib.Performance,
+					BaseSpeed: 2048,
+				}, {
+					ID:        2,
+					Grade:     numalib.Performance,
+					BaseSpeed: 2048,
+					Disable:   true,
+				}, {
+					ID:        3,
+					Grade:     numalib.Performance,
+					BaseSpeed: 2048,
+					Disable:   true,
 				}},
+				OverrideWitholdCompute: 2048, // set by client reserved field
 			},
 		},
 		Memory: NodeMemoryResources{
@@ -661,7 +688,8 @@ func TestScoreFitBinPack(t *testing.T) {
 	node.NodeResources.Compatibility()
 	node.ReservedResources = &NodeReservedResources{
 		Cpu: NodeReservedCpuResources{
-			CpuShares: 2048,
+			CpuShares:        2048,
+			ReservedCpuCores: []uint16{2, 3},
 		},
 		Memory: NodeReservedMemoryResources{
 			MemoryMB: 4096,

nomad/structs/structs.go

@@ -2247,6 +2247,19 @@ func (n *Node) Canonicalize() {
 	}
 }
 
+// Comparable returns a comparable version of the node's resources available for scheduling.
+// This conversion can be lossy so care must be taken when using it.
+func (n *Node) Comparable() *ComparableResources {
+	if n == nil {
+		return nil
+	}
+
+	resources := n.NodeResources.Comparable()
+	resources.Subtract(n.ReservedResources.Comparable())
+
+	return resources
+}
+
 func (n *Node) Copy() *Node {
 	if n == nil {
 		return nil
@@ -3244,16 +3257,16 @@ func (n *NodeResources) Comparable() *ComparableResources {
 		return nil
 	}
 
-	usableCores := n.Processors.Topology.UsableCores().Slice()
-	reservableCores := helper.ConvertSlice(usableCores, func(id hw.CoreID) uint16 {
+	allCores := n.Processors.Topology.AllCores().Slice()
+	cores := helper.ConvertSlice(allCores, func(id hw.CoreID) uint16 {
 		return uint16(id)
 	})
 
 	c := &ComparableResources{
 		Flattened: AllocatedTaskResources{
 			Cpu: AllocatedCpuResources{
 				CpuShares:     int64(n.Processors.Topology.TotalCompute()),
-				ReservedCores: reservableCores,
+				ReservedCores: cores,
 			},
 			Memory: AllocatedMemoryResources{
 				MemoryMB: n.Memory.MemoryMB,

scheduler/feasible/preemption.go

@@ -162,13 +162,7 @@ func (p *Preemptor) Copy() *Preemptor {
 
 // SetNode sets the node
 func (p *Preemptor) SetNode(node *structs.Node) {
-	nodeRemainingResources := node.NodeResources.Comparable()
-
-	// Subtract the reserved resources of the node
-	if c := node.ReservedResources.Comparable(); c != nil {
-		nodeRemainingResources.Subtract(c)
-	}
-	p.nodeRemainingResources = nodeRemainingResources
+	p.nodeRemainingResources = node.Comparable()
 }
 
 // SetCandidates initializes the candidate set from which preemptions are chosen