Using lockForUpdate in the already loaded model from the constructor

I mean I have a Job that handles the insertion of rewards to a user when they successfully register.

Here's the code for the controller where I dispatch the ProcessReward job.

$user = User::create...

ProcessReward::dispatch($user)

Here's the code for the ProcessReward job.

// noticed, we already have the User object here

public function __construct(private User $user)
{}

public function handle()
{
    $user = $this->user;

    DB::transaction(function () use ($user) {
        $user->lockForUpdate(); // here I use the lockForUpdate()
        Reward::create([
            'user_id' => $user->id,
            'amount' => 1,
        ]);
    });
}

My question is, Is my approach of calling the lockForUpdate() is okay and has no drawbacks?

I can do this, but I am querying the same use which I think doesn't make sense. Because the user is already loaded in the constructor.

public function handle()
{
    $user = User::where('id', $this->user->id)->lockForUpdate()->first();

    DB::transaction(function () {
        Reward::create([
            'user_id' => $user->id,
            'amount' => 1,
        ]);
    });
}

LaryAI

2 years ago

Level 58

Yes, your approach of calling lockForUpdate() in the ProcessReward job is correct and has no drawbacks. Since you already have the User object loaded in the constructor, you can directly call lockForUpdate() on it without querying the database again.

Here's the modified code for the ProcessReward job:

public function __construct(private User $user)
{}

public function handle()
{
    $user = $this->user;

    DB::transaction(function () use ($user) {
        $user->lockForUpdate(); // lock the user record for update
        Reward::create([
            'user_id' => $user->id,
            'amount' => 1,
        ]);
    });
}

By calling lockForUpdate() on the already loaded User object, you ensure that the user record is locked for update within the transaction. This prevents any other concurrent transactions from modifying the user record until the current transaction is completed.

This approach is more efficient as it avoids an additional database query to fetch the user record again.

2 likes

NoLAstNamE

2 years ago

Level 8

@LaryAI Because this answer is from AI. I really need a second thought from anyone.

NoLAstNamE

2 years ago

Level 8

@LaryAI If I'm not mistaken, because the $user is already loaded in the constructor and we just chain the ->lockForUpdate(); in it. It ends up that we lock it after getting it, right?

NoLAstNamE

2 years ago

Level 8

@LaryAI In the original code, the lockForUpdate() method is called on the $user object within the transaction block, after the $user object has already been retrieved from the database. This means that the selected rows are not locked for update until after the $user object has been retrieved, which could potentially allow other transactions to modify the same rows concurrently.

Is the above statement correct? So this means I really need to query the user again to apply the "lock"?

public function handle()
{
    $user = User::where('id', $this->user->id)->lockForUpdate()->first();

    DB::transaction(function () {
        Reward::create([
            'user_id' => $user->id,
            'amount' => 1,
        ]);
    });
}

1 like

Please or to participate in this conversation.