fix(epp): Add check for unpopulated load attributes for TTFT load gate - #2652
Open
d0w wants to merge 4 commits into
Open
fix(epp): Add check for unpopulated load attributes for TTFT load gate#2652d0w wants to merge 4 commits into
d0w wants to merge 4 commits into
Conversation
github-actions
Bot
added
area/epp
area/scheduling
size/L
d0w
marked this pull request as ready for review
September 1, 2026 21:11
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are documentation mismatches/typos in updated helper comments and a metrics test gap that fails to assert the new missing_signal outcome label.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes the prefix-cache-affinity-filter TTFT load gate so it no longer compares observed TTFT values against substituted defaults when endpoints are missing load-related attributes, and it surfaces that “blind” state via a dedicated decision outcome.
Changes:
- Update TTFT computation helpers to return
(value, ok)and exclude endpoints with missing TTFT signals from the best-TTFT calculations. - Skip the TTFT load gate and keep the sticky set when either sticky or non-sticky candidates lack the configured TTFT signal, recording a
missing_signaldecision outcome. - Add unit tests and update plugin documentation/README to describe the missing-signal behavior.
File summaries
| File | Description |
|---|---|
| pkg/epp/framework/plugins/scheduling/filter/prefixcacheaffinity/README.md | Documents updated TTFT load gate behavior when TTFT signals are missing. |
| pkg/epp/framework/plugins/scheduling/filter/prefixcacheaffinity/plugin.go | Implements “missing signal” detection by excluding endpoints without TTFT attributes and skipping the gate when either side is blind. |
| pkg/epp/framework/plugins/scheduling/filter/prefixcacheaffinity/plugin_test.go | Adds tests covering missing-signal behavior for predictor and throughput TTFT sources. |
| pkg/epp/framework/plugins/scheduling/filter/prefixcacheaffinity/metrics.go | Adds the missing_signal decision outcome label. |
| pkg/epp/framework/plugins/scheduling/filter/prefixcacheaffinity/metrics_test.go | Extends decision-outcome metric tests with a missing-signal case (but needs a small fix to assert it). |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| }, | ||
| } | ||
|
|
||
| allOutcomes := []string{outcomeSticky, outcomeNoMatch, outcomeLoadOverride, outcomeExploration, outcomeNotApplicable} |
Comment on lines
+269
to
+273
| // endpointTTFT returns the predicted TTFT (ms) for an endpoint, either from the | ||
| // latency predictor or estimated from in-flight tokens and peak prefill | ||
| // throughput. Endpoints missing the required attribute contribute no signal: | ||
| // MaxFloat64 on the predictor path (never the fastest), 0 in-flight tokens on | ||
| // the throughput path (no observed load). | ||
| func (p *Plugin) endpointTTFT(ep fwksched.Endpoint) float64 { | ||
| // the throughput path (no observed load). Returns the ttft and false if predictiond data was missing. |
Comment on lines
289
to
290
| // inFlightTokens returns an endpoint's in-flight token count, or 0 when the | ||
| // attribute is absent (no observed load). |
d0w
force-pushed
the
ttft-missing-signal
branch
from
September 1, 2026 21:27
04234e6 to
873ca80
Compare
Contributor
|
🚨 Unsigned commits detected! Please sign your commits. For instructions on how to set up GPG/SSH signing and verify your commits, please see GitHub Documentation. |
Signed-off-by: Derek Xu <derxu@redhat.com>
Signed-off-by: Derek Xu <derxu@redhat.com>
Signed-off-by: Derek Xu <derxu@redhat.com>
Signed-off-by: Derek Xu <derxu@redhat.com>
d0w
force-pushed
the
ttft-missing-signal
branch
from
September 1, 2026 21:28
873ca80 to
973f0df
Compare
github-actions
Bot
added
kind/bug
Collaborator
|
/assign @LukeAVanDrie |
d0w
changed the title
github-actions
Bot
added
kind/bug
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind bug
What this PR does / why we need it:
Previously, with the prefix-cache-affinity-filter, the load gate compared loaded values against defaults that would cause the paths to fail in opposite directions. When endpoints (whether sticky or not) were missing certain attributes such as
LatencyPredictionInfoorInFlightLoad.Tokensthe substituted value would be arbitrarily 0 orMaxFloat64causing the gate to unwillingly fire or not fire.The implementation changes the TTFT and inFlightToken computation functions to return booleans to determine if the computation is ok or not. If not ok (meaning there was no data), the endpoint is excluded from the bestTTFT calculation. If there was no data from either sticky/non-sticky set, then default to the sticky set. When there is missing data causing a failure toward stickiness, a
missing_signalcount is incremented.Tests were added as well as an adjustment to the README to note this behavior.
Fixes #2650
Release note (write
NONEif no user-facing change):