ShouldBeUniqueUntilProcessing job with afterCommit get persist lock on failed database transactions and unable to dispatch again

[aprokopenko]

Laravel Version

10.6.2, 10.15.0

PHP Version

8.2.6

Database Driver & Version

MySQL 8.0.30; Redis 7

Description

If queue afterCommit flag is true and database transaction failed (will be rolled back), then all jobs dispatched with ShouldBeUniqueUntilProcessing interface will get a persistent lock, and you can't dispatch them again at all (because there are no any instruments to clear the locks from redis).

This happens because on transaction rollback the Job is not dispatched, but lock is already set and is not released.

So afterwards - you can't dispatch this job again, because the lock is exists.

The only possible workaround right now is to set some $uniqueFor value, so the lock will expire in some time. But this is a bit wrong behavior, when I really need ShouldBeUniqueUntilProcessing without expiration.

Similar issues were reported already without any fix:

• Run after transaction ShouldBeUniqueUntilProcessing job blocks jobs creation on DB transaction failed #45506 (Laravel 9)
• Run after transaction ShouldBeUniqueUntilProcessing job blocks jobs creation on DB transaction failed #45488 (Laravel 8)

Steps To Reproduce

• Install Laravel
• Set queue and cache drivers to redis
• Add job like:

<?php

declare(strict_types=1);

namespace App\Tmp;

use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldBeUniqueUntilProcessing;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Support\Facades\Log;

class UniqueJob implements ShouldQueue, ShouldBeUniqueUntilProcessing
{
    use Dispatchable, Queueable;

    public function handle()
    {
        Log::info('Job executed successfully.');
    }
}

• Add command and register it in Kernel or ServiceProvider

<?php

declare(strict_types=1);

namespace App\Tmp;

use App\Models\User;
use Illuminate\Console\Command;
use Illuminate\Support\Facades\DB;

class DispatchJobWithinFailedTransaction extends Command
{
    protected $signature = 'app:dispatch-job-within-failed-transaction
        {--commit : Commit transaction for success case}
        {--rollback : Rollback transaction for failure case}
        ';
    protected $description = 'Replicate bug with dispatching job within failed transaction';

    public function handle()
    {
        $this->line('Start transaction');
        DB::beginTransaction();

        $this->line('Dispatch "after commit" job with lock');
        UniqueJob::dispatch()
            ->afterCommit();

        if ($this->option('commit')) {
            $this->line('Commit transaction');
            DB::commit();
        }

        if ($this->option('rollback')) {
            $this->line('Rollback transaction');
            DB::rollBack();
        }
    }
}

• run queue process with php artisan queue:work
• run successful case to make sure all working fine with php artisan app:dispatch-job-within-failed-transaction --commit
• you should see successful logs in laravel.log and queue job status
• run negative case with php artisan app:dispatch-job-within-failed-transaction --rollback > no job has been executed, this is okay
• run successful case again php artisan app:dispatch-job-within-failed-transaction --commit
• BUG: job is not dispatched and is not handled, because lock is persistent here already

[aprokopenko]

I created a Pull Request. Unfortunately, lot of tests failed, because the logic is different now.
Would be great if you can validate logic is correct and guide me how to set up local environment to run framework tests (not app tests), because I can't debug them locally right now with just simple setup of framework repository. (If logic is okay, I can invest my time to fix tests as well)

This is a very tricky part, I tried to follow the main concept of queue classes structure, but unfortunately only Queue class has the things I need, so I moved some logic there from PendingDispatch class.

We have critical bugs in our project because of that. And we can't bind anything via container, because all objects are created with new.

[driesvints]

Let's see how your PR goes first 👍

[aprokopenko]

Let's see how your PR goes first +1

As I sad, my PR failed with auto-tests, and I can't find any instruction how to configure them correctly to be the same as it does automatically. I even posted a question in Discussions (#47764), but no response on this.
Without this setup I can't complete the PR, because it requires updating existing unit tests (because of main logic of dispatching is different with my update). I fixed all issues and tested manually with my PR, but without fixing unit tests it won't be checked/merged according to rules mentioned in Contributing docs

[aprokopenko]

@driesvints , in case you missed the PR, it was already linked to this issue: #47765.

[driesvints]

I'm going to close this now because no new PR has been made. Feel free to try again if you want 👍