fix: use Score as primary sort key in GPU scheduler policy by mesutoezdil · Pull Request #2012 · Project-HAMi/HAMi

mesutoezdil · 2026-07-03T19:49:43Z

Fix the GPU scheduler sort in Less(). NUMA was the primary key, which broke binpack and spread when devices span NUMA nodes. Score is now the primary key and NUMA is the tiebreaker.

To keep NUMA affinity working, NUMA grouping is preserved when a pod requests numa-bind (nvidia.com/numa-bind). Fit() accumulates a contiguous same-NUMA run, so score-primary interleaving would make numa-bind requests fail even when one NUMA node can host them. A NumaBind flag on DeviceUsageList selects the sort mode.

Changes

pkg/scheduler/policy/gpu_policy.go: Less() sorts by Score first, NUMA is the tiebreaker; NumaBind keeps NUMA grouping.
pkg/scheduler/scheduler.go: set NumaBind from the pod numa-bind annotation.
tests: policy unit tests (100% coverage) and a Fit()-level test for both the numa-bind and the score-primary paths.

Fixes #1806
Closes #2010
Part of #1889

Summary by CodeRabbit

New Features
- Added NUMA-aware GPU device ordering that preserves NUMA affinity when NUMA binding is requested.
- Scheduler now reads the pod annotation to enable/disable NUMA-binding behavior during device selection.
Bug Fixes
- Refined device ordering precedence between NUMA grouping and utilization scoring for both binpack and spread strategies.
Tests
- Added and expanded NUMA-focused scheduler tests covering contiguous same-NUMA selection with binding and globally idlest selection across NUMA when binding is disabled.

gemini-code-assist · 2026-07-03T19:49:48Z

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

coderabbitai · 2026-07-03T19:49:51Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a5e1daf2-dfdd-4862-a920-54a6a8756442

📥 Commits

Reviewing files that changed from the base of the PR and between 583f4c7 and 6162edb.

📒 Files selected for processing (1)

pkg/scheduler/scheduler.go

🚧 Files skipped from review as they are similar to previous changes (1)

pkg/scheduler/scheduler.go

📝 Walkthrough

Walkthrough

GPU device sorting now supports optional NUMA grouping. Pod annotations propagate NUMA-binding state into scheduling, while score-first and NUMA-aware ordering are covered by policy and NVIDIA fit tests.

Changes

GPU NUMA sorting and scheduling

Layer / File(s)	Summary
Device ordering semantics `pkg/scheduler/policy/gpu_policy.go`, `pkg/scheduler/policy/gpu_policy_test.go`	`DeviceUsageList` adds `NumaBind`; sorting uses NUMA grouping when enabled and score-first ordering otherwise, with expanded comparator tests.
Pod annotation propagation `pkg/scheduler/scheduler.go`	The scheduler parses `nvidia.NumaBind` and assigns the result to `DeviceUsageList`.
NUMA-aware fit validation `pkg/scheduler/numa_sort_test.go`	Tests validate NUMA-contiguous selection when binding is enabled and globally score-first selection when it is disabled.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Pod
  participant Scheduler
  participant DeviceUsageList
  participant NVIDIA_Fit
  Pod->>Scheduler: submit GPU request and NUMA annotation
  Scheduler->>DeviceUsageList: set policy and NumaBind
  DeviceUsageList->>NVIDIA_Fit: provide ordered devices
  NVIDIA_Fit-->>Scheduler: return selected devices

Possibly related PRs

Project-HAMi/HAMi#1874: Updates the same GPU sorting comparator and NUMA-binding propagation paths.
Project-HAMi/HAMi#2011: Modifies the GPU scheduling comparator for another per-pod scheduling-policy flag.
Project-HAMi/HAMi#2046: Also modifies the scheduler’s buildNodeUsage path used to construct per-node device usage data.

Suggested labels: enhancement

Suggested reviewers: lengrongfu, archlitchi

Poem

I’m a rabbit sorting GPUs neat,
By score and NUMA, row by seat.
Bind them close when pods request,
Or choose the idle one that’s best.
Hop, hop—tests confirm the feat!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: making score the primary GPU scheduler sort key.
Linked Issues check	✅ Passed	The changes implement score-first sorting, NUMA tie-breaking, and NUMA-bind preservation as requested by `#1806` and `#2010`.
Out of Scope Changes check	✅ Passed	The added tests and helper updates are directly related to the GPU scheduler sorting fix and NUMA-bind behavior.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

hami-robot · 2026-07-03T20:03:52Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mesutoezdil
Once this PR has been reviewed and has the lgtm label, please assign wawa0210 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

codecov · 2026-07-03T20:08:25Z

Codecov Report

❌ Patch coverage is 92.85714% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/scheduler/scheduler.go	77.77%	2 Missing ⚠️

Flag	Coverage Δ
unittests	`60.53% <92.85%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
pkg/scheduler/policy/gpu_policy.go	`100.00% <100.00%> (ø)`
pkg/scheduler/scheduler.go	`67.20% <77.77%> (+0.43%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

NUMA was the primary key in Less(), which broke binpack and spread when devices span NUMA nodes. Score is now primary, NUMA is the tiebreaker. Keep NUMA grouping when a pod requests numa-bind, so Fit can still accumulate a same-NUMA run and NUMA affinity is not broken. Fixes Project-HAMi#1806 Closes Project-HAMi#2010 Part of Project-HAMi#1889 Signed-off-by: mesutoezdil <mesudozdil@gmail.com>

Same conflict shape as feat/mutex-policy: upstream refactored inline node-usage construction into buildNodeUsage() while this branch added NumaBind support to the same code path. Kept the buildNodeUsage refactor and carried NumaBind: numaBindingRequested(task) into it. Signed-off-by: mesutoezdil <mesudozdil@gmail.com>

…icy-sort

mesutoezdil · 2026-07-17T19:59:53Z

@coderabbitai review

coderabbitai · 2026-07-17T20:00:00Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

wawa0210 · 2026-07-17T20:00:01Z

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

@coderabbitai review

coderabbitai · 2026-07-17T20:00:20Z

@wawa0210 Reviewing the latest changes now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

mesutoezdil · 2026-07-17T20:02:56Z

@coderabbitai full review

coderabbitai · 2026-07-17T20:03:03Z

✅ Action performed

Full review finished.

coderabbitai

🧹 Nitpick comments (1)

pkg/scheduler/scheduler.go (1)
531-542: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Simplify boolean parsing.

Since strconv.ParseBool always returns false when it encounters an error (e.g., an invalid or empty string), you can safely discard the error to simplify the return statement.
♻️ Proposed refactor
 func numaBindingRequested(task *corev1.Pod) bool {
 	if task == nil {
 		return false
 	}
 	v, ok := task.Annotations[nvidia.NumaBind]
 	if !ok {
 		return false
 	}
-	enforce, err := strconv.ParseBool(v)
-	return err == nil && enforce
+	enforce, _ := strconv.ParseBool(v)
+	return enforce
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/scheduler/scheduler.go` around lines 531 - 542, In numaBindingRequested,
simplify the strconv.ParseBool handling by discarding the parse error and
returning only the parsed boolean value. Preserve the existing nil-pod and
missing-annotation behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pkg/scheduler/scheduler.go`:
- Around line 531-542: In numaBindingRequested, simplify the strconv.ParseBool
handling by discarding the parse error and returning only the parsed boolean
value. Preserve the existing nil-pod and missing-annotation behavior.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 30b690ff-1102-467a-93bf-6627d95cff50

📥 Commits

Reviewing files that changed from the base of the PR and between 125c8c6 and 583f4c7.

📒 Files selected for processing (4)

pkg/scheduler/numa_sort_test.go
pkg/scheduler/policy/gpu_policy.go
pkg/scheduler/policy/gpu_policy_test.go
pkg/scheduler/scheduler.go

strconv.ParseBool returns false on error, so the parse error can be discarded without changing behavior. No functional change. Signed-off-by: mesutoezdil <mesudozdil@gmail.com>

mesutoezdil · 2026-07-17T20:41:49Z

/assign @archlitchi

hami-robot Bot added do-not-merge/work-in-progress dco-signoff: yes labels Jul 3, 2026

github-actions Bot added the kind/bug Something isn't working label Jul 3, 2026

hami-robot Bot added the size/XS label Jul 3, 2026

hami-robot Bot added size/M and removed size/XS labels Jul 3, 2026

mesutoezdil temporarily deployed to nvidia July 3, 2026 20:11 — with GitHub Actions Inactive

mesutoezdil force-pushed the fix/scheduler-policy-sort branch from 78d9d87 to e1858ae Compare July 3, 2026 20:54

mesutoezdil changed the title ~~fix/feat: GPU scheduler policy sort bug fix and policy combination~~ fix: use Score as primary sort key in GPU scheduler policy Jul 3, 2026

mesutoezdil temporarily deployed to nvidia July 3, 2026 21:02 — with GitHub Actions Inactive

mesutoezdil force-pushed the fix/scheduler-policy-sort branch from e1858ae to c7b6cf5 Compare July 10, 2026 13:27

hami-robot Bot added size/L and removed size/M labels Jul 10, 2026

mesutoezdil mentioned this pull request Jul 4, 2026

feat: new mutex GPU scheduling policy #2011

Open

mesutoezdil temporarily deployed to nvidia July 10, 2026 13:35 — with GitHub Actions Inactive

mesutoezdil marked this pull request as ready for review July 10, 2026 14:29

hami-robot Bot removed the do-not-merge/work-in-progress label Jul 10, 2026

hami-robot Bot requested review from archlitchi and lengrongfu July 10, 2026 14:29

mesutoezdil force-pushed the fix/scheduler-policy-sort branch from c7b6cf5 to b83a621 Compare July 10, 2026 14:47

mesutoezdil temporarily deployed to nvidia July 10, 2026 14:56 — with GitHub Actions Inactive

mesutoezdil temporarily deployed to nvidia July 13, 2026 07:53 — with GitHub Actions Inactive

Merge remote-tracking branch 'upstream/master' into fix/scheduler-pol…

583f4c7

…icy-sort

coderabbitai Bot added the enhancement label Jul 17, 2026

coderabbitai Bot mentioned this pull request Jul 17, 2026

fix(scheduler): make NUMA sort opt-in for binpack/spread policies #1874

Open

mesutoezdil temporarily deployed to nvidia July 17, 2026 13:51 — with GitHub Actions Inactive

coderabbitai Bot removed the enhancement label Jul 17, 2026

coderabbitai Bot reviewed Jul 17, 2026

View reviewed changes

refactor(scheduler): simplify numaBindingRequested bool parsing

6162edb

strconv.ParseBool returns false on error, so the parse error can be discarded without changing behavior. No functional change. Signed-off-by: mesutoezdil <mesudozdil@gmail.com>

coderabbitai Bot added the enhancement label Jul 17, 2026

mesutoezdil deployed to nvidia July 17, 2026 20:36 — with GitHub Actions Active

hami-robot Bot assigned archlitchi Jul 17, 2026

Uh oh!

Conversation

mesutoezdil commented Jul 3, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

gemini-code-assist Bot commented Jul 3, 2026

Uh oh!

coderabbitai Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

hami-robot Bot commented Jul 3, 2026

Uh oh!

codecov Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mesutoezdil commented Jul 17, 2026

Uh oh!

coderabbitai Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wawa0210 commented Jul 17, 2026

Uh oh!

coderabbitai Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mesutoezdil commented Jul 17, 2026

Uh oh!

coderabbitai Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mesutoezdil commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mesutoezdil commented Jul 3, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 3, 2026 •

edited

Loading

codecov Bot commented Jul 3, 2026 •

edited

Loading

coderabbitai Bot commented Jul 17, 2026 •

edited

Loading

coderabbitai Bot commented Jul 17, 2026 •

edited

Loading

coderabbitai Bot commented Jul 17, 2026 •

edited

Loading