Skip to content

Commit bc1a915

Browse files
Fix comments and add missing tests.
Signed-off-by: Mohammad <mohammad.nassar@ibm.com>
1 parent 51b331f commit bc1a915

8 files changed

Lines changed: 506 additions & 30 deletions

File tree

pkg/epp/framework/plugins/requestcontrol/dataproducer/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ The framework resolves a DAG from each plugin's `Produces` and `Consumes` declar
3535
- `inflight-load-producer` **optionally** consumes `PrefixCacheMatchInfo` from an approx or precise prefix producer; prefix-discounting is applied automatically when the attribute is present.
3636
- `p2p-source-producer` **requires** `PrefixCacheMatchInfo` from a prefix producer; set `prefixMatchInfoProducerName` to select a non-default producer instance. Omitting it binds the default key, which auto-wires the approximate producer (no error) — set it explicitly for precise-only deployments. Set `prefillProfileName` to match a renamed `disagg-profile-handler` prefill profile.
3737
- `predicted-latency-producer` **optionally** consumes `PrefixCacheMatchInfo`; set `prefixMatchInfoProducerName` in its config to the name of the prefix producer instance.
38-
- `latency-observer-producer` **requires** `InFlightLoad`, so `inflight-load-producer` is ordered ahead of it and auto-created when absent. It must itself be listed under `dataLayer.sources`, which is what drives its recompute; auto-creation alone leaves every endpoint reading cold.
38+
- `latency-observer-producer` **requires** `InFlightLoad`, so `inflight-load-producer` is ordered ahead of it and auto-created when absent. It must itself be listed under `dataLayer.sources`; auto-creation from `latency-observation-scorer`'s required data key only wires the attribute, not the periodic tick that publishes it. See the [producer README](latencyobserver/README.md#configuration).
3939

4040
## Related documentation
4141

pkg/epp/framework/plugins/requestcontrol/dataproducer/latencyobserver/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ It builds on the request-control hooks and publishes statistics as a datalayer a
3030

3131
| hook | what it does |
3232
|---|---|
33-
| `Produce` | Update inflight load at the time of current request |
33+
| `Produce` | Read and pin each candidate's in-flight load, before this request joins it |
3434
| `PreRequest` | Record the endpoint decision to enable capturing endpoint statistics |
3535
| `ResponseBody`, first chunk | Capture TTFT and append it to that endpoint's observation window |
3636
| `Dispatch` | Every `intervalDuration`, recompute the percentiles and publish |
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
/*
2+
Copyright 2026 The llm-d Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package latencyobserver
18+
19+
import (
20+
"context"
21+
"testing"
22+
"time"
23+
24+
"github.com/stretchr/testify/assert"
25+
"github.com/stretchr/testify/require"
26+
)
27+
28+
// testEndpointID is the namespaced name of the endpoint the tests observe.
29+
const testEndpointID = "default/a"
30+
31+
// flushConfig is tuned for tests: a short bucket so the floor path runs, and a
32+
// low minRequests so a handful of observations is enough to be trusted.
33+
func flushConfig() Config {
34+
cfg := DefaultConfig
35+
cfg.WindowSize, cfg.MaxRequests, cfg.MinRequests = 64, 16, 4
36+
cfg.BucketDuration, cfg.BucketHistorySize = "1s", 4
37+
return cfg
38+
}
39+
40+
// flushAt recomputes the endpoint's snapshot at a controlled time. Dispatch
41+
// reads time.Now(), so tests drive publish directly to steer the bucket clock.
42+
func flushAt(p *Observer, now time.Time) {
43+
p.publish(context.Background(), testEndpointID, p.stateFor(testEndpointID), now)
44+
}
45+
46+
func TestResolveConfig(t *testing.T) {
47+
resolved, err := DefaultConfig.resolve()
48+
require.NoError(t, err)
49+
assert.Equal(t, time.Second, resolved.interval)
50+
assert.Equal(t, 100, resolved.maxRequests)
51+
assert.InDelta(t, 0.25, resolved.lowPercentile, 1e-9, "percentiles resolve to fractions")
52+
53+
invalid := map[string]func(*Config){
54+
"zero window": func(c *Config) { c.WindowSize = 0 },
55+
"maxRequests above window": func(c *Config) { c.MaxRequests = c.WindowSize + 1 },
56+
"zero minRequests": func(c *Config) { c.MinRequests = 0 },
57+
"bucket history below two": func(c *Config) { c.BucketHistorySize = 1 },
58+
"percentiles inverted": func(c *Config) { c.LowPercentile, c.TypicalPercentile = 50, 25 },
59+
"unparsable duration": func(c *Config) { c.IntervalDuration = "soon" },
60+
"non-positive duration": func(c *Config) { c.BucketDuration = "-1m" },
61+
}
62+
for name, mutate := range invalid {
63+
t.Run(name, func(t *testing.T) {
64+
cfg := DefaultConfig
65+
mutate(&cfg)
66+
_, err := cfg.resolve()
67+
require.Error(t, err)
68+
})
69+
}
70+
}
71+
72+
func TestFlush(t *testing.T) {
73+
now := time.Now()
74+
75+
t.Run("publishes the load anchors from the short window", func(t *testing.T) {
76+
p := newObserverWithConfig(t, flushConfig())
77+
// TTFT rises with the in-flight count it was dispatched at, which is the
78+
// relationship the scorer's curve captures.
79+
for i := range 10 {
80+
p.record(testEndpointID, 0.1+float64(i)*0.1, int64(i), now.Add(time.Duration(i)*time.Millisecond))
81+
}
82+
83+
flushAt(p, now.Add(time.Second))
84+
85+
s := p.stateFor(testEndpointID).published.Load()
86+
require.NotNil(t, s)
87+
assert.Equal(t, 10, s.RecentRequestCount)
88+
assert.Equal(t, 4, s.CalibrationThreshold)
89+
assert.Less(t, s.LowLoadTTFT, s.TypicalLoadTTFT, "P25 must sit below P50")
90+
assert.Less(t, s.InflightAtLowLoad, s.InflightAtTypicalLoad, "the faster band must be less loaded")
91+
})
92+
93+
t.Run("the floor arrives only once a bucket closes", func(t *testing.T) {
94+
p := newObserverWithConfig(t, flushConfig())
95+
for i := range 10 {
96+
ttft := 1.0
97+
if i < 2 {
98+
ttft = 0.1 // a fast tail the floor must follow
99+
}
100+
p.record(testEndpointID, ttft, 1, now.Add(time.Duration(i)*time.Millisecond))
101+
}
102+
103+
flushAt(p, now) // starts the bucket clock
104+
assert.Zero(t, p.stateFor(testEndpointID).published.Load().FloorTTFT)
105+
106+
// One bucket later exactly: the bucket window looks back bucketDuration,
107+
// so flushing further out would close an empty bucket.
108+
flushAt(p, now.Add(time.Second))
109+
s := p.stateFor(testEndpointID).published.Load()
110+
assert.Greater(t, s.FloorTTFT, 0.0)
111+
assert.Less(t, s.FloorTTFT, 1.0, "tracks the fast requests, not the bulk")
112+
})
113+
114+
t.Run("Floor withholds the value below minRequests", func(t *testing.T) {
115+
p := newObserverWithConfig(t, flushConfig()) // minRequests 4
116+
for i := range 2 {
117+
p.record(testEndpointID, 0.1, 1, now.Add(time.Duration(i)*time.Millisecond))
118+
}
119+
flushAt(p, now)
120+
flushAt(p, now.Add(time.Second))
121+
122+
s := p.stateFor(testEndpointID).published.Load()
123+
assert.Greater(t, s.FloorTTFT, 0.0, "the raw floor is computed")
124+
assert.Zero(t, s.Floor(), "but Floor() withholds it")
125+
})
126+
}
127+
128+
// The datalayer drives the recompute: one Dispatch per endpoint publishes that
129+
// endpoint's snapshot, and the plugin accepts no extractors.
130+
func TestDispatch(t *testing.T) {
131+
ctx := context.Background()
132+
p := newObserverWithConfig(t, flushConfig())
133+
p.record(testEndpointID, 0.3, 1, time.Now())
134+
135+
require.NoError(t, p.Dispatch(ctx, newDataEndpoint()))
136+
assert.NotNil(t, p.stateFor(testEndpointID).published.Load())
137+
assert.Equal(t, time.Second, p.Interval(), "the configured interval is the dispatch cadence")
138+
require.NoError(t, p.Dispatch(ctx, nil), "a nil endpoint is a no-op")
139+
assert.Error(t, p.AppendExtractor(p), "this dispatcher sources nothing")
140+
}

pkg/epp/framework/plugins/requestcontrol/dataproducer/latencyobserver/hooks.go

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package latencyobserver
1818

1919
import (
2020
"context"
21+
"maps"
2122
"time"
2223

2324
"sigs.k8s.io/controller-runtime/pkg/log"
@@ -53,13 +54,21 @@ func (d *dispatchInfo) Clone() fwkplugin.StateData {
5354
return &cp
5455
}
5556

56-
// Produces declares the percentile snapshot this producer publishes, and the
57-
// request-scoped key Produce pins: the endpoint scope rejects undeclared writes.
57+
// inflightAtDispatch is what each candidate was carrying when Produce ran, keyed
58+
// by endpoint ID. It lives in PluginState, not on the endpoint: the data-parallel
59+
// and disaggregated profile handlers rebuild endpoints with empty attribute maps.
60+
type inflightAtDispatch map[string]int64
61+
62+
// Clone implements [fwkplugin.StateData].
63+
func (m inflightAtDispatch) Clone() fwkplugin.StateData {
64+
cp := make(inflightAtDispatch, len(m))
65+
maps.Copy(cp, m)
66+
return cp
67+
}
68+
69+
// Produces declares the percentile snapshot this producer publishes.
5870
func (p *Observer) Produces() map[fwkplugin.DataKey]any {
59-
return map[fwkplugin.DataKey]any{
60-
p.percentilesDataKey: attrlatency.TTFTPercentiles{},
61-
p.inflightAtDispatchDataKey: attrconcurrency.InFlightLoad{},
62-
}
71+
return map[fwkplugin.DataKey]any{p.percentilesDataKey: attrlatency.TTFTPercentiles{}}
6372
}
6473

6574
// Consumes declares the in-flight load as Required, so the DAG runs
@@ -70,19 +79,22 @@ func (p *Observer) Consumes() fwkplugin.DataDependencies {
7079
}
7180
}
7281

73-
// Produce pins each candidate's in-flight load under a request-scoped key, so
74-
// PreRequest sees the load carried before this request landed. Reading it in
75-
// PreRequest instead would race: inflight-load-producer increments its counters
76-
// there too, and hook order between plugins is undefined. Produce is DAG-ordered.
77-
func (p *Observer) Produce(_ context.Context, _ *fwksched.InferenceRequest, endpoints []fwksched.Endpoint) error {
82+
// Produce pins every candidate's in-flight load, so PreRequest sees the load
83+
// carried before this request landed. Reading it there instead would race:
84+
// inflight-load-producer increments its counters in its own PreRequest, and hook
85+
// order between plugins is undefined. Produce is DAG-ordered.
86+
func (p *Observer) Produce(_ context.Context, request *fwksched.InferenceRequest, endpoints []fwksched.Endpoint) error {
87+
if request == nil || request.RequestID == "" || p.PluginState == nil {
88+
return nil
89+
}
90+
pinned := make(inflightAtDispatch, len(endpoints))
7891
for _, endpoint := range endpoints {
7992
if endpoint == nil || endpoint.GetMetadata() == nil {
8093
continue
8194
}
82-
endpoint.Put(p.inflightAtDispatchDataKey, &attrconcurrency.InFlightLoad{
83-
Requests: readInFlightRequests(endpoint, p.inFlightLoadDataKey),
84-
})
95+
pinned[endpoint.GetMetadata().ID.String()] = readInFlightRequests(endpoint, p.inFlightLoadDataKey)
8596
}
97+
p.PluginState.Write(request.RequestID, inflightStateKey, pinned)
8698
return nil
8799
}
88100

@@ -108,14 +120,24 @@ func (p *Observer) PreRequest(ctx context.Context, request *fwksched.InferenceRe
108120
log.FromContext(ctx).V(logutil.DEBUG).Info("Skipping TTFT tracking: no request ID or no primary target")
109121
return nil
110122
}
123+
endpointID := endpoint.GetMetadata().ID.String()
111124
p.PluginState.Write(request.RequestID, dispatchStateKey, &dispatchInfo{
112-
endpointID: endpoint.GetMetadata().ID.String(),
113-
inflight: readInFlightRequests(endpoint, p.inflightAtDispatchDataKey),
125+
endpointID: endpointID,
126+
inflight: p.pinnedInflight(request.RequestID, endpointID),
114127
dispatchedAt: time.Now(),
115128
})
116129
return nil
117130
}
118131

132+
// pinnedInflight returns what Produce recorded, or zero if it never ran.
133+
func (p *Observer) pinnedInflight(requestID, endpointID string) int64 {
134+
pinned, err := fwkplugin.ReadPluginStateKey[inflightAtDispatch](p.PluginState, requestID, inflightStateKey)
135+
if err != nil {
136+
return 0
137+
}
138+
return pinned[endpointID]
139+
}
140+
119141
// primaryTarget returns the endpoint the primary profile selected, or nil. TTFT
120142
// belongs to whichever endpoint produced the first token.
121143
func primaryTarget(result *fwksched.SchedulingResult) fwksched.Endpoint {

0 commit comments

Comments
 (0)