[YUNIKORN-2500] Use GetPreemptableResource, GetRemainingGuaranteedRes… #830
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #830 +/- ##
==========================================
- Coverage 79.35% 79.17% -0.18%
==========================================
Files 82 82
Lines 11362 11330 -32
==========================================
- Hits 9016 8971 -45
- Misses 2019 2033 +14
+ Partials 327 326 -1 ☔ View full report in Codecov by Sentry. |
|
Ran the e2e test in my local setup to verify the changes. It is running with out any failures. |
There was a problem hiding this comment.
-1. I know that there is a spreadsheet of behavioral cases that has been created for this to document how the preemption logic should behave in a variety of circumstances. I would absolutely like to see a PR consisting only of test cases for each of the identified scenarios. We can disable the failing ones for now, but that would give us a baseline for how "compliant" the existing preemption logic is. Then YUNIKORN-2500 can be rebased on top of that, re-enabling the tests that it fixes. That will ensure we don't regress compared to the existing behavior.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #830 +/- ##
==========================================
+ Coverage 78.01% 78.22% +0.21%
==========================================
Files 97 97
Lines 12119 12177 +58
==========================================
+ Hits 9455 9526 +71
+ Misses 2354 2346 -8
+ Partials 310 305 -5 ☔ View full report in Codecov by Sentry. |
PR #847 covers all the different test cases but skipped for now. Once the same has been merged, this pr will fix all those test cases with necessary changes. |
| @@ -816,6 +874,29 @@ func ComponentWiseMinPermissive(left, right *Resource) *Resource { | |||
| return out | |||
| } | |||
|
|
|||
| // ComponentWiseMinOnlyExisting Returns a new Resource with the smallest value for resource type | |||
| // existing only in left but not vice versa. | |||
| func ComponentWiseMinOnlyExisting(left, right *Resource) *Resource { | |||
There was a problem hiding this comment.
This is the 3rd version of ComponentWiseMin... need to think about a refactor in a follow up jira
…ource in Preemption flow
…n cases * Fixed lint warnings * Test Code cleanup & Minor changes
manirajv06
force-pushed
the
YUNIKORN-2500
branch
from
f50893b
to
1738928
Compare
…ource in Preemption flow
What is this PR for?
Use GetPreemptableResource, GetRemainingGuaranteedResource instead of old methods in preemption flow. In addition, there are some other changes to ensure the whole flow works as earlier.
What type of PR is it?
Todos
What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-2500
How should this be tested?
Screenshots (if appropriate)
Questions: