framework icon indicating copy to clipboard operation
framework copied to clipboard

onOneServer() locks are not stored on lock_connection

Open mortenscheel opened this issue 8 months ago • 5 comments

Laravel Version

12.10.2

PHP Version

8.3.20

Database Driver & Version

No response

Description

Context

In config/cache.php you can define a separate connection to use for locks: https://github.com/laravel/laravel/blob/f6e4638ee6ca1cd40aa7c56311d89ea3d91a24f8/config/cache.php#L74-L78

This makes it possible to run cache:clear without losing mutexes and atomic locks. The lock_connection is used when calling

  • Cache::lock(...)->get()
  • Schedule::command(...)->withoutOverlapping()

It might be used in other contexts too, but those are the ones I'm aware of.

The problem

Schedule::command(...)->onOneServer() creates the lock on the regular cache connection via https://github.com/laravel/framework/blob/bcc92201ab6d786477fed578cc2e3ea4c4e1b093/src/Illuminate/Console/Scheduling/CacheSchedulingMutex.php#L41-L46

Which means that cache:clear will wipe those locks and allow another server to pick up the task.

Thoughts

I'm not familiar enough with the source to send a PR, and I don't know if there's a good reason for this behavior. Naively I would assume that onOneServer() should use CacheEventMutex in stead of CacheSchedulingMutex(which is apparently only used in this context).

Steps To Reproduce

  1. Use a cache driver with a separate lock_connection
  2. Configure a scheduled job with onOneServer(), which calls sleep(60).
  3. Run schedule:run
  4. Observe where the lock is stored.

mortenscheel avatar Apr 30 '25 12:04 mortenscheel

Some people are suggesting complicated workarounds for this issue: https://stackoverflow.com/questions/79183842/laravel-ononeserver-method-of-task-scheduler-break-with-php-artisan-cacheclea

mortenscheel avatar Apr 30 '25 12:04 mortenscheel

https://github.com/laravel/framework/blob/e333cc80b8f5414daec8ef7b15c1333685c4988b/src/Illuminate/Foundation/Console/Kernel.php#L312-L314

Laravel uses this value.

crynobone avatar May 07 '25 06:05 crynobone

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

github-actions[bot] avatar May 07 '25 06:05 github-actions[bot]

@crynobone as far as I can see, that just determines which cache store to use. It still won't use that store's lock_connection.

Of course you could define a dedicated cache store and use it via this undocumented environment variable, but I don't see why that should be necessary when we already use lock_connection for all the other lock types. It just seems inconsistent. Maybe there's a good reason, but lock_connection isn't documented at all, so it's hard to say what it's actually meant for.

mortenscheel avatar May 07 '25 08:05 mortenscheel

@crynobone So if I need to use a redis on my schedule cache I can define

// config/cache.php

return [
    'schedule_store' => 'redis'
];

or set the SCHEDULE_CACHE_STORE env var with redis?

rafa-acioly avatar May 16 '25 13:05 rafa-acioly

Should we close this now after merging #56472?

AhmedAlaa4611 avatar Aug 14 '25 18:08 AhmedAlaa4611

Thanks for fixing 👍

mortenscheel avatar Aug 18 '25 13:08 mortenscheel