diff --git a/pkg/scheduler/numa_sort_test.go b/pkg/scheduler/numa_sort_test.go index cfcee1ea7..653850a5a 100644 --- a/pkg/scheduler/numa_sort_test.go +++ b/pkg/scheduler/numa_sort_test.go @@ -27,13 +27,14 @@ import ( "github.com/Project-HAMi/HAMi/pkg/device" "github.com/Project-HAMi/HAMi/pkg/device/nvidia" "github.com/Project-HAMi/HAMi/pkg/scheduler/policy" + "github.com/Project-HAMi/HAMi/pkg/util" ) // sortedDevices mirrors what fitInDevices does: it wraps the devices in a -// DeviceUsageList (carrying Policy and NumaBind), sorts them, and returns the -// sorted device slice in the same order Fit() will iterate. -func sortedDevices(devs []*device.DeviceUsage, scores []float32, policyName string, numaBind bool) []*device.DeviceUsage { - dl := policy.DeviceUsageList{Policy: policyName, NumaBind: numaBind} +// DeviceUsageList (carrying Policy and NumaIgnore), sorts them, and returns +// the sorted device slice in the same order Fit() will iterate. +func sortedDevices(devs []*device.DeviceUsage, scores []float32, policyName string, numaIgnore bool) []*device.DeviceUsage { + dl := policy.DeviceUsageList{Policy: policyName, NumaIgnore: numaIgnore} for i, d := range devs { dl.DeviceLists = append(dl.DeviceLists, &policy.DeviceListsScore{Device: d, Score: scores[i]}) } @@ -54,10 +55,10 @@ func numaTestNvidia() *nvidia.NvidiaGPUDevices { }) } -// A 2-card numa-bind request fits only on NUMA 0 (two devices there, one on -// NUMA 1 with a middle score). Fit needs a contiguous same-NUMA run, so the -// sort decides the outcome: score-primary interleaves NUMA 1 and fails; -// NumaBind grouping keeps NUMA 0 together and fits. +// A 2-card request fits only on NUMA 0 (two devices there, one on NUMA 1 +// with a middle score). Fit needs a contiguous same-NUMA run, so the sort +// decides the outcome: NUMA grouping (the default) keeps NUMA 0 together and +// fits; NumaIgnore's pure-Score order interleaves NUMA 1 in and breaks it. func TestNumaBindSortPreservesAffinity(t *testing.T) { nv := numaTestNvidia() mk := func(id string, numa int) *device.DeviceUsage { @@ -66,7 +67,7 @@ func TestNumaBindSortPreservesAffinity(t *testing.T) { Type: nvidia.NvidiaGPUDevice, Health: true, Numa: numa, } } - // A(n0,score5), B(n0,score1), C(n1,score3): score-primary spread interleaves + // A(n0,score5), B(n0,score1), C(n1,score3): pure-Score spread interleaves // C between A and B. devs := []*device.DeviceUsage{mk("A_n0", 0), mk("B_n0", 0), mk("C_n1", 1)} scores := []float32{5, 1, 3} @@ -76,12 +77,12 @@ func TestNumaBindSortPreservesAffinity(t *testing.T) { Annotations: map[string]string{nvidia.NumaBind: "true"}, }} - // Without NUMA grouping the same-NUMA run is broken and Fit fails. - unfit, _, _ := nv.Fit(sortedDevices(devs, scores, "spread", false), req, pod, &device.NodeInfo{}, &device.PodDevices{}) + // With NUMA ignored, the same-NUMA run is broken and Fit fails. + unfit, _, _ := nv.Fit(sortedDevices(devs, scores, "spread", true), req, pod, &device.NodeInfo{}, &device.PodDevices{}) assert.Equal(t, unfit, false) - // With NUMA grouping (our fix) the two NUMA-0 devices stay contiguous and fit. - fit, result, _ := nv.Fit(sortedDevices(devs, scores, "spread", true), req, pod, &device.NodeInfo{}, &device.PodDevices{}) + // By default the two NUMA-0 devices stay contiguous and fit. + fit, result, _ := nv.Fit(sortedDevices(devs, scores, "spread", false), req, pod, &device.NodeInfo{}, &device.PodDevices{}) assert.Equal(t, fit, true) assert.Equal(t, len(result[nvidia.NvidiaGPUDevice]), 2) for _, r := range result[nvidia.NvidiaGPUDevice] { @@ -89,8 +90,9 @@ func TestNumaBindSortPreservesAffinity(t *testing.T) { } } -// #1806: without numa-bind, Score wins across NUMA. The idlest device is on -// the lower NUMA id, which the old NUMA-primary sort would skip. Score-primary +// #1806: with hami.io/topology-aware-scoring: "false", Score wins across +// NUMA. The idlest device is on the lower NUMA id, which the default +// NUMA-primary sort would skip in favor of the NUMA-1 group. NumaIgnore // picks the globally idlest device regardless of NUMA. func TestScorePrimarySelectsAcrossNuma(t *testing.T) { nv := numaTestNvidia() @@ -105,9 +107,11 @@ func TestScorePrimarySelectsAcrossNuma(t *testing.T) { scores := []float32{1, 8, 5} req := device.ContainerDeviceRequest{Nums: 1, Memreq: 100, Coresreq: 10, Type: nvidia.NvidiaGPUDevice} - pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{}} + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{util.GPUTopologyAwareAnnotationKey: "false"}, + }} - fit, result, _ := nv.Fit(sortedDevices(devs, scores, "spread", false), req, pod, &device.NodeInfo{}, &device.PodDevices{}) + fit, result, _ := nv.Fit(sortedDevices(devs, scores, "spread", true), req, pod, &device.NodeInfo{}, &device.PodDevices{}) assert.Equal(t, fit, true) assert.Equal(t, len(result[nvidia.NvidiaGPUDevice]), 1) assert.Equal(t, result[nvidia.NvidiaGPUDevice][0].UUID, "P_n0") diff --git a/pkg/scheduler/policy/gpu_policy.go b/pkg/scheduler/policy/gpu_policy.go index a58b483b3..e28694380 100644 --- a/pkg/scheduler/policy/gpu_policy.go +++ b/pkg/scheduler/policy/gpu_policy.go @@ -32,9 +32,13 @@ type DeviceListsScore struct { type DeviceUsageList struct { DeviceLists []*DeviceListsScore Policy string - // NumaBind groups devices by NUMA so Fit can accumulate a same-NUMA run. - // When false, Score is the primary key and NUMA only breaks ties (#1806). + // NumaBind is kept for backward compatibility with the nvidia.com/numa-bind + // annotation; NUMA grouping is now the default (see NumaIgnore) so this no + // longer changes Less's behavior. NumaBind bool + // NumaIgnore sorts purely by Score, ignoring NUMA locality. Set via the + // hami.io/topology-aware-scoring: "false" annotation (#1806). + NumaIgnore bool } func (l DeviceUsageList) Len() int { @@ -59,30 +63,29 @@ func (l DeviceUsageList) Less(i, j int) bool { return ni < nj } - // numa-bind: keep NUMA groups contiguous for Fit's same-NUMA accumulation. - if l.NumaBind { + // numa-ignore: Score is primary, NUMA only breaks ties for stable ordering (#1806). + if l.NumaIgnore { if binpack { - if ni == nj { + if si != sj { return si < sj } - return ni > nj + return ni < nj } - // default policy is spread - if ni == nj { + if si != sj { return si > sj } return ni < nj } - // score primary, NUMA tiebreaker (#1806). + // default: NUMA groups first, Score orders devices within a NUMA node. if binpack { - if si != sj { + if ni == nj { return si < sj } - return ni < nj + return ni > nj } // default policy is spread - if si != sj { + if ni == nj { return si > sj } return ni < nj @@ -100,6 +103,7 @@ func (l DeviceUsageList) DeepCopy() DeviceUsageList { DeviceLists: deviceLists, Policy: l.Policy, NumaBind: l.NumaBind, + NumaIgnore: l.NumaIgnore, } } diff --git a/pkg/scheduler/policy/gpu_policy_test.go b/pkg/scheduler/policy/gpu_policy_test.go index a97f2d5f0..b2102db84 100644 --- a/pkg/scheduler/policy/gpu_policy_test.go +++ b/pkg/scheduler/policy/gpu_policy_test.go @@ -125,6 +125,7 @@ func TestDeviceUsageList_Less(t *testing.T) { name string policy string numaBind bool + numaIgnore bool deviceLists []*DeviceListsScore expectedLess bool }{ @@ -138,78 +139,42 @@ func TestDeviceUsageList_Less(t *testing.T) { expectedLess: true, }, { - name: "Binpack: score primary, lower score sorts first across NUMA", + name: "Binpack: NUMA primary, higher NUMA sorts first regardless of Score", policy: "binpack", deviceLists: []*DeviceListsScore{ - {Device: &device.DeviceUsage{Numa: 1, Used: 10}, Score: 10}, - {Device: &device.DeviceUsage{Numa: 0, Used: 20}, Score: 20}, - }, - expectedLess: true, - }, - { - name: "Binpack: score primary, higher score sorts last even on lower NUMA", - policy: "binpack", - deviceLists: []*DeviceListsScore{ - {Device: &device.DeviceUsage{Numa: 0, Used: 10}, Score: 10}, - {Device: &device.DeviceUsage{Numa: 1, Used: 20}, Score: 20}, - }, - expectedLess: true, - }, - { - name: "Binpack: NUMA tiebreaker when scores equal", - policy: "binpack", - deviceLists: []*DeviceListsScore{ - {Device: &device.DeviceUsage{Numa: 0}, Score: 15}, - {Device: &device.DeviceUsage{Numa: 1}, Score: 15}, + {Device: &device.DeviceUsage{Numa: 1, Used: 10}, Score: 5}, + {Device: &device.DeviceUsage{Numa: 0, Used: 20}, Score: 99}, }, expectedLess: true, }, { - name: "Binpack: score primary over NUMA, high-NUMA high-score sorts last", + name: "Binpack: same NUMA falls back to lower Score first", policy: "binpack", deviceLists: []*DeviceListsScore{ - {Device: &device.DeviceUsage{Numa: 1}, Score: 20}, {Device: &device.DeviceUsage{Numa: 0}, Score: 10}, + {Device: &device.DeviceUsage{Numa: 0}, Score: 20}, }, - expectedLess: false, - }, - { - name: "Spread: score primary, higher score sorts first", - policy: "spread", - deviceLists: []*DeviceListsScore{ - {Device: &device.DeviceUsage{Numa: 0, Used: 10}, Score: 10}, - {Device: &device.DeviceUsage{Numa: 0, Used: 20}, Score: 20}, - }, - expectedLess: false, - }, - { - name: "Spread: score primary, lower score sorts last across NUMA", - policy: "spread", - deviceLists: []*DeviceListsScore{ - {Device: &device.DeviceUsage{Numa: 1, Used: 10}, Score: 10}, - {Device: &device.DeviceUsage{Numa: 0, Used: 20}, Score: 20}, - }, - expectedLess: false, + expectedLess: true, }, { - name: "Spread: score primary over NUMA, high-NUMA high-score sorts first", + name: "Spread: NUMA primary, lower NUMA sorts first regardless of Score", policy: "spread", deviceLists: []*DeviceListsScore{ - {Device: &device.DeviceUsage{Numa: 1}, Score: 20}, - {Device: &device.DeviceUsage{Numa: 0}, Score: 10}, + {Device: &device.DeviceUsage{Numa: 0}, Score: 5}, + {Device: &device.DeviceUsage{Numa: 1}, Score: 99}, }, expectedLess: true, }, { - name: "Spread: NUMA tiebreaker when scores equal", + name: "Spread: same NUMA falls back to higher Score first", policy: "spread", deviceLists: []*DeviceListsScore{ - {Device: &device.DeviceUsage{Numa: 0}, Score: 15}, - {Device: &device.DeviceUsage{Numa: 1}, Score: 15}, + {Device: &device.DeviceUsage{Numa: 0}, Score: 20}, + {Device: &device.DeviceUsage{Numa: 0}, Score: 10}, }, expectedLess: true, }, - // NumaBind=true: NUMA is primary, Score orders within a NUMA node. + // NumaBind is now a no-op (default already groups by NUMA); kept for compatibility. { name: "Binpack+NumaBind: NUMA primary, higher NUMA sorts first regardless of Score", policy: "binpack", @@ -277,6 +242,46 @@ func TestDeviceUsageList_Less(t *testing.T) { }, expectedLess: true, }, + { + name: "Binpack NumaIgnore score primary", + policy: "binpack", + numaIgnore: true, + deviceLists: []*DeviceListsScore{ + {Device: &device.DeviceUsage{Numa: 0, Used: 10}, Score: 10}, + {Device: &device.DeviceUsage{Numa: 1, Used: 20}, Score: 20}, + }, + expectedLess: true, + }, + { + name: "Spread NumaIgnore score primary", + policy: "spread", + numaIgnore: true, + deviceLists: []*DeviceListsScore{ + {Device: &device.DeviceUsage{Numa: 1, Used: 10}, Score: 10}, + {Device: &device.DeviceUsage{Numa: 0, Used: 20}, Score: 20}, + }, + expectedLess: false, + }, + { + name: "Binpack NumaIgnore: NUMA tiebreaker when scores equal", + policy: "binpack", + numaIgnore: true, + deviceLists: []*DeviceListsScore{ + {Device: &device.DeviceUsage{Numa: 0}, Score: 15}, + {Device: &device.DeviceUsage{Numa: 1}, Score: 15}, + }, + expectedLess: true, + }, + { + name: "Spread NumaIgnore: NUMA tiebreaker when scores equal", + policy: "spread", + numaIgnore: true, + deviceLists: []*DeviceListsScore{ + {Device: &device.DeviceUsage{Numa: 0}, Score: 15}, + {Device: &device.DeviceUsage{Numa: 1}, Score: 15}, + }, + expectedLess: true, + }, } for _, tt := range tests { @@ -284,6 +289,7 @@ func TestDeviceUsageList_Less(t *testing.T) { l := DeviceUsageList{ Policy: tt.policy, NumaBind: tt.numaBind, + NumaIgnore: tt.numaIgnore, DeviceLists: tt.deviceLists, } i, j := 0, 1 @@ -307,7 +313,8 @@ func TestDeviceUsageListDeepCopy(t *testing.T) { { name: "fully populated", original: DeviceUsageList{ - Policy: "binpack", + Policy: "binpack", + NumaIgnore: true, DeviceLists: []*DeviceListsScore{ { Device: &device.DeviceUsage{ diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index 6c67bc4ac..42cfbfab0 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -543,12 +543,14 @@ func numaBindingRequested(task *corev1.Pod) bool { func buildNodeUsage(node *device.NodeInfo, task *corev1.Pod) *NodeUsage { userGPUPolicy := util.GetGPUSchedulerPolicyByPod(device.GPUSchedulerPolicy, task) + numaIgnore := task != nil && task.Annotations != nil && task.Annotations[util.GPUTopologyAwareAnnotationKey] == "false" nodeUsage := &NodeUsage{ Node: node.Node, NodeInfo: node, Devices: policy.DeviceUsageList{ Policy: userGPUPolicy, NumaBind: numaBindingRequested(task), + NumaIgnore: numaIgnore, DeviceLists: make([]*policy.DeviceListsScore, 0), }, } diff --git a/pkg/util/types.go b/pkg/util/types.go index 0e0d9ffc5..ab3e934c1 100644 --- a/pkg/util/types.go +++ b/pkg/util/types.go @@ -85,6 +85,8 @@ const ( NodeSchedulerPolicyAnnotationKey = "hami.io/node-scheduler-policy" // GPUSchedulerPolicyAnnotationKey is user set Pod annotation to change this default GPU policy. GPUSchedulerPolicyAnnotationKey = "hami.io/gpu-scheduler-policy" + // GPUTopologyAwareAnnotationKey disables NUMA-first sorting when set to "false". + GPUTopologyAwareAnnotationKey = "hami.io/topology-aware-scoring" ) func (s SchedulerPolicyName) String() string {