search

kubernetes-sigs / kueue

Kubernetes-native Job Queueing
https://kueue.sigs.k8s.io
Apache License 2.0
1.49k stars 268 forks source link

Add FlavorAssignmentStrategy or flavor priorities to ClusterQueue #312

Open alculquicondor opened 2 years ago

alculquicondor commented 2 years ago

What would you like to be added:

A field in ClusterQueueSpec to influence how flavors are selected when there is already some quota in use.

Why is this needed:

Currently, we strictly go in the order determined by the ClusterQueue resource. But cluster administrators might need different optimization criteria, for example:

Completion requirements:

This enhancement requires the following artifacts:

The artifacts should be linked in subsequent comments.

alculquicondor commented 2 years ago

/kind api-change

kerthcet commented 2 years ago

It seems like it's part of https://github.com/kubernetes-sigs/kueue/issues/171, I'd like to take a look so we can release 0.2.0 soon. /assign

kerthcet commented 2 years ago

Sorry, it actually should be https://github.com/kubernetes-sigs/kueue/issues/328, but I can also take a look of this.

alculquicondor commented 2 years ago

This is not part of #328. Yes, it will need to be validated, but we don't have this field yet.

kerthcet commented 2 years ago

I thought about it a little, I think minimizeBorrowing should be the default policy we should follow, we can borrow resources but we should avoid it if possible for sharing between clusterQueues can be break up.

Besides, rather than prioritize the flavors by the slice order, maybe we should a new field priority, this is a more common style and easy to organize the configurations. And yes, this should also be a default policy.

I don't know whether we should introduce policies like binpack/spread/balanced to FlavorAssignmentStrategy(or FlavorManageStrategy), if we do, it seems we reinvent the wheels in scheduling. I think we can let this fly for a while until community users really need it.

kerthcet commented 2 years ago

cc @alculquicondor for thoughts

alculquicondor commented 2 years ago

We can only change the default behavior if we introduce a new API version. We can probably do it as we launch v1beta1.

maybe we should a new field priority

I don't think that adds much value. Going in order is a clear API.

I don't know whether we should introduce policies like binpack/spread/balanced to FlavorAssignmentStrategy

As long as there is a valid use case, we can consider it. Otherwise, I would start with the obvious ones: InOrder, MinimizeBorrowing.

kerthcet commented 2 years ago

maybe we should a new field priority

I don't think that adds much value. Going in order is a clear API.

They can't express the meaning that two flavors are of the same priority. This is useful when we want to apply more policies on flavors, like minimizeBorrowing by priority.

But as you say, let's wait to see if we really need them.

alculquicondor commented 2 years ago

but if you use minimizeBorrowing, we would first sort by how much they borrow. If there are multiple flavors that borrow the same (or borrow nothing), we fallback to the order in the list. It is comprehensive enough, in my opinion.

kerthcet commented 2 years ago

I think priority should be the first decision factor, then we apply other policies. Whatever, I'll try to implement these two strategies as mentioned and evolve gradually.

alculquicondor commented 2 years ago

I see what you mean. You want to do minimizeBorrowing within a group of flavors. Then, if it doesn't fit, go to the next group of flavors which have lower priority.

To keep some level of "backwards compatibility", we can add a validation rule that priorities can only be decreasing in the list.

kerthcet commented 2 years ago

So the conclusion is: 1) Add a new field Priority to api field Flavor

type Flavor struct {
    Name ResourceFlavorReference `json:"name"`
    Quota Quota `json:"quota"`
        Priority `json:"priority"`
}

2) Add a strategy minimizeBorrowing. (We may consider it as a default policy when graduating to a new version)

3) Add a validation rule that priorities can only be decreasing in the list. (We may cancel this when graduating to a new version?)

Right?

alculquicondor commented 2 years ago
  1. I agree, although probably a pointer.
  2. Do we have another strategy? Once you add priority, minimizeBorrowing seems like the only reasonable option for flavors with the same priority.
  3. I would always keep the validation. It makes the API easier to interpret by a human. Also it would be nice if you don't have to define priorities if you don't need them. Then I'm not sure if we should add default priorities (all zero or decreasing)?

Maybe worth a design doc after all? It doesn't have to be long.

kerthcet commented 2 years ago

Yes, I'll write a simplify KEP for this.

kerthcet commented 2 years ago

FYI: I'm already working on this, but considering the k/k KEP deadline is approaching and I'll have a holiday in Oct., let me focus on k/k first, and turn back ASAP.

alculquicondor commented 2 years ago

yup, k/k deadlines are tight

FYI #401

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

tenzen-y commented 1 year ago

/lifecycle frozen

maaft commented 1 year ago

Hi, any eta for this? I'd really like to use preemption and as far as I see, this is the only blocker left for v0.3! :rocket:

kerthcet commented 1 year ago

This might be after https://github.com/kubernetes-sigs/kueue/pull/532/, and the design may also be changed(TBD) since we changed the API, so likely this will not be included in v0.3(preemption is already big enough), but I'll try my best, Let's see. I'm working on another feature now.

maaft commented 1 year ago

just to confirm: preemption will be included in v0.3 regardless of this feature? That'll be great :)

kerthcet commented 1 year ago

just to confirm: preemption will be included in v0.3 regardless of this feature? That'll be great :)

It's not great, it's my bad... :(

alculquicondor commented 1 year ago

Yes, preemption is independent of this feature. I'm also working on #532 before the release.