KEP-5732: Remove desiredCount from the TAS KEP#5949
KEP-5732: Remove desiredCount from the TAS KEP#594944past4 wants to merge 2 commits intokubernetes:masterfrom
Conversation
k8s-ci-robot
added
the
sig/scheduling
k8s-ci-robot
added
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
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.
4 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