feat: Track pod count as a scalar resource #969
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
📊 Performance Benchmark ResultsComparing PR ( Legend
Raw benchmark dataPR branch: Main branch: |
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
itsomri
force-pushed
the
fix/max-pods-as-resource
branch
7 times, most recently
from
1131ad9 to
39f9e25
Compare
Alternative approach to fixing max pods predicate with releasing pods. Instead of tracking allocated pod count separately, treat pods as a scalar resource similar to CPU and memory. Changes: - Add pods to node allocatable resources (v1.ResourcePods) - Add pods to task resource requirements (1 pod per task, 2 for shared GPU) - Remove explicit max pods check from predicates (resource accounting handles it) - Update test helpers to include pods in node resources - Add test demonstrating releasing pods don't block preemption This approach naturally handles releasing pods because they are tracked in the Releasing resources, making their pod count available for new allocations. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Implement predicate-based pod counting that accurately accounts for GPU group reservation pods. Previously, all shared GPU tasks added +1 to pod count, but the correct behavior is: - First fractional pod on a GPU: +2 pods (task + reservation) - Additional pods on same GPU: +1 pod (task only) Changes: - Remove blanket +1 pod count from pod_info ResReq - Add checkMaxPodsWithGpuGroupReservation predicate that: * Determines if task creates new GPU group using allocation logic * Counts reservation pods being freed when GPU groups fully release * Only adds reservation pod count when new group is created - Export GPU sharing functions for predicate access - Store session in predicates plugin for GPU allocation logic access Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Add comprehensive e2e tests to validate accurate pod counting with GPU sharing and max pods limits: 1. Simple case: Verify preemption works on node at max pods 2. Fractions allocation: Verify fraction pod cannot allocate on node at maxPods-1 (would need maxPods+1 with reservation pod) 3. Proper reservation calculation: Verify fraction can preempt another fraction and reuse the same GPU group reservation pod These tests dynamically read the node's max pod capacity instead of hardcoding values, making them portable across different cluster configurations. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
itsomri
force-pushed
the
fix/max-pods-as-resource
branch
from
6ed007d to
4d5651a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixed max pod tracking on scheduling simulations.
Changes:
Checklist
Breaking Changes
Additional Notes