Repository navigation
[YUNIKORN-3237] DS Preemption: Improve victim selection algorithm - #1184
Open
PoiBlackTea wants to merge 2 commits into
Open
PoiBlackTea wants to merge 2 commits into
PoiBlackTea wants to merge 2 commits into
Conversation
Sort required node preemption victims with SortAllocationsBasedOnAsk, using the node capacity as total, and remove the now unused SortAllocations. Update TestGetVictims and TestPlaceholderDataWithPlaceholderPreemption for the new victim order.
PoiBlackTea
marked this pull request as ready for review
October 6, 2026 14:36
rjgoyln
reviewed
Oct 7, 2026
rjgoyln
left a comment
rjgoyln
left a comment
Contributor
There was a problem hiding this comment.
I left two notes below on BestEffort victims and test coverage for the new sort inputs.
Please feel free to resolve them if they’re not applicable.
HuangTing-Yao
self-requested a review
October 8, 2026 10:21
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1184 +/- ##
==========================================
+ Coverage 83.27% 83.33% +0.06%
==========================================
Files 104 104
Lines 14460 14622 +162
==========================================
+ Hits 12041 12185 +144
- Misses 2131 2142 +11
- Partials 288 295 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Use the resource types the node is short of as the sort reference, and add TestSortAllocations to cover the node capacity and shortfall inputs.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 switches DS (required node) preemption to
SortAllocationsBasedOnAsk, same as YUNIKORN-3236 did for inter-queue preemption. DS preemption only works on a single node, so I pass the node capacity astotal. After thisSortAllocationshas no callers left, so I removed it together with its test.Test changes:
TestGetVictimsstill picks the same four victims, just in the new order.TestPlaceholderDataWithPlaceholderPreemptionnow creates the first five placeholders an hour earlier, so the last one is still the youngest victim.Type of change
Jira issue
Jira ID : https://issues.apache.org/jira/browse/YUNIKORN-3237
AI Tooling
If an AI tool was used:
Check https://www.apache.org/legal/generative-tooling.html for details.
How has this been tested?
make test_allwas run, and no failures reported.Questions:
release-noteslabel.Screenshots or other details
NA