scheduler: Fixed a bug where the bandwidth of reserved cores was not taken into account

Author: mvegter

client/client.go

@@ -3323,8 +3323,7 @@ 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
+	total := node.AvailableResources()
 	allocated := c.getAllocatedResources(node)
 
 	// Emit allocated
@@ -3342,20 +3341,13 @@ 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
+	unallocated := total.Copy()
+	unallocated.Subtract(allocated)
+	metrics.SetGaugeWithLabels([]string{"client", "unallocated", "memory"}, float32(unallocated.Flattened.Memory.MemoryMB), baseLabels)
+	metrics.SetGaugeWithLabels([]string{"client", "unallocated", "disk"}, float32(unallocated.Shared.DiskMB), baseLabels)
+	metrics.SetGaugeWithLabels([]string{"client", "unallocated", "cpu"}, float32(unallocated.Flattened.Cpu.CpuShares), baseLabels)
 
-	// 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
-
-	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 total.Flattened.Networks {
 		// Determined the used resources
 		var usedMbits int
 		totalIdx := allocated.Flattened.Networks.NetIndex(n)

client/lib/numalib/topology.go

@@ -282,6 +282,16 @@ func (st *Topology) NumECores() int {
 	return total
 }
 
+// TotalCores returns the number of logical cores detected. The UsableCores will
+// be equal to or less than this value.
+func (st *Topology) TotalCores() *idset.Set[hw.CoreID] {
+	result := idset.Empty[hw.CoreID]()
+	for _, cpu := range st.Cores {
+		result.Insert(cpu.ID)
+	}
+	return result
+}
+
 // UsableCores returns the number 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).

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 available node resources are a super set of those that are being allocated
+	available := node.AvailableResources()
 	if superset, dimension := available.Superset(used); !superset {
 		return false, dimension, used, nil
 	}
@@ -232,20 +230,9 @@ 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)
-	}
-
-	// Compute the free percentage
-	freePctCpu = 1 - (float64(util.Flattened.Cpu.CpuShares) / nodeCpu)
-	freePctRam = 1 - (float64(util.Flattened.Memory.MemoryMB) / nodeMem)
+	resources := node.AvailableResources()
+	freePctCpu = 1 - (float64(util.Flattened.Cpu.CpuShares) / float64(resources.Flattened.Cpu.CpuShares))
+	freePctRam = 1 - (float64(util.Flattened.Memory.MemoryMB) / float64(resources.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,
@@ -248,6 +259,38 @@ func TestAllocsFit(t *testing.T) {
 	must.Eq(t, 1000, used.Flattened.Cpu.CpuShares)
 	must.Eq(t, []uint16{0}, used.Flattened.Cpu.ReservedCores)
 	must.Eq(t, 1024, used.Flattened.Memory.MemoryMB)
+
+	a3 := &Allocation{
+		AllocatedResources: &AllocatedResources{
+			Tasks: map[string]*AllocatedTaskResources{
+				"web": {
+					Cpu: AllocatedCpuResources{
+						CpuShares: 1000,
+					},
+					Memory: AllocatedMemoryResources{
+						MemoryMB: 512,
+					},
+				},
+			},
+		},
+	}
+
+	// Should fit one allocation
+	fit, dim, used, err = AllocsFit(n, []*Allocation{a3}, nil, false)
+	must.NoError(t, err)
+	must.True(t, fit, must.Sprintf("failed for dimension %q", dim))
+	must.Eq(t, 1000, used.Flattened.Cpu.CpuShares)
+	must.Eq(t, []uint16{}, used.Flattened.Cpu.ReservedCores)
+	must.Eq(t, 512, used.Flattened.Memory.MemoryMB)
+
+	// Should not fit second allocation
+	fit, dim, used, err = AllocsFit(n, []*Allocation{a3, a3}, 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, []uint16{}, used.Flattened.Cpu.ReservedCores)
+	must.Eq(t, 1024, used.Flattened.Memory.MemoryMB)
 }
 
 func TestAllocsFit_Cores(t *testing.T) {
@@ -649,8 +692,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 +719,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

@@ -2352,6 +2352,24 @@ func (n *Node) Stub(fields *NodeStubFields) *NodeListStub {
 	return s
 }
 
+// AvailableResources returns a comparable version of the resources available
+// for scheduling workloads.
+func (n *Node) AvailableResources() *ComparableResources {
+	if n == nil {
+		return nil
+	}
+
+	available := n.NodeResources.Comparable()
+	reserved := n.ReservedResources.Comparable()
+
+	// ReservedResources is a simple representation of the excluded resources. It is not aware of the hardware topology,
+	// hence we need to manually account for the reserved cpu and the cpu of the reserved cores.
+	available.Subtract(reserved)
+	available.Flattened.Cpu.CpuShares = int64(n.NodeResources.Processors.Topology.UsableCompute())
+
+	return available
+}
+
 // NodeListStub is used to return a subset of job information
 // for the job list
 type NodeListStub struct {
@@ -3239,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 {
+	totalCores := n.Processors.Topology.TotalCores().Slice()
+	cores := helper.ConvertSlice(totalCores, 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.AvailableResources()
 }
 
 // SetCandidates initializes the candidate set from which preemptions are chosen