Skip to content

Commit 95e8b0f

Browse files
committed
test(inflightload): table-driven flap test covering token counters
Consolidate the per-request flap regression into a single table-driven test and extend coverage to the token counter, which receives the same instance-capture fix but was previously only asserted via the request counter. Cases cover both counters across both release paths: - request counter via the EndOfStream eviction path (OnEvicted) - token counter via the EndOfStream eviction path (OnEvicted) - token counter via the StartOfStream early-release path (releaseTokensEarly) Each case runs the same flap sequence (join, track A, delete + rejoin, track B, release A then B) and asserts A's release lands on the orphaned counter, leaving B's live counter accurate and never negative. Signed-off-by: Luke Van Drie <lukevandrie@google.com>
1 parent af755f5 commit 95e8b0f

1 file changed

Lines changed: 82 additions & 53 deletions

File tree

  • pkg/epp/framework/plugins/requestcontrol/dataproducer/inflightload

pkg/epp/framework/plugins/requestcontrol/dataproducer/inflightload/producer_test.go

Lines changed: 82 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -311,64 +311,93 @@ func TestInFlightLoadProducer_DumpStateCapsEndpoints(t *testing.T) {
311311
require.Equal(t, int64(104), state.Endpoints[0].Requests)
312312
}
313313

314-
// TestInFlightLoadProducer_FlapDoesNotUnderflow asserts that an in-flight request's release lands
315-
// on the exact counter instance it incremented, even after the endpoint is deleted and recreated
316-
// under the same NamespacedName. A release from a request that predates the flap hits the orphaned
317-
// counter, leaving the live counter accurate and never negative.
314+
// TestInFlightLoadProducer_FlapDoesNotUnderflow asserts that a request's release lands on the exact
315+
// counter instance it incremented, even after the endpoint is deleted and recreated under the same
316+
// NamespacedName. A release that predates the flap hits the orphaned counter, leaving the live
317+
// counter accurate and never negative. Cases cover the request and token counters across both the
318+
// EndOfStream eviction path (OnEvicted) and the StartOfStream early-release path (releaseTokensEarly).
318319
func TestInFlightLoadProducer_FlapDoesNotUnderflow(t *testing.T) {
319320
t.Parallel()
320321

321-
producer := newTestProducer(t)
322-
ctx := context.Background()
323-
endpointName := "flap-endpoint"
324-
endpointID := fullEndpointName(endpointName)
325-
326-
// E joins the endpoint set. The same Endpoint object is reused for the
327-
// matching delete so the registeredEndpoints stale-delete guard allows
328-
// cleanup (mirrors the datalayer's same-pointer add/delete contract).
329-
epA := newStubSchedulingEndpoint(endpointName)
330-
require.NoError(t, producer.Extract(ctx, datalayer.EndpointEvent{
331-
Type: datalayer.EventAddOrUpdate,
332-
Endpoint: epA,
333-
}))
334-
335-
// Request A is routed to endpoint E.
336-
reqA := makeTokenRequest("req-A", 4)
337-
resA := makeSchedulingResult(endpointName)
338-
producer.PreRequest(ctx, reqA, resA)
339-
require.Equal(t, int64(1), producer.requestTracker.get(endpointID))
322+
requests := func(p *InFlightLoadProducer, id string) int64 { return p.requestTracker.get(id) }
323+
tokens := func(p *InFlightLoadProducer, id string) int64 { return p.tokenTracker.get(id) }
324+
325+
tests := []struct {
326+
name string
327+
addEstimatedOutputTokens bool
328+
release requestcontrol.Response // chunk that triggers the release under test
329+
read func(*InFlightLoadProducer, string) int64
330+
inputA, inputB int // input tokens for requests A and B
331+
liveAfterA, liveAfterB int64 // live counter after A's, then B's, PreRequest
332+
liveAfterReleaseA int64 // live counter after A releases, while B is still in flight
333+
}{
334+
{
335+
name: "request counter, EndOfStream eviction",
336+
release: requestcontrol.Response{EndOfStream: true},
337+
read: requests,
338+
inputA: 4, inputB: 4,
339+
liveAfterA: 1, liveAfterB: 1, liveAfterReleaseA: 1,
340+
},
341+
{
342+
name: "token counter, EndOfStream eviction",
343+
addEstimatedOutputTokens: true, // 4 input + 6 estimated output = 10 tokens per request
344+
release: requestcontrol.Response{EndOfStream: true},
345+
read: tokens,
346+
inputA: 4, inputB: 4,
347+
liveAfterA: 10, liveAfterB: 10, liveAfterReleaseA: 10,
348+
},
349+
{
350+
name: "token counter, StartOfStream early release",
351+
release: requestcontrol.Response{StartOfStream: true}, // output excluded: tokens == input
352+
read: tokens,
353+
inputA: 4, inputB: 6,
354+
liveAfterA: 4, liveAfterB: 6, liveAfterReleaseA: 6,
355+
},
356+
}
340357

341-
// E flaps: it leaves the endpoint set (NotReady) while A is still in flight,
342-
// dropping its counter.
343-
require.NoError(t, producer.Extract(ctx, datalayer.EndpointEvent{
344-
Type: datalayer.EventDelete,
345-
Endpoint: epA,
346-
}))
347-
require.Equal(t, int64(0), producer.requestTracker.get(endpointID), "counter dropped on delete")
358+
const endpointName = "flap-endpoint"
359+
extract := func(t *testing.T, p *InFlightLoadProducer, typ datalayer.EventType, ep datalayer.Endpoint) {
360+
require.NoError(t, p.Extract(context.Background(), datalayer.EndpointEvent{Type: typ, Endpoint: ep}))
361+
}
348362

349-
// E rejoins with the same NamespacedName but a new Endpoint object; request
350-
// B's PreRequest recreates a fresh counter instance under the same key.
351-
epB := newStubSchedulingEndpoint(endpointName)
352-
require.NoError(t, producer.Extract(ctx, datalayer.EndpointEvent{
353-
Type: datalayer.EventAddOrUpdate,
354-
Endpoint: epB,
355-
}))
356-
reqB := makeTokenRequest("req-B", 4)
357-
resB := makeSchedulingResult(endpointName)
358-
producer.PreRequest(ctx, reqB, resB)
359-
require.Equal(t, int64(1), producer.requestTracker.get(endpointID), "B recreated the counter")
360-
361-
// A completes. Its release must hit the orphaned counter, not B's live one, which still holds B.
362-
reqA.SchedulingResult = resA
363-
producer.ResponseBody(ctx, reqA, &requestcontrol.Response{EndOfStream: true}, nil)
364-
require.Equal(t, int64(1), producer.requestTracker.get(endpointID),
365-
"A's release must not discount B's live counter")
366-
367-
// B completes. The live counter settles at 0 and never underflows.
368-
reqB.SchedulingResult = resB
369-
producer.ResponseBody(ctx, reqB, &requestcontrol.Response{EndOfStream: true}, nil)
370-
require.Equal(t, int64(0), producer.requestTracker.get(endpointID),
371-
"counter must settle at 0, never negative")
363+
for _, tc := range tests {
364+
t.Run(tc.name, func(t *testing.T) {
365+
producer := newTestProducer(t)
366+
producer.addEstimatedOutputTokens = tc.addEstimatedOutputTokens
367+
ctx := context.Background()
368+
endpointID := fullEndpointName(endpointName)
369+
370+
// E joins. The same Endpoint object is reused for its delete so the registeredEndpoints
371+
// guard allows cleanup (the datalayer's same-pointer add/delete contract).
372+
epA := newStubSchedulingEndpoint(endpointName)
373+
extract(t, producer, datalayer.EventAddOrUpdate, epA)
374+
reqA := makeTokenRequest("req-A", tc.inputA)
375+
resA := makeSchedulingResult(endpointName)
376+
producer.PreRequest(ctx, reqA, resA)
377+
require.Equal(t, tc.liveAfterA, tc.read(producer, endpointID))
378+
379+
// E flaps and rejoins under the same NamespacedName; B recreates a fresh counter instance.
380+
extract(t, producer, datalayer.EventDelete, epA)
381+
require.Equal(t, int64(0), tc.read(producer, endpointID), "counter dropped on delete")
382+
epB := newStubSchedulingEndpoint(endpointName)
383+
extract(t, producer, datalayer.EventAddOrUpdate, epB)
384+
reqB := makeTokenRequest("req-B", tc.inputB)
385+
resB := makeSchedulingResult(endpointName)
386+
producer.PreRequest(ctx, reqB, resB)
387+
require.Equal(t, tc.liveAfterB, tc.read(producer, endpointID), "B recreated the counter")
388+
389+
// A releases. It must hit the orphaned counter, not B's live one.
390+
reqA.SchedulingResult = resA
391+
producer.ResponseBody(ctx, reqA, &tc.release, nil)
392+
require.Equal(t, tc.liveAfterReleaseA, tc.read(producer, endpointID),
393+
"A's release must not discount B's live counter")
394+
395+
// B releases. The live counter settles at 0 and never underflows.
396+
reqB.SchedulingResult = resB
397+
producer.ResponseBody(ctx, reqB, &tc.release, nil)
398+
require.Equal(t, int64(0), tc.read(producer, endpointID), "counter must settle at 0, never negative")
399+
})
400+
}
372401
}
373402

374403
// TestInFlightLoadProducer_CrashWithHighLoadDoesNotUnderflow models a pod that crashes while

0 commit comments

Comments
 (0)