karmada icon indicating copy to clipboard operation
karmada copied to clipboard

Propose binding priority and preemption

Open whitewindmills opened this issue 1 year ago • 9 comments

What type of PR is this? /kind design

What this PR does / why we need it: Propose binding priority and preemption

Which issue(s) this PR fixes: Fixes #4938

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

whitewindmills avatar May 28 '24 09:05 whitewindmills

/cc @RainbowMango @XiShanYongYe-Chang

whitewindmills avatar May 28 '24 09:05 whitewindmills

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 48.12%. Comparing base (d4c2793) to head (59a6afd). Report is 1195 commits behind head on master.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4993      +/-   ##
==========================================
- Coverage   53.33%   48.12%   -5.21%     
==========================================
  Files         252      668     +416     
  Lines       20482    55291   +34809     
==========================================
+ Hits        10924    26609   +15685     
- Misses       8836    26948   +18112     
- Partials      722     1734    +1012     
Flag Coverage Δ
unittests 48.12% <ø> (-5.21%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar May 28 '24 09:05 codecov-commenter

Thanks~ /assign

XiShanYongYe-Chang avatar May 29 '24 07:05 XiShanYongYe-Chang

/assign @RainbowMango

whitewindmills avatar Jun 03 '24 06:06 whitewindmills

Hi @whitewindmills I guess it's a good chance to introduce this proposal at tomorrow's community meeting, what do you think?

RainbowMango avatar Jun 17 '24 07:06 RainbowMango

hi all, can we continue this proposal?

whitewindmills avatar Jul 10 '24 02:07 whitewindmills

hi all, can we continue this proposal?

+1

Monokaix avatar Jul 25 '24 01:07 Monokaix

Hi all,

Thank you all for the amazing feedback today! In summary, our design currently only targets the resource that is scheduled in a single cluster, and the preemption only happens for the bindings in one cluster.

Here are the points we agree with in this proposal:

  1. Need to redesign the scheduler queue so that it is aware of the priority and it will pop out the resource binding with highest priority first.
  2. Reuse the native priorityClass and create a new API if we need to extend the functionality in the future.
  3. Resolving the priority value and preemptionPolicy/preemptionBehavior to the resource binding so that the scheduler won’t need to query the priorityClass to find the values.
  4. If scheduling fails due to insufficient resources (or can not find feasible clusters), we should continue attempting to schedule other pending ResourcesBindings instead of blocking the whole scheduling process.
  5. Reschedule the preempted bindings. (Need to carefully consider the backoff time for the preempted bindings to avoid reschedule the preempted bindings before the preemptor binding.)

These are the points where we have different views or additional questions and thoughts:

  1. Could you please clarify if there are any use cases that require binding the priorityClassName with the PropagationPolicy/ClusterPropagationPolicy? Maybe because you don't want to enforce that all the resources must have a priorityClassName filed? We plan to put the priority and preemptionBehavior/preemptionPolicy in the replicaRequirements. So the user can just specify the priorityClassName in the resource and create the resource the same as what they do in the single cluster. We can use custom resource interpreter to propagate the priority and preemptionPolicy to the replicaRequirements.
  2. We propose using scheduler-estimators to find the victim bindings since they have the accurate information of the clusters, and the scheduler will use the victim bindings to decide which bindings to preempt and perform preemption.
  3. If possible, we would like to have an option or feature gate or a field in the binding for controlling if preempted bindings should be rescheduled.

Thanks for your time again! Please feel free to provide any comment. Reference: Our Proposal - https://docs.google.com/document/d/1MixmgLwnmiRrukyFP25JvE2hqiqB3f_JKT12tXiOsbc/edit?tab=t.0 cc @RainbowMango @kevin-wangzefeng @wengyao04 @LeonZh0u

seanlaii avatar Oct 15 '24 20:10 seanlaii

@RainbowMango can this proposal be merged?

whitewindmills avatar Dec 20 '24 06:12 whitewindmills

I want to give it another look and see if any comments from the others.

RainbowMango avatar Dec 20 '24 09:12 RainbowMango

@kevin-wangzefeng @RainbowMango PTAL

whitewindmills avatar Feb 18 '25 07:02 whitewindmills

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

The full list of commands accepted by this bot can be found here.

The pull request process is described here

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

karmada-bot avatar Feb 18 '25 07:02 karmada-bot