Skip to content

Commit 724c717

Browse files
committed
fix(flowcontrol): comma-ok downcasts and crash attribution on owned goroutines
dispatchItem and the sweep predicate hard-asserted *FlowItem while evictAll already handled the cast gracefully; all three now use comma-ok, surfacing an unexpected type as an error or a skipped item instead of a panic in the dispatch loop. The processor run loop, cleanup sweep, and sweep workers defer utilruntime.HandleCrashWithLogger so an unknown bug logs the panic with component context before the default handlers repanic. Signed-off-by: Luke Van Drie <lukevandrie@google.com>
1 parent 4f19100 commit 724c717

1 file changed

Lines changed: 13 additions & 2 deletions

File tree

pkg/epp/flowcontrol/controller/internal/processor.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"time"
2727

2828
"github.com/go-logr/logr"
29+
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
2930
"k8s.io/utils/clock"
3031

3132
logutil "github.com/llm-d/llm-d-router/pkg/common/observability/logging"
@@ -178,6 +179,9 @@ func (p *Processor) SubmitOrBlock(ctx context.Context, item *FlowItem) error {
178179
// It uses a `select` statement to interleave accepting new requests with dispatching existing ones, balancing
179180
// responsiveness with throughput.
180181
func (p *Processor) Run(ctx context.Context) {
182+
// Log any panic with processor context before the default handlers repanic; the process
183+
// fail-stops rather than continuing on state a panicked goroutine may have left inconsistent.
184+
defer utilruntime.HandleCrashWithLogger(p.logger)
181185
p.logger.V(logutil.DEFAULT).Info("Processor run loop starting.")
182186
defer p.logger.V(logutil.DEFAULT).Info("Processor run loop stopped.")
183187

@@ -450,7 +454,11 @@ func (p *Processor) dispatchItem(itemAcc flowcontrol.QueueItemAccessor) error {
450454
return nil
451455
}
452456

453-
removedItem := removedItemAcc.(*FlowItem)
457+
removedItem, ok := removedItemAcc.(*FlowItem)
458+
if !ok {
459+
// Nothing to finalize on an unknown type; surface the error so the cycle moves to the next band.
460+
return fmt.Errorf("internal error: item %q for flow %s has unexpected type %T", req.ID(), key, removedItemAcc)
461+
}
454462
p.logger.V(logutil.TRACE).Info("Item dispatched.", "flowKey", req.FlowKey(), "requestID", req.ID())
455463
removedItem.FinalizeWithOutcome(types.QueueOutcomeDispatched, nil)
456464
return nil
@@ -461,6 +469,7 @@ func (p *Processor) dispatchItem(itemAcc flowcontrol.QueueItemAccessor) error {
461469
func (p *Processor) runCleanupSweep(ctx context.Context) {
462470
defer p.wg.Done()
463471
logger := p.logger.WithName("runCleanupSweep")
472+
defer utilruntime.HandleCrashWithLogger(logger)
464473
logger.V(logutil.DEFAULT).Info("Cleanup sweep goroutine starting.")
465474
defer logger.V(logutil.DEFAULT).Info("Cleanup sweep goroutine stopped.")
466475

@@ -482,7 +491,8 @@ func (p *Processor) runCleanupSweep(ctx context.Context) {
482491
func (p *Processor) sweepFinalizedItems() {
483492
processFn := func(managedQ contracts.ManagedQueue, logger logr.Logger) {
484493
predicate := func(itemAcc flowcontrol.QueueItemAccessor) bool {
485-
return itemAcc.(*FlowItem).FinalState() != nil
494+
item, ok := itemAcc.(*FlowItem)
495+
return ok && item.FinalState() != nil
486496
}
487497
removedItems := managedQ.Cleanup(predicate)
488498
if len(removedItems) > 0 {
@@ -598,6 +608,7 @@ func (p *Processor) processAllQueuesConcurrently(
598608
var wg sync.WaitGroup
599609
for range numWorkers {
600610
wg.Go(func() {
611+
defer utilruntime.HandleCrashWithLogger(logger)
601612
for task := range tasks {
602613
processFn(task.mq, task.logger)
603614
}

0 commit comments

Comments
 (0)