Skip to content

Commit 493cec1

Browse files
authored
fix: flowcontrol - finalize requests during shutdown (#2156)
fixes #2099 Signed-off-by: nicole-lihui <nicole.li@daocloud.io>
1 parent 34478d1 commit 493cec1

3 files changed

Lines changed: 89 additions & 3 deletions

File tree

pkg/epp/flowcontrol/controller/controller.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,8 +378,15 @@ func (fc *FlowController) tryDistribution(
378378
return item, finalErr
379379
}
380380

381+
func finalizeOnControllerShutdown(item *internal.FlowItem) (types.QueueOutcome, error) {
382+
item.Finalize(types.ErrFlowControllerNotRunning)
383+
384+
finalState := item.FinalState()
385+
return finalState.Outcome, finalState.Err
386+
}
387+
381388
// awaitFinalization blocks until an item is finalized, either by the processor (synchronously) or by the controller
382-
// itself due to context expiry (asynchronously).
389+
// itself due to context expiry or shutdown (asynchronously).
383390
func (fc *FlowController) awaitFinalization(
384391
reqCtx context.Context,
385392
item *internal.FlowItem,
@@ -388,13 +395,20 @@ func (fc *FlowController) awaitFinalization(
388395
case <-reqCtx.Done():
389396
// Asynchronous Finalization (Controller-initiated):
390397
// The request Context expired (Cancellation/TTL) while the item was being processed.
398+
if fc.parentCtx.Err() != nil {
399+
return finalizeOnControllerShutdown(item)
400+
}
401+
391402
cause := context.Cause(reqCtx)
392403
item.Finalize(cause)
393404

394405
// The processor will eventually discard this "zombie" item during its cleanup sweep.
395406
finalState := item.FinalState()
396407
return finalState.Outcome, finalState.Err
397408

409+
case <-fc.parentCtx.Done():
410+
return finalizeOnControllerShutdown(item)
411+
398412
case finalState := <-item.Done():
399413
// Synchronous Finalization (Processor-initiated):
400414
// The processor finalized the item (Dispatch, Reject, Shutdown).

pkg/epp/flowcontrol/controller/controller_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,71 @@ func TestFlowController_EnqueueAndWait(t *testing.T) {
367367
assert.Equal(t, types.QueueOutcomeRejectedOther, outcome,
368368
"outcome should be QueueOutcomeRejectedOther on shutdown")
369369
})
370+
t.Run("OnControllerShutdownDuringFinalization", func(t *testing.T) {
371+
t.Parallel()
372+
ctx, cancel := context.WithCancel(t.Context())
373+
h := newUnitHarness(ctx, t, &Config{}, nil, nil)
374+
item := internal.NewItem(newTestRequest(defaultFlowKey), 0, time.Now())
375+
376+
result := make(chan struct {
377+
outcome types.QueueOutcome
378+
err error
379+
}, 1)
380+
go func() {
381+
outcome, err := h.fc.awaitFinalization(context.Background(), item)
382+
result <- struct {
383+
outcome types.QueueOutcome
384+
err error
385+
}{outcome: outcome, err: err}
386+
}()
387+
388+
cancel()
389+
select {
390+
case r := <-result:
391+
require.Error(t, r.err, "awaitFinalization must fail when controller shuts down")
392+
assert.ErrorIs(t, r.err, types.ErrFlowControllerNotRunning,
393+
"error should wrap ErrFlowControllerNotRunning")
394+
assert.Equal(t, types.QueueOutcomeRejectedOther, r.outcome,
395+
"outcome should be QueueOutcomeRejectedOther on shutdown")
396+
case <-time.After(time.Second):
397+
t.Fatal("awaitFinalization did not return after controller shutdown")
398+
}
399+
})
400+
t.Run("OnControllerShutdownTakesPrecedenceOverRequestCancellation", func(t *testing.T) {
401+
t.Parallel()
402+
ctx, cancel := context.WithCancel(t.Context())
403+
h := newUnitHarness(ctx, t, &Config{}, nil, nil)
404+
reqCtx, reqCancel := context.WithCancel(context.Background())
405+
item := internal.NewItem(newTestRequest(defaultFlowKey), 0, time.Now())
406+
407+
reqCancel()
408+
cancel()
409+
410+
outcome, err := h.fc.awaitFinalization(reqCtx, item)
411+
require.Error(t, err, "awaitFinalization must fail when controller shuts down")
412+
assert.ErrorIs(t, err, types.ErrFlowControllerNotRunning,
413+
"controller shutdown should take precedence over request cancellation")
414+
assert.Equal(t, types.QueueOutcomeRejectedOther, outcome,
415+
"shutdown should return the rejected outcome")
416+
})
417+
t.Run("OnControllerShutdownPreservesQueuedOutcome", func(t *testing.T) {
418+
t.Parallel()
419+
ctx, cancel := context.WithCancel(t.Context())
420+
h := newUnitHarness(ctx, t, &Config{}, nil, nil)
421+
item := internal.NewItem(newTestRequest(defaultFlowKey), 0, time.Now())
422+
item.SetHandle(&fwkfcmocks.MockQueueItemHandle{})
423+
424+
cancel()
425+
426+
outcome, err := h.fc.awaitFinalization(context.Background(), item)
427+
require.Error(t, err, "awaitFinalization must fail when controller shuts down")
428+
assert.ErrorIs(t, err, types.ErrEvicted,
429+
"a queued item should be evicted, not rejected, during shutdown")
430+
assert.ErrorIs(t, err, types.ErrFlowControllerNotRunning,
431+
"queued shutdown should preserve the shutdown cause")
432+
assert.Equal(t, types.QueueOutcomeEvictedOther, outcome,
433+
"a queued item should return the evicted outcome")
434+
})
370435

371436
t.Run("OnRegistryConnectionError", func(t *testing.T) {
372437
t.Parallel()

pkg/epp/flowcontrol/integration_test.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,11 @@ func TestConcurrentEnqueueDuringShutdown(t *testing.T) {
782782

783783
h := newHarness(t, harnessOpts{
784784
detector: detector,
785+
controllerCfg: &controller.Config{
786+
DefaultRequestTTL: 0,
787+
ExpiryCleanupInterval: 10 * time.Millisecond,
788+
EnqueueChannelBufferSize: 100,
789+
},
785790
})
786791

787792
key := flowcontrol.FlowKey{ID: "flow-a", Priority: 0}
@@ -791,8 +796,8 @@ func TestConcurrentEnqueueDuringShutdown(t *testing.T) {
791796
for i := 0; i < numRequests; i++ {
792797
id := fmt.Sprintf("req-%d", i)
793798
go func() {
794-
req := &testRequest{id: id, key: key, byteSize: 100, ttl: 5 * time.Minute}
795-
outcome, err := h.fc.EnqueueAndWait(h.ctx, req)
799+
req := &testRequest{id: id, key: key, byteSize: 100}
800+
outcome, err := h.fc.EnqueueAndWait(context.Background(), req)
796801
results <- dispatchResult{id: id, outcome: outcome, err: err}
797802
}()
798803
}
@@ -809,6 +814,8 @@ func TestConcurrentEnqueueDuringShutdown(t *testing.T) {
809814
"no request should dispatch (detector is blocked and controller is shutting down)")
810815
require.Error(t, r.err,
811816
"every request should receive an error during shutdown")
817+
require.ErrorIs(t, r.err, fcTypes.ErrFlowControllerNotRunning,
818+
"every request should report the shutdown cause")
812819
case <-time.After(5 * time.Second):
813820
t.Fatalf("request %d hung during concurrent shutdown", i)
814821
}

0 commit comments

Comments
 (0)