Skip to content

fix: use Score as primary sort key in GPU scheduler policy#2012

Draft
mesutoezdil wants to merge 1 commit into
Project-HAMi:masterfrom
mesutoezdil:fix/scheduler-policy-sort
Draft

fix: use Score as primary sort key in GPU scheduler policy#2012
mesutoezdil wants to merge 1 commit into
Project-HAMi:masterfrom
mesutoezdil:fix/scheduler-policy-sort

Conversation

@mesutoezdil

@mesutoezdil mesutoezdil commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Fix sort bug in Less() where NUMA was the primary key, breaking binpack and spread behavior across NUMA nodes. Score is now primary, NUMA is tiebreaker when scores are equal.

Changes

pkg/scheduler/policy/gpu_policy.go

  • Less(): Score is primary sort key, NUMA is tiebreaker

pkg/scheduler/policy/gpu_policy_test.go

  • Updated existing cases to match correct sort semantics
  • Added 4 binpack and 4 spread cases covering Score-vs-NUMA interactions

Fixes #1806
Closes #2010
Part of #1889

Test plan

  • go test ./pkg/scheduler/policy/... --race -count=1
  • Verify binpack packs into most-loaded GPU across NUMA nodes
  • Verify spread picks least-loaded GPU across NUMA nodes

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Note

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

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

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ee251fd2-355c-4e7f-bbdc-45a00dd717b7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

hami-robot Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

[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:

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

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

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
unittests 59.12% <100.00%> (+0.02%) ⬆️

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%> (ø)

... and 1 file with indirect coverage changes

🚀 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(), breaking binpack and spread intent
when devices are on different NUMA nodes. Score is now primary, NUMA
is tiebreaker when scores are equal.

Fixes Project-HAMi#1806
Closes Project-HAMi#2010
Part of Project-HAMi#1889

Signed-off-by: mesutoezdil <mesudozdil@gmail.com>
@mesutoezdil mesutoezdil force-pushed the fix/scheduler-policy-sort branch from 78d9d87 to e1858ae Compare July 3, 2026 20:54
@mesutoezdil 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

mesutoezdil commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

the original Less() was comparing NUMA first and only falling through to Score when NUMA matched — which means a device on a higher NUMA node could sort ahead of a lower-scored device on a lower NUMA node. thats backwards for binpack where you want to fill the most-used GPU first regardless of which NUMA domain its in.

making Score the primary key and NUMA the tiebreaker is correct. the added test cases cover the cross-NUMA score ordering too which is what was actually broken.

Why are you commenting on a draft? This is a topic we're currently working on.
And why do you feel the need to share the AI output?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix/feat: GPU scheduler policy sort bug fix and policy combination gpu scheduler policy sort bug

1 participant