-
Notifications
You must be signed in to change notification settings - Fork 206
feat: add DeadlinePriority plugin in intra-flow dispatch policy #1960
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?
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: googs1025 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 |
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@LukeAVanDrie |
| return DeadlinePriorityPolicyName | ||
| } | ||
|
|
||
| // RequiredQueueCapabilities returns an empty slice, indicating that this policy can operate with any queue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: likely copy-paste error
|
Initial pass LGTM, but I will take a deeper look first thing Monday morning. Thanks for contributing! I want to double check how I default EffectiveTTL if undefined. Regarding QueueCapability pairings, this will be validated automatically by the Flow Registry. We guarantee the paired queue / policy are compatible, so no need to document this in your go-docs. The RequiredCapabilities function is sufficient. As you may have noticed, pretty much every IntraFlow impl will just return PeekHead. When I align this with the actual EPP plugin model, I will probably make the official plugin the comparator, not the policy impl (which is just imperative code boilerplate). I can handle migrating your policy then. There should be a README.md which gives a (slightly outdated) policy overview. It also described how to get auto-test coverage for the black box contracts that the orchestrating engine relies upon. Basically, you'll get some free test coverage simply by registering your policy. |
|
Nit: An alternative policy name here may be Earliest Deadline First (EDF). This is the conventional name and a bit more expressive. If GAIE ever supports TTFT latency targets we can use the strictest deadline (TTL vs target, if defined) as the deadline. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Introduce a new intra-flow dispatch policy named "DeadlinePriority" that
schedules requests based on deadline urgency, computed as EnqueueTime +
EffectiveTTL. Requests without a valid TTL are treated as lowest priority
and ordered by FCFS for fairness.
Which issue(s) this PR fixes:
Fixes #1913
Does this PR introduce a user-facing change?: