KEP-5732: Topology-aware workload scheduling #5733
Conversation
k8s-ci-robot
added
the
sig/scheduling
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 44past4 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
k8s-ci-robot
added
kind/kep
| // Level specifies the key of the node label representing the topology domain. | ||
| // All pods within the PodGroup must be colocated within the same domain instance. | ||
| // Examples: "topology.kubernetes.io/rack" | ||
| Level string |
There was a problem hiding this comment.
Let's specify what happens if a PodGroup is replicated (there are multiple PodGroup instances/replicas).
I'm assuming that for each of those domain at that level is chosen, but there is not coordination between those (i.e. different pod group instances may be scheduled in different domains, but they can also share them).
| type DRAConstraint struct { | ||
| // ResourceClaimName specifies the name of a specific ResourceClaim | ||
| // within the PodGroup's pods that this constraint applies to. | ||
| ResourceClaimName *string |
There was a problem hiding this comment.
What does ResourceClaimName mean if a given PodGroup is replicated (there are multiple podgroup instances/replicas)?
This would effectively mean sharing the same RC across multiple instances, which in many cases would be highly misleading.
However, arguably there can be usecases for it too, but then the algorithm effectively should consider all podgroup instances in a single round, but for that we don't know how many groups we even have.
@macsko - FYI (as this is slightly colliding with the kep-4671 update)
So thinking about that more, I'm wondering if we can introduce that without further enhancing the API now (i.e. adding the replicas field to PodGroup).
Another alternative would be to very explicitly split the pod-group-replica constraints from the constraints across all pod-group-replicas and (at least for Alpha) focus only on the former.
So something more like (exact names and structures to be refined):
type PodGroupAllReplicasSchedulingConstraints {
ResourceClaimName *string // This one is supported only if Replicas=1
}
type PodGroupReplicaSchedulingConstraints {
ResourceClaimTemplateName *string // Separate RC is created from this template for every replica.
}
There was a problem hiding this comment.
In case if the PodGroup is replicated the meaning of ResourceClaimName will depend on whether we will be scheduling those replicas together or not. If they will be scheduled separately then scheduling of the first replica will lock the referenced ResourceClaim and the subsequent replicas will not have any freedom when it comes to its allocation - there will be only one possible placement for them. When scheduling multiple replicas at once we can try to choose a DRA allocation which allows us to schedule the highest number of replicas (assuming that we do not provide all-or-nothing semantics for multiple replicas).
There was a problem hiding this comment.
I wasn't asking about the implementation aspect.
I wanted to take a step back and understand what is the actual usecase we're trying to address and figure out if/how we should represent it to make it intuitive to users when they have replicated PodGroup. I feel that the API as currently described can be pretty confusing in this case.
|
|
||
| // ResourceClaimTemplateName specifies the name of a ResourceClaimTemplate. | ||
| // This applies to all ResourceClaim instances generated from this template. | ||
| ResourceClaimTemplateName *string |
There was a problem hiding this comment.
Who creates and manages lifecycle of the RC created from that template?
There was a problem hiding this comment.
Here we are assuming that the lifecycle of RC is managed outside of kube-scheduler. One option for this is to have it managed by the specific workload controller like for instance LeaderWorkerSet which could create a RC when creating a new replica. This would be very inconvenient so probably we should have a single controller which could do this just by watching Workload objects. We had a discussion with @johnbelamaric about this. Either way this should be outside of the scope of this KEP.
There was a problem hiding this comment.
The lifecycle is what I plan to address in #5729.
There was a problem hiding this comment.
OK - so this matches my thinking.
But the primary question now is - why do we need it then?
If we have some external entity (whether it's dedicated controller or e.g. LWS controller) that will create RC whenever it is needed (it should create it before we will actually do the scheduling), then what scheduler really needs to be aware and is an input for it is that RC (that it will be finding a best allocation for) not the template itself. It doesn't care about the template.
So I think we're aligned on the intention, but I don't really understand how that will be used.
| // PodGroupInfo holds information about a specific PodGroup within a Workload, | ||
| // including a reference to the Workload, the PodGroup's name, and its replica index. | ||
| // This struct is designed to be extensible with more fields in the future. | ||
| type PodGroupInfo struct { |
There was a problem hiding this comment.
PodGroupInfo was already introduced in scheduler as part of initial gang-scheduling implementation. However, this is now focused on the pods and their state:
https://github.com/kubernetes/kubernetes/blob/master/pkg/scheduler/backend/workloadmanager/podgroupinfo.go#L52
Do you suggest reuse that structure or create a second one for it?
@macsko
There was a problem hiding this comment.
The existing PodGroupInfo is much closer to the PodSetInfo proposed below so if we will consider using it we should probably rename the proposed PodGroupInfo to something like PodGroupMetadata or WorkloadPodGroupRefrence.
There was a problem hiding this comment.
It's closer regarding what kind of information it keeps, but the granularity is different (it may contain pods of different signatures).
So my point is - we need to align that. Having two different things with same names will be pretty misleading.
| // PodSetInfo holds information about a specific PodSet within a PodGroup, | ||
| // primarily the list of Pods. | ||
| // This struct is designed to be extensible with more fields in the future. | ||
| type PodSetInfo struct { |
There was a problem hiding this comment.
Should PodGroupInfo keep a list of its PodSetInfos?
There was a problem hiding this comment.
In my mind PodGroupInfo should only provide basic information about the PodGroup configuration and not the actual state of the PodGroup/pods which are part of this PodGroup so I do not see the need to have a reference from PodGroupInfo to PodSetInfos.
There was a problem hiding this comment.
Should PodSetInfo then have a link to PodGroupInfo?
I think we will need a way to iterate over podsets within podgroup and this seems like a good place to allow for it.
| // PodSetAssignment represents the assignment of pods to nodes within a PodSet for a specific Placement. | ||
| type PodSetAssignment struct { | ||
| // PodToNodeMap maps a Pod name (string) to a Node name (string). | ||
| PodToNodeMap map[string]string |
There was a problem hiding this comment.
Do we need dra assignments too?
There was a problem hiding this comment.
This is a good question. We might need them for the PodGroup pods binding phase which comes after the selection of the placement for a PodGroup has been finished. So provided that we can capture those when we are checking the placement feasibility then yes, we should have DRA assignments here as well.
| // DRA's AllocationResult from DRAAllocations. | ||
| // All pods within the PodSet, when being evaluated against this Placement, | ||
| // are restricted to the nodes matching this NodeAffinity. | ||
| NodeAffinity *corev1.NodeAffinity |
There was a problem hiding this comment.
Implementation detail - given NodeAffinity, finding the nodes that match it is O(N) operation with N being the set of all nodes in the cluster. We together with NodeAffinity here, we should probably also store the exact list of nodes to avoid recomputing it over and over again.
| with fallbacks (e.g., prefer Rack, fallback to Block). This would introduce | ||
| a Rank field to the Placement struct. | ||
|
|
||
| 2. **Optional/Preferred Scheduling Constraints:** Constraints that serve purely |
There was a problem hiding this comment.
I would make it out-of-scope for this KEP even for further releases - we can do that in a follow-up KEP if needed.
| 2. **Optional/Preferred Scheduling Constraints:** Constraints that serve purely | ||
| as scoring mechanisms without hard requirements. | ||
|
|
||
| 3. **Multi-level Scheduling Constraints:** Handling nested constraints (e.g., |
There was a problem hiding this comment.
Same here - let's land TAS in a reasonably small scope first and iterate on that in a followups.
There was a problem hiding this comment.
I guess rephrasing these two comment - I think the extensions generally make sense to me, but I would claim that we should make them explicitly non-goals for this KEP and mark them more as future follow-up extensions.
There was a problem hiding this comment.
That makes sense I will update the description that those are potential extensions for this feature which can be implemented after alpha but they will require separate KEPs.
| Block -> Rack). This would involve iterative placement generation and a | ||
| Parent field in the Placement struct. | ||
|
|
||
| 4. **Pod Group Replicas Support:** Optimizing scheduling for identical |
There was a problem hiding this comment.
The way it's described it's not really pod group replicas support - we need to support pod group replicas from the very beginning.
What you're saying here is that we can optimize the scheduling latency for them, right?
There was a problem hiding this comment.
Yes, this is correct. Replicas will be supported but they will be scheduled one by one. With some changes to the algorithm we can try to optimize this process but this is out of scope for this KEP. I will rephrase it to make it clear.
|
|
||
| - **Selection:** Select the Placement with the highest score. | ||
|
|
||
| - **Binding:** Proceed to bind pods to the assigned nodes and resources. |
There was a problem hiding this comment.
We might need to move the pods once again through their pod-by-pod cycles. They have to call Reserve, Permit etc. to move to the binding successfully. Will the expected placement be expressed using NominatedNodeNames or differently?
| // -- Add other fields below for future extensions -- | ||
| } | ||
|
|
||
| // PodSetInfo holds information about a specific PodSet within a PodGroup, |
There was a problem hiding this comment.
Can you define in the KEP what is a PodSet and any information about homogeneity?
| // ResourceClaimName specifies the name of a specific ResourceClaim | ||
| // within the PodGroup's pods that this constraint applies to. | ||
| ResourceClaimName *string | ||
|
|
||
| // ResourceClaimTemplateName specifies the name of a ResourceClaimTemplate. | ||
| // This applies to all ResourceClaim instances generated from this template. | ||
| ResourceClaimTemplateName *string |
There was a problem hiding this comment.
How do these fields relate to the ResourceClaim references that Pods already have? What happens if the sets of claims referenced by a Workload and its Pods are different?
There was a problem hiding this comment.
+1 to this question, it needs to be answered here
| type PodGroupSchedulingConstraints struct { | ||
| // TopologyConstraints specifies desired topological placements for all pods | ||
| // within this PodGroup. | ||
| TopologyConstraints []TopologyConstraint |
There was a problem hiding this comment.
Does multiple topology constraints actually make sense here? What would be the usecase for it?
| type DRAConstraint struct { | ||
| // ResourceClaimName specifies the name of a specific ResourceClaim | ||
| // within the PodGroup's pods that this constraint applies to. | ||
| ResourceClaimName *string |
There was a problem hiding this comment.
I wasn't asking about the implementation aspect.
I wanted to take a step back and understand what is the actual usecase we're trying to address and figure out if/how we should represent it to make it intuitive to users when they have replicated PodGroup. I feel that the API as currently described can be pretty confusing in this case.
|
|
||
| // ResourceClaimTemplateName specifies the name of a ResourceClaimTemplate. | ||
| // This applies to all ResourceClaim instances generated from this template. | ||
| ResourceClaimTemplateName *string |
There was a problem hiding this comment.
OK - so this matches my thinking.
But the primary question now is - why do we need it then?
If we have some external entity (whether it's dedicated controller or e.g. LWS controller) that will create RC whenever it is needed (it should create it before we will actually do the scheduling), then what scheduler really needs to be aware and is an input for it is that RC (that it will be finding a best allocation for) not the template itself. It doesn't care about the template.
So I think we're aligned on the intention, but I don't really understand how that will be used.
| // ResourceClaimName specifies the name of a specific ResourceClaim | ||
| // within the PodGroup's pods that this constraint applies to. | ||
| ResourceClaimName *string | ||
|
|
||
| // ResourceClaimTemplateName specifies the name of a ResourceClaimTemplate. | ||
| // This applies to all ResourceClaim instances generated from this template. | ||
| ResourceClaimTemplateName *string |
There was a problem hiding this comment.
+1 to this question, it needs to be answered here
| // PodGroupInfo holds information about a specific PodGroup within a Workload, | ||
| // including a reference to the Workload, the PodGroup's name, and its replica index. | ||
| // This struct is designed to be extensible with more fields in the future. | ||
| type PodGroupInfo struct { |
There was a problem hiding this comment.
It's closer regarding what kind of information it keeps, but the granularity is different (it may contain pods of different signatures).
So my point is - we need to align that. Having two different things with same names will be pretty misleading.
| // PodSetInfo holds information about a specific PodSet within a PodGroup, | ||
| // primarily the list of Pods. | ||
| // This struct is designed to be extensible with more fields in the future. | ||
| type PodSetInfo struct { |
There was a problem hiding this comment.
Should PodSetInfo then have a link to PodGroupInfo?
I think we will need a way to iterate over podsets within podgroup and this seems like a good place to allow for it.
|
|
||
| // DRAConstraints specifies constraints on how Dynamic Resources are allocated | ||
| // across the PodGroup. | ||
| DRAConstraints []DRAConstraint |
There was a problem hiding this comment.
Continuing my thoughts from other comments here.
The primary goal that we wanted to ensure with this KEP are:
- building the foundations for TAS and having the first version of the algorithm
- proving that the algorithm is compatible with both DRA and topology-based requirements
I think this KEP is achieving it.
However, the more I think about it, the more concerns I have about this kind of API. Up until now I thought that we can actually decouple and postpone the discussion of lifecycle of pod-group-owned (or workload-owned) RCs to later, but some of my comments below already suggest it's not that clear and may influence the API.
So I actually started thinking if (for the sake of faster and incremental progress), we shouldn't slightly revise the scope and goals of this KEP, in particular:
- remove the "DRAConstraints" from the scope (and couple it the lifecycle of PodGroup/RC discussion we'll have in DRA: ResourceClaim Support for Workloads #5729 - @nojnhuh )
- ensure that the proposal is compatible with DRA-based constraints at lower level;
namely, scheduler should not really manage the lifecycle of RC and those RC should just be an input to scheduler (whether on PodGroup level, Workload-level or some to-be-introduced level).
So what if instead we would prove that it works by simply:
- ensuring that some internal interface in scheduler (or maybe a scheduler-framework level one?) can actually accept RCs as an additional constraint to the WorkloadCycle
- we add a test at that level, that scheduling works if we pass topology constraints as RCs
That would allow us to decouple the core of the changes in that KEP from all the discussions about how to represent it in the API, how is it coupled with lifecycle etc. And hopefully unblock this KEP much faster and still proving the core of what we need.
@johnbelamaric @erictune @44past4 @dom4ha @sanposhiho @macsko - for your thoughts too
There was a problem hiding this comment.
I think that makes sense. Decoupling can help execution. We would treat the lifecycle and allocation of RCs in #5729. Allocation implies the constraint. #5194 should also merge with #5729, I think. It was conceived prior to the existence of the Workload API and I think #5729 encompasses a more holistic set of functionality.
| - **Action:** Iterate over distinct values of the topology label (TAS) or | ||
| available ResourceSlices (DRA). | ||
|
|
||
| - **Output:** A list of Placement objects. |
There was a problem hiding this comment.
Will this phase generate all possible placements or only a subset of those? To reduce scheduling latency, in pod-by-pod scheduling we limit the number of feasible nodes to process (percentageOfNodesToScore setting). It may be important if the placements could overlap.
There was a problem hiding this comment.
This phase is assumed to generate all possible placements. However what we could do is to stop checking them in phase 2 if enough feasible placements has been found. The minimum number of feasible placements could be configurable and it could limit the scheduling latency in case of large number of placements. However I would treat this as a future optimization which could be done after some scalability testing.
|
/assign I'm a small bandwidth-ed these days, but will take a look at this one for sure.. |
| - **State:** Temporarily assigns AllocationResults to ResourceClaims during | ||
| the Assume phase. | ||
|
|
||
| **PlacementBinPackingPlugin (New)** Implements `PlacementScorer`. Scores |
There was a problem hiding this comment.
I think this plugin can prevent the current PodGroup from fragmenting larger Levels, but it cannot prevent the current PodGroup from fragmenting smaller levels. If the current podgroup uses fewer than all the nodes in this Placement, then there could be multiple podsAssignment options, and different options may have different fragmentation effects. Since pod-at-a-time scheduling within the Placement is greedy, we won't consider multiple podsAssignment options.
Its not clear to me that you can influence this enough using the per-pod Score plugins.
| Name() string | ||
|
|
||
| // GeneratePlacements generates a list of potential Placements for the given PodGroup and PodSet. | ||
| // Each Placement represents a candidate set of resources (e.g., nodes matching a selector) |
There was a problem hiding this comment.
Consider saying that the GeneratePlacements interface does not have any compatibility guarantees across versions. If/when we later add Prioritized Placement Scheduling, or Multi-level Scheduling Constraints, we will want to change GeneratePlacements.
| // Placement represents a candidate domain for scheduling a PodSet. | ||
| // It defines a set of nodes and/or proposed Dynamic Resource Allocation (DRA) | ||
| // resource bindings necessary to satisfy the PodSet's requirements within that domain. | ||
| type Placement struct { |
There was a problem hiding this comment.
What is the valid lifetime of a Placement object? Is it only one cycle of Workload Cycle?
If so, then we don't need to worry about updating Placements when we update our view of the cluster.
Maybe state this.
|
|
||
| 5. **Explicit Topology Definition:** Using a Custom Resource (NodeTopology) to | ||
| define and alias topology levels, removing the need for users to know exact | ||
| node label keys. |
There was a problem hiding this comment.
Explicit Topology Information also provided these things:
- An explicit total order on levels within one Topology object (needed for Multi-level and Prioritized Placement Scheduling)
- An implicit label hierarchy requirement
- A level
nlabel's nodes must be a subset of only one leveln+1label. - Useful for Multi-level placement, and for the hierarchical aggregated capacity optimization.
- A level
- A way to limit the number of levels
- Limit by validating the list length in a Topology object.
- Limiting levels limits one term of algorithm complexity.
- A way to discourage creation of too many Topology objects
- Only admins or cloud providers should create these usually.
Taken together, these properties make it easier to avoid the case where there are many more TAS-relevant labels (key/value pairs) than there are nodes.
Also, while the initial algorithm is going to be greedy, in the sense that it examines one workload at a time, future algorithms may want to examine multiple workloads at once to find jointly optimal placements. By allowing excess complexity in the structure of topology labels at the outset, we will limit our ability to do future global optimizations.
I think it is fine to leave Explicit Topology Definition out of Alpha. However, before GA, we should either have beta Explicit Topology Definition, or have documented requirement for (1) the maximum number of label keys used for TAS, (2) partial order requirement over all TAS keys, and (3) nesting requirement for TAS labels.
Otherwise, it will be hard to enforce those later.
|
|
||
| // DRAConstraints specifies constraints on how Dynamic Resources are allocated | ||
| // across the PodGroup. | ||
| DRAConstraints []DRAConstraint |
8 participants
One-line PR description: Initial version of Topology-aware workload scheduling KEP
Issue link: Topology-aware workload scheduling #5732
/sig scheduling