laravel/framework

ShouldBeUnique doesn't clear lock when job is finished.

rflatt-reassured opened this issue · 7 comments

  • Laravel Version: 8.47.0
  • PHP Version: 7.4.10
  • Horizon Version 5.7.9

Description:

ShouldBeUnique and ShouldBeUniqueUntilProcessing do not release lock unless a timeout is set, and then it only releases after the timeout has expired, not after the job has been processed.

Steps To Reproduce:

class ... implements ShouldQueue, ShouldBeUnique
{
    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

    private $userId;

    /**
     * The unique ID of the job.
     *
     * @return string
     */
    public function uniqueId()
    {
        return $this->userId;
    }    

    /**
     * Create a new job instance.
     *
     * @return void
     */
    public function __construct($userId)
    {
        $this->userId = $userId;
    }

    /**
     * Execute the job.
     *
     * @return void
     */
    public function handle()
    {
        try {
            //call the stored proc to assign a lead:
            $assigned = \DB::select('CALL ...(' . $this->userId . ')');
            $data = [
                'Status' => 'success',
                'result_id' => $result[0]->id
            ];
            event(new BroadcastToUser($this->userId, 'event', $data));

        } catch (\Exception $e) {
            event(new BroadcastToUser($this->userId, 'event', ['Status' => 'error']));
        }
    }

The above job runs once for $this->userId, but then will never be added to the queue again, unless I remove ShouldBeUnique.

What do you mean with "do not release lock"? There's not a lot of info here. Not sure what's wrong?

I can add a job with a $userId = 123 (for example) to the queue, and it will process it. If I try adding it to the queue again, it won't add it because the lock for $this->userId is still there:

    /**
     * The unique ID of the job.
     *
     * @return string
     */
    public function uniqueId()
    {
        return $this->userId;
    } 

I briefly changed the cache driver for the job locks to be database:

    /**
     * Get the cache driver for the unique job lock.
     *
     * @return \Illuminate\Contracts\Cache\Repository
     */
    public function uniqueVia()
    {
        return Cache::driver('database');
    }

The expiration was set to 24 hours from now. The lock did not seem to be released after the job was processed. There were no failed jobs,

So the lock for userId = 123 still existed, and I could not add any further jobs to the queue for that id, even when there were no jobs in the queue.

What cache driver are you using at first?

I was using redis.

The problem seems to have cleared itself. Removing ShouldBeUnique, sending jobs with the same id, and then putting it back seems to have fixed the issue with no other code changes. It is a bit concerning, but I can no longer replicate the issue.

Good that you got it solved 👍

Not sure how to request to have the issue reopened, but I have the exact same issue with the file cache driver.

When I debug (after the first dispatch), I can clearly see that Illuminate\Foundation\Bus\PendingDispatch\shouldDispatch() returns false while doing:

return $cache->lock(
    $key = 'laravel_unique_job:'.get_class($this->job).$uniqueId,
    $this->job->uniqueFor ?? 0
)->get();

I tried to find where the lock is supposed to be released, but I failed. It seems that the class CallQueuedHandler only checks for ShouldBeUniqueUntilProcessing (and not ShouldBeUnique) before calling ensureUniqueJobLockIsReleased(). I hope I'm following the code correctly.

If anyone runs into this and have multiple servers serving same Laravel app, check that all your servers have same configuration for config/database.php/redis/lock_connection:
https://github.com/laravel/laravel/blob/10.x/config/cache.php#L80

Explanation:
I ran into this issue when migrating to Laravel 10, and also updated config/database.php which had a new 'lock_connection' => 'default'. I have containerized my app into separate app-web, app-queue-worker, app-scheduler containers and after I've done code upgrade I started testing with upgrading only web part which meant:

  • there was new Laravel web code with lock_connection=default , that dispatched jobs setting unique lock in the 'default' redis connection/server
  • queue workers were running with old code and had no lock_connection set (cache), so they would try to release unique lock in the 'cache' redis connection/server which was not there. Meaning jobs kept hanging unless I'd connect to redis-cli and delete locks manually.