From 9f7ffb69af3e6c5cbfd740efa39ccdf9ba975de6 Mon Sep 17 00:00:00 2001 From: Benoit Sigoure Date: Mon, 6 Jul 2026 21:47:17 +0000 Subject: [PATCH] scaleset: tolerate invalid instance status transitions in job message handlers HandleJobsCompleted and HandleJobsStarted only tolerated ErrNotFound when updating an instance's status/runner_status in response to a scale set job message. If the instance had already transitioned to a terminal state (e.g. "deleted", because the reconciler noticed it went missing from the provider/github after a host failure) by the time a delayed "job started"/"job completed" message arrived, UpdateInstance would reject the transition with a BadRequestError. That error was treated as fatal, causing handleSessionMessage to return before advancing the message cursor or deleting the message. Since the same poisoned message would then be redelivered forever, the entire scale set listener would get stuck retrying it, blocking every subsequent message (including new job assignments) for that scale set. Observed in prod: two scale sets stuck in a tight retry loop logging "invalid instance status transition from deleted to pending_delete" hundreds of times, with a large backlog of queued jobs and idle runners that could never receive them. Treat BadRequestError from UpdateInstance the same way ErrNotFound is already treated: log and move on, since it means some other codepath already resolved the runner's terminal state and there's nothing left for us to do. --- workers/scaleset/scaleset_helper.go | 31 ++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/workers/scaleset/scaleset_helper.go b/workers/scaleset/scaleset_helper.go index e0871d197..9a02a4e78 100644 --- a/workers/scaleset/scaleset_helper.go +++ b/workers/scaleset/scaleset_helper.go @@ -122,10 +122,25 @@ func (w *Worker) HandleJobsCompleted(jobs []params.ScaleSetJobMessage) (err erro locking.Lock(job.RunnerName, w.consumerID) _, err := w.store.UpdateInstance(w.ctx, job.RunnerName, runnerUpdateParams) if err != nil { - if !errors.Is(err, runnerErrors.ErrNotFound) { - locking.Unlock(job.RunnerName, false) - return fmt.Errorf("updating runner %s: %w", job.RunnerName, err) + if errors.Is(err, runnerErrors.ErrNotFound) { + locking.Unlock(job.RunnerName, true) + continue + } + if errors.Is(err, runnerErrors.ErrBadRequest) { + // The instance is no longer in a state that can transition to + // pending_delete (e.g. it was already deleted or is being deleted + // by some other codepath, such as the reconciler noticing it went + // missing from the provider/github). There is nothing further for + // us to do here. Treating this as fatal would cause this message + // to be retried forever, since we never advance past it or delete + // it, blocking every subsequent message for this scale set. + slog.InfoContext(w.ctx, "runner already being handled/removed; ignoring completed job status update", + "runner_name", job.RunnerName, "error", err) + locking.Unlock(job.RunnerName, true) + continue } + locking.Unlock(job.RunnerName, false) + return fmt.Errorf("updating runner %s: %w", job.RunnerName, err) } locking.Unlock(job.RunnerName, false) } @@ -160,6 +175,16 @@ func (w *Worker) HandleJobsStarted(jobs []params.ScaleSetJobMessage) (err error) locking.Unlock(job.RunnerName, true) continue } + if errors.Is(err, runnerErrors.ErrBadRequest) { + // The runner is already in a terminal/being-deleted state (e.g. it + // was reaped after going missing from the provider or github). Same + // reasoning as in HandleJobsCompleted: don't fail the whole message, + // or we'll retry it forever and block the scale set's message queue. + slog.InfoContext(w.ctx, "runner already being handled/removed; ignoring started job status update", + "runner_name", job.RunnerName, "error", err) + locking.Unlock(job.RunnerName, true) + continue + } locking.Unlock(job.RunnerName, false) return fmt.Errorf("updating runner %s: %w", job.RunnerName, err) }