Skip to content

[WIP][Core][GPU fraction] Change StrictPack to check availability per-bundle instead of aggregated bundle to prevent false positive - #62344

Open
dancingactor wants to merge 4 commits into
ray-project:masterfrom
dancingactor:fix-strict-pack
Open

[WIP][Core][GPU fraction] Change StrictPack to check availability per-bundle instead of aggregated bundle to prevent false positive#62344
dancingactor wants to merge 4 commits into
ray-project:masterfrom
dancingactor:fix-strict-pack

Conversation

@dancingactor

@dancingactor dancingactor commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

PR Description

This PR is the first step in fixing the STRICT_PACK false positive issue during node availability checks.

To completely resolve this issue, we are taking a two-step approach:

  1. This PR make StrictPack check availability per-bundle instead of the aggregated bundle
  2. After Upcoming PR updates NodeResources to track per-instance availability, SubtractNodeAvailableResources and AddNodeAvailableResources will be updated to perform per-instance manipulation instead of scalar math

STRICT_PACK False Positive Problem

Currently, scheduling relies on aggregated scalar values. For example, consider a node with two GPUs, each with 0.5 availability (an aggregated capacity of 1.0). If STRICT_PACK needs to schedule two bundles requiring 0.1 and 0.7 GPUs, the aggregated request is 0.8.

The scheduler sees 0.8 <= 1.0 and incorrectly assumes the node is a valid fit. However, during local allocation, the node actually can't fit the 0.7 bundle, resulting in a false positive and unnecessary spillbacks.

Solution

Instead of checking the availability of an aggregated bundle, we evaluate each bundle individually using the following way:

  • We loop through each bundle and check its individual availability.
  • If it fits, we temporarily deduct its usage via SubtractNodeAvailableResources.
  • If any bundle in the group fails to fit, we immediately roll back the successful allocations using AddNodeAvailableResources and reject the node.

Note: Once SubtractNodeAvailableResources is updated in the upcoming PR, we will be able to use it directly to determine whether a node can fit a specific bundle, bypassing the need for a separate NodeResources::IsAvailable check.

Testing

This logic successfully passes StrictPackFractionalUnitResourceTest, which is not included in this PR since it relies on the NodeResources::available data structure changes introduced in the upcoming PR

Related PR

Related to #62005

…aggregating to prevent false positive

Signed-off-by: dancingactor <s990346@gmail.com>
@dancingactor
dancingactor requested a review from a team as a code owner April 4, 2026 10:54
@dancingactor dancingactor changed the title [Core][GPU fraction][3/n] Change StrictPack to check availability per-bundle instead of aggregating to prevent false positive [Core][GPU fraction][3/n] Change StrictPack to check availability per-bundle instead of aggregated bundle to prevent false positive Apr 4, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the BundleStrictPackSchedulingPolicy to perform per-bundle resource checks, avoiding the limitations of aggregated resource requests. Feedback highlights that pre-filtering of candidate nodes using the aggregated request still occurs and should be removed to ensure the fix is effective. Other recommendations include using RAY_CHECK for consistency when restoring resources and optimizing the allocated_requests vector with reserve.

Comment thread src/ray/raylet/scheduling/policy/bundle_scheduling_policy.cc
Comment thread src/ray/raylet/scheduling/policy/bundle_scheduling_policy.cc
Comment thread src/ray/raylet/scheduling/policy/bundle_scheduling_policy.cc
Comment thread src/ray/raylet/scheduling/policy/bundle_scheduling_policy.cc
Signed-off-by: dancingactor <s990346@gmail.com>
@ray-gardener ray-gardener Bot added core Issues that should be addressed in Ray Core community-contribution Contributed by the community labels Apr 4, 2026
- add RAY_CHECK for AddNodeAvailableResources

Signed-off-by: dancingactor <s990346@gmail.com>
@Yicheng-Lu-llll Yicheng-Lu-llll self-assigned this Apr 4, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 0ae717d. Configure here.

Comment thread src/ray/raylet/scheduling/policy/bundle_scheduling_policy.cc
@dancingactor dancingactor changed the title [Core][GPU fraction][3/n] Change StrictPack to check availability per-bundle instead of aggregated bundle to prevent false positive [WIP][Core][GPU fraction][3/n] Change StrictPack to check availability per-bundle instead of aggregated bundle to prevent false positive Apr 4, 2026
Signed-off-by: dancingactor <s990346@gmail.com>
@dancingactor dancingactor changed the title [WIP][Core][GPU fraction][3/n] Change StrictPack to check availability per-bundle instead of aggregated bundle to prevent false positive [Core][GPU fraction][3/n] Change StrictPack to check availability per-bundle instead of aggregated bundle to prevent false positive Apr 5, 2026
@Yicheng-Lu-llll

Copy link
Copy Markdown
Member

Thank you! Please also consider adding a performance changes section in the description.

Comment thread src/ray/raylet/scheduling/policy/bundle_scheduling_policy.cc
@github-actions

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

You can always ask for help on our discussion forum or Ray's public slack channel.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

@github-actions github-actions Bot added the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Apr 20, 2026
@MengjinYan MengjinYan added unstale A PR that has been marked unstale. It will not get marked stale again if this label is on it. and removed stale The issue is stale. It will be closed within 7 days unless there are further conversation labels Apr 27, 2026
@dancingactor dancingactor changed the title [Core][GPU fraction][3/n] Change StrictPack to check availability per-bundle instead of aggregated bundle to prevent false positive [WIP][Core][GPU fraction][3/n] Change StrictPack to check availability per-bundle instead of aggregated bundle to prevent false positive Apr 29, 2026
@dancingactor dancingactor changed the title [WIP][Core][GPU fraction][3/n] Change StrictPack to check availability per-bundle instead of aggregated bundle to prevent false positive [WIP][Core][GPU fraction] Change StrictPack to check availability per-bundle instead of aggregated bundle to prevent false positive May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution Contributed by the community core Issues that should be addressed in Ray Core unstale A PR that has been marked unstale. It will not get marked stale again if this label is on it.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants