KEP-5732: Remove desiredCount from the TAS KEP#5949
KEP-5732: Remove desiredCount from the TAS KEP#5949k8s-ci-robot merged 2 commits intokubernetes:masterfrom
Conversation
k8s-ci-robot
added
the
sig/scheduling
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
the
size/M
| ```go | ||
| // BasicSchedulingPolicy indicates that standard Kubernetes | ||
| // scheduling behavior should be used. | ||
| type BasicSchedulingPolicy struct { |
There was a problem hiding this comment.
We know that TAS for non-gangs is tricky due to edge cases (e.g., the scheduler making placement decisions based on a partial view of the pods). I treat this as an (unsuccessful) attempt to fix the problem. While removing it now makes sense until we have a more thorough design, I think we shouldn't just drop the context entirely.
Instead of simply removing it, should we add a short paragraph to the KEP outlining the current TAS limitations for basic policies, noting that this will be addressed in future releases? We could also mention scheduling gates as a currently available (albeit imperfect) mitigation.
The 'Phase 2' algorithm section might be a good place for this. We could expand on point '3. If all pods fit, the Placement is marked Feasible' to explain why this works perfectly for gangs (waiting for MinCount in pre-enqueue), but might behave inconsistently for basic policies (where the scheduler's decision depends on the number of observed pods).
There was a problem hiding this comment.
Instead of simply removing it, should we add a short paragraph to the KEP outlining the current TAS limitations for basic policies, noting that this will be addressed in future releases? We could also mention scheduling gates as a currently available (albeit imperfect) mitigation.
+1, but I think only the problem of "not-ready" pods is worth addressing as a part of TAS as it makes the feature working sub-optimally.
Addressing the desired future number of pods is rather an independent extension to TAS and IMHO deserves a separate KEP.
There was a problem hiding this comment.
I have updated Phase 2 algorithm description with a point explaining the limitations of using TAS with Basic Scheduling Policy. Please take a look.
44past4
force-pushed
the
remove-desired-count
branch
from
7bf0cc3 to
54508a8
Compare
| ```go | ||
| // BasicSchedulingPolicy indicates that standard Kubernetes | ||
| // scheduling behavior should be used. | ||
| type BasicSchedulingPolicy struct { |
There was a problem hiding this comment.
Instead of simply removing it, should we add a short paragraph to the KEP outlining the current TAS limitations for basic policies, noting that this will be addressed in future releases? We could also mention scheduling gates as a currently available (albeit imperfect) mitigation.
+1, but I think only the problem of "not-ready" pods is worth addressing as a part of TAS as it makes the feature working sub-optimally.
Addressing the desired future number of pods is rather an independent extension to TAS and IMHO deserves a separate KEP.
|
FTR, dropped PR with the discussion: kubernetes/kubernetes#137137 |
Add warning about using the Basic Scheduling Policy with TAS.
| - **Potential Optimization:** Pre-filtering can check aggregate resources | ||
| requested by PodGroup Pods before running the full simulation. | ||
|
|
||
| - **Basic Scheduling Policy Handling:** The current algorithm may exhibit |
There was a problem hiding this comment.
The same applies to Gang Policy if more pods than minCount are created.
There was a problem hiding this comment.
Yes, this is true that we have a similar problem with Gang Policy but in case of Gang Policy we have explicit information on minCount coming from the user and it is their conscious decision if they are setting minCount to lower number compared to total number of pods. So in the context of Gang Policy TAS adheres to explicit user intent and makes sure that it is fulfilled. For Basic scheduling policy user may expect some best effort behaviour from TAS which in many cases will simply not be true so this disclaimer makes sense for me for Basic Policy but not necessary for Gang Policy.
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 44past4, dom4ha The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
5 participants
One-line PR description: Removes adding desiredCount field to Basic and Gang PodGroup scheduling policies from the scope of TAS KEP.
Issue link: KEP-5732: Topology-aware workload scheduling #5732
Other comments:
/sig scheduling