fix(cloud): drop tasks past payload store retention#4369
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
|
|
| for _, _task := range toFail { | ||
| task := _task | ||
|
|
||
| failGroup.Go(func() error { |
There was a problem hiding this comment.
do we do this kind of thing concurrently in other places? just checking to make sure we're being consistent, not sure off the top of my head
|
|
||
| // return the payloads which were found alongside the error so callers can proceed | ||
| // with the rest of the batch | ||
| return optsToPayload, &PayloadNotFoundError{Missing: missing} |
There was a problem hiding this comment.
this sketches me out a little - feels odd to be returning both a result and an error
Benchmark resultsCompared against |
Description
Resolves the FIXME in the dispatcher's populateTaskData: when bulk input retrieval fails, the dispatcher previously requeued every task in the batch regardless of whether the error was transient (e.g. an S3 rate limit) or permanent (the payload no longer exists in the external store). Tasks whose offloaded payloads are permanently gone would loop forever through assign/requeue, never reaching a terminal state. Because these zombie tasks also hold GROUP_ROUND_ROBIN concurrency slots, they can block all other work behind the same concurrency keys and build up a large backlog for the tenant.
This PR introduces a sentinel error, repository.ErrPayloadNotFound. When the dispatcher sees this error from a bulk retrieval, it retries retrieval per task and splits the batch: tasks whose payloads are permanently missing are failed non-retryably with a descriptive error message.
A companion change in the cloud S3 external store wraps NoSuchKey errors on index file fetches (and external IDs missing from an index file) with the new sentinel; it depends on this PR being released first.
Type of change
Checklist
Changes have been:
🤖 AI Disclosure