Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[10.x] Add JobTimingOut event to Worker #48893

Conversation

sebapastore
Copy link

Add a JobTimingOut event in order to allow developers to resolve the scenario described in #48392.

The main problem described in the issue is that if the job has a transaction that is still running when the job hits timeout, then any database operation that is sent before rolling back the job transaction, will not be executed as expected.

There already exists a JobTimedOut event, but this event is emitted after some processes that may involve database access in some scenarios. So, by adding this new event, developers can roll back database transactions before these mentioned processes are executed.

I prepared a repository in order to test this.

There are two jobs TimeOutJobWithoutTransactionRollback and TimeOutJobWithTransactionRollback:

class TimeOutJobWithoutTransactionRollback implements ShouldQueue
{
    ...

    public function handle(): void
    {
        DB::transaction(fn() => sleep(999));
    }
}
class TimeOutJobWithTransactionRollback implements ShouldQueue
{
    ...

    public function handle(): void
    {
        DB::transaction(fn() => sleep(999));
    }
}

There is also a event listener that will make DB rollback only for TimeOutJobWithTransactionRollback :

class HandleJobTimingOut
{
    ...

    {
        if ($event->job->resolveName() === TimeOutJobWithTransactionRollback::class) {
            DB::rollBack();
        }
    }
}

So you can try for example:

\Bus::batch([new TimeOutJobWithoutTransactionRollback(), new TimeOutJobWithoutTransactionRollback()])
    ->allowFailures()
    ->dispatch();

and that will reproduce the mentioned issue. Then you can try:

\Bus::batch([new TimeOutJobWithTransactionRollback(), new TimeOutJobWithTransactionRollback()])
    ->allowFailures()
    ->dispatch();

and the issue is solved with the rollback.

So, this PR resolve #48392.

@sebapastore sebapastore changed the title Add JobTimingOut event to Worker [10.x] Add JobTimingOut event to Worker Nov 3, 2023
@taylorotwell
Copy link
Member

This doesn't really fix the issue. It just adds an event which the developer then has to listen to (in every Laravel application that uses batches?) to fix the issue themselves. I think we should try to address the root issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Batch Callbacks not triggering if job timeout while in transaction
2 participants