-
Notifications
You must be signed in to change notification settings - Fork 76
Refactor: Externalize Scheduler's saturation logic and criticality-based service differentiation #805
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Refactor: Externalize Scheduler's saturation logic and criticality-based service differentiation #805
Conversation
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: LukeAVanDrie The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hi @LukeAVanDrie. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/ok-to-test |
This change necessitates regression testing. It should be no-op, but I will get quantitative data to confirm that. |
I split out the addition of the saturation detector subdir into a separate PR to be submitted before this one (#808 ). It is just unused until this PR gets submitted, wiring it up. |
112b943
to
48cc9a0
Compare
a3d9090
to
9d273fa
Compare
9d273fa
to
83486ac
Compare
This commit refactors the request processing pipeline, externalizing saturation detection and criticality-based service differentiation from the Scheduler. These responsibilities are now primarily managed by the RequestControl.Director. This change is a preparatory step for the introduction of a new Flow Controller component, which will eventually absorb these admission control duties. Key changes include: - Introduced `PreDispatch` method to `RequestControl.Director` It utilizes the `SaturationDetector` for admission control of non-critical requests and handles request criticality to determine if saturation checks are bypassed. - The saturation detection logic for dropping non-critical requests is intentionally preserved within the `Director` at this stage. This allows the option to bypass the future Flow Controller component during its maturation, ensuring the existing saturation and sheddable request behavior can be maintained as a fallback. - Simplified the `Scheduler` to focus solely on preference-based filtering and pod selection for requests that have already been admitted by the `Director`. - Removed the `SheddableRequestFilter` and the distinct critical/sheddable filter paths from the `Scheduler`'s internal logic. The `Scheduler` now applies a single, unified preference filter chain to all incoming requests. - Updated `main.go` to instantiate the `SaturationDetector`, wiring it into the request handling flow. - Updated tests across `scheduler_test.go`, `director_test.go`, and `filter_test.go` to align with the new component responsibilities, adding additional coverage where necessary. This refactoring leads to a cleaner architecture, making the `Scheduler` a more focused component and centralizing initial admission control logic, while paving the way for the future Flow Controller. This is aligned with the direction in `0683-epp-architecture-proposal` and should be nearly no-op in terms of EPP behavior.
83486ac
to
4a7de3f
Compare
@LukeAVanDrie: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
This commit refactors the request processing pipeline, externalizing saturation detection and criticality-based service differentiation from the Scheduler. These responsibilities are now primarily managed by the RequestControl.Director.
This change is a preparatory step for the introduction of a new Flow Controller component, which will eventually absorb these admission control duties.
Diff base is: #808 (split out for easier reviewing)
Related to: #674
Key changes include:
PreDispatch
method toRequestControl.Director
. It utilizes theSaturationDetector
for admission control of non-critical requests and handles request criticality to determine if saturation checks are bypassed.Director
at this stage. This allows the option to bypass the future Flow Controller component during its maturation, ensuring the existing saturation and sheddable request behavior can be maintained as a fallback.Scheduler
to focus solely on preference-based filtering and pod selection for requests that have already been admitted by theDirector
.SheddableRequestFilter
and the distinct critical/sheddable filter paths from theScheduler
's internal logic. TheScheduler
now applies a single, unified preference filter chain to all incoming requests.main.go
to instantiate theSaturationDetector
, wiring it into the request handling flow.scheduler_test.go
,director_test.go
, andfilter_test.go
to align with the new component responsibilities, adding additional coverage where necessary.This refactoring leads to a cleaner architecture, making the
Scheduler
a more focused component and centralizing initial admission control logic, while paving the way for the future Flow Controller.This is aligned with the direction in
0683-epp-architecture-proposal
and should be nearly no-op in terms of EPP behavior.