Skip to content

Commit c66cb9f

Browse files
committed
fix(metrics): guard overflow series from flow GC pruning
A flow whose client-chosen fairness ID equals the overflow value must not delete the shared series that aggregates every capped-out tenant, so DeleteFlowControlFlowSeries now no-ops on the overflow value. Also document the revive-during-GC gauge race as an accepted limitation and clarify in docs/metrics.md that the model labels share one cap pool and that caps apply over the process lifetime. Signed-off-by: Luke Van Drie <lukevandrie@google.com>
1 parent f43c211 commit c66cb9f

3 files changed

Lines changed: 41 additions & 4 deletions

File tree

docs/metrics.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,11 @@ and the release stage is ALPHA. Request and latency metrics share the label set
6464
`{model_name, target_model_name, fairness_id, priority}`.
6565

6666
Client-derived label values are cardinality-bounded: `model_name` and `target_model_name` (from the
67-
request body) and `fairness_id` (from the `x-llm-d-inference-fairness-id` header) are each capped at
68-
1000 distinct values, after which new values are reported as `other`. Model names configured through
69-
InferenceModelRewrite rules never fold to `other`. Flow control series for a `fairness_id` are
70-
removed when its flow is garbage collected.
67+
request body) share a cap of 1000 distinct values, and `fairness_id` (from the
68+
`x-llm-d-inference-fairness-id` header) is capped at 1000 distinct values. Caps apply over the
69+
lifetime of the process; once a cap is reached, new values are reported as `other`. Model names
70+
configured through InferenceModelRewrite rules never fold to `other`. Flow control series for a
71+
`fairness_id` are removed when its flow is garbage collected.
7172

7273
### Request and latency
7374

pkg/epp/metrics/cardinality_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,3 +208,30 @@ func TestQueueDurationBoundsModelLabels(t *testing.T) {
208208
require.Equal(t, testCap+1, promtestutil.CollectAndCount(llmdFlowControlRequestQueueDuration),
209209
"the llm_d_epp family must be bounded identically")
210210
}
211+
212+
// A client can choose the overflow value itself as its fairness ID; GC of that flow must not
213+
// delete the shared overflow series that aggregates every capped-out tenant.
214+
func TestDeleteFlowControlFlowSeriesPreservesOverflowSeries(t *testing.T) {
215+
old := fairnessLabelLimiter
216+
fairnessLabelLimiter = newBoundedLabel(1)
217+
flowControlRequestEnqueueDuration.Reset()
218+
llmdFlowControlRequestEnqueueDuration.Reset()
219+
t.Cleanup(func() {
220+
fairnessLabelLimiter = old
221+
flowControlRequestEnqueueDuration.Reset()
222+
llmdFlowControlRequestEnqueueDuration.Reset()
223+
})
224+
225+
// tenant-a fills the single cap slot; tenant-b folds to the overflow series.
226+
RecordFlowControlRequestEnqueueDuration("tenant-a", "0", "Dispatched", time.Millisecond)
227+
RecordFlowControlRequestEnqueueDuration("tenant-b", "0", "Dispatched", time.Millisecond)
228+
require.Equal(t, 2, promtestutil.CollectAndCount(flowControlRequestEnqueueDuration),
229+
"setup: expected the admitted series plus the overflow series")
230+
231+
DeleteFlowControlFlowSeries(overflowValue, "0")
232+
233+
require.Equal(t, 2, promtestutil.CollectAndCount(flowControlRequestEnqueueDuration),
234+
"deleting the overflow value must be a no-op; the shared overflow series must survive")
235+
require.Equal(t, 2, promtestutil.CollectAndCount(llmdFlowControlRequestEnqueueDuration),
236+
"the llm_d_epp family must be preserved identically")
237+
}

pkg/epp/metrics/metrics.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -900,7 +900,16 @@ func IncFlowControlRequestsTotal(outcome, priority, inferencePool string) {
900900
// derived from client input, so its cardinality is unbounded; the flow registry calls this when it
901901
// garbage-collects an idle flow so that the metric vectors track live flows instead of growing
902902
// monotonically with every fairness ID ever observed.
903+
//
904+
// Pruning is not synchronized with recording: a request reviving the flow concurrently with GC can
905+
// have a queue gauge increment deleted here while its paired decrement lands afterwards, leaving
906+
// the queue size/bytes gauges negative until the flow's next collection deletes the series again.
903907
func DeleteFlowControlFlowSeries(fairnessID, priority string) {
908+
// The overflow value aggregates every capped-out fairness ID, so a flow whose client-chosen ID
909+
// equals it must not delete the shared series.
910+
if fairnessID == overflowValue {
911+
return
912+
}
904913
labels := prometheus.Labels{"fairness_id": fairnessID, "priority": priority}
905914
flowControlRequestQueueDuration.DeletePartialMatch(labels)
906915
flowControlRequestEnqueueDuration.DeletePartialMatch(labels)

0 commit comments

Comments
 (0)