[13.x] Job expiration based on job timeout#60751
Conversation
|
|
||
| if (! is_null($timeout)) { | ||
| $reservationOffset = (int) $timeout + 10 - $this->retryAfter; | ||
| } |
There was a problem hiding this comment.
For both the Redis and database drivers:
- If a timeout is set on the job, that is used
- If no timeout is set on the job, the timeout on the worker is used
- If no timeout on the worker or the job, then the
retry_afteris used (same as now)
| "--backoff={$options->backoff}", | ||
| "--memory={$options->memory}", | ||
| "--sleep={$options->sleep}", | ||
| "--timeout={$options->timeout}", |
There was a problem hiding this comment.
This is for queue:listen and queue:work --once. For those commands the --timeout option currently isn't used (because pcntl isn't used to kill the worker like it is in queue:work). After this PR the timeout still isn't used for anything except for the job expiration when expire_after_timeout is enabled.
|
|
||
| $this->raiseWorkerStartingEvent($connectionName, $queue, $options); | ||
|
|
||
| $this->shareWorkerTimeout($connectionName, $options); |
There was a problem hiding this comment.
For sharing the timeout set on the worker (e.g. queue:work --timeout=300). This timeout is used when a job doesn't define a timeout of it's own.
|
I've also hit this and ended up creating a new queue and connection, just for one job. So, this PR is very much appreciated 🫶 at least puts some light on it |
This PR adds an
expire_after_timeoutoption to the Redis and database queue drivers. When enabled, jobs expire 10 seconds after their timeout, instead of naively based onretry_after.Quick recap on how job timeouts, expiration, and retries work:
$tries = 1fail instantly when they time out$tries > 1get killed when they time out. The queue starts the next attempt when the previous attempt has expired.Currently, jobs always expire
retry_afterseconds after they're started, which causes two problems:retry_aftermust be longer than your longest-running job. If it's shorter, the job gets retried while it's still running, so it ends up running twice at the same time.retry_afterwas set to 1 hour. That meant a short job that timed out would sit for a full hour before retrying. Even worse, nothing gets logged when this happens, so the job essentially disappears for an hour (which is not fun to debug).Enabling
expire_after_timeoutfixes both these problems, and in most cases it removes the need forretry_afterentirely.retry_afteris now only used for jobs that run without a timeout (e.g.$timeout = 0).retry_afterand the newexpire_after_timeoutare only used by the Redis and database queue drivers:now + retry_afteras the job expiration. Withexpire_after_timeoutenabled it storesnow + timeout + 10reserved_atcolumn that gets set tonowwhen a worker picks up the job. Withexpire_after_timeoutenabled it'snow + timeout + 10 - retry_after. This works nicely with the existing query that finds expired jobs viareserved_at <= now - retry_after(it does make thereserved_atname misleading, but I think that's fine)This PR is backwards compatible and the feature is disabled by default, nothing changes for existing applications when this is merged. A small follow-up PR for Horizon is needed to enable this flag there too.