-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
Error removing jobs from the reserved queue #47356
Comments
Heya, thanks for reporting. We'll need more info and/or code to debug this further. Can you please create a repository with the command below, commit the code that reproduces the issue as one separate commit on the main/master branch and share the repository here? Please make sure that you have the latest version of the Laravel installer in order to run this command. Please also make sure you have both Git & the GitHub CLI tool properly set up. laravel new bug-report --github="--public" Please do not amend and create a separate commit with your custom changes. After you've posted the repository, we'll try to reproduce the issue. Thanks! |
Is this reproducible when the |
@tillkruss seems that works: |
My first thought is Lua scripts. They are used by the Queue component, but will disregard any For this scenario the only solution is: Don't use data mutations for Alternatively, refactor everything to not use Lua (not a bad idea) and use atomic It may be good to actually unset the two |
@tillkruss nice finds. Thanks! @andrej-schaefer are you willing to work on a PR? |
@driesvints im sorry but im not BTW: do i have to create the commit for the reproduction if it is already reproduced :), due to my limited time it will take a while. |
No worries. I'll keep this open then and see if anyone can help here. |
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! |
Maybe that's something for the docs? |
I mean, when creating unique jobs through ShouldBeUnique interface it means it is unique for the current execution, that multiple workers are not picking a job with the same jobId in parallel? |
@tillkruss : Would this be something relay.so, being the successor of phpredis, would/could handle differently? Besides, I'd guess there's quite a connection to #40569 & #41718 , which sadly wasn't tackled back than. |
@Huggyduggy: Theoretically, yes. But I think it's much better to unset any compression/serializer on the |
Thanks @tillkruss - I've drafted a PR, fixing the same issue as reported by @andrej-schaefer. I'll submit it against the current 10.x branch tomorrow. Sad that it's prohibiting using compression within queues, guess we'll stick to manually compressing our larger objects within our event listeners & jobs prior to dispatching 😌 |
Can somebody advise whether this issue would also affect Laravel Horizon. I can't reproduce the issue from a quick test. |
Looks like Taylor noted he'd appreciate a note being added to the docs: #48180 (comment). |
Laravel Version
10.13.1
PHP Version
8.2.6
Database Driver & Version
No response
Description
We encountered problems with job uniqueness. The jobs, which are configured to have a maximum of 1 attempt, failed multiple times and couldn't be written to the failed_jobs table due to the failed_jobs_uuid_unique constraint.
It seems that the failed_jobs_uuid_unique constraint may be a separate bug for jobs that have a "ShouldBeUnique" implementation, and there is an issue with error handling when they fail multiple times. Upon further investigation through the stack trace, we found that the issue was caused by the MaxAttemptsExceededException.
After removing the unique constraint from the database (for easier debugging), we narrowed down the error to the following log line:
After diving deeper into debugging, we discovered that the jobs were not being removed from the "reserved" queue correctly, and the workers were executing them again, resulting in the MaxAttemptsExceededException.
Further examination of the source code revealed a failed removal of entries in \Illuminate\Queue\RedisQueue::deleteReserved. The result of the execution was always 0, indicating that the entry was not being removed.
After spending more hours experimenting with different Redis clients and configurations, we found that setting either compression and/or serialization in the Redis database configuration resulted in a faulty zrem request to Redis. As a workaround, we disabled the two options for the default database, which is used by the worker, and did not investigate the problem further.
Our hypothesis is that the lack of compression or serialization in the Redis client causes a mismatch between the member in the zrem request, leading to the entry not being deleted.
Workaround was to disable serializer and compression
Hope i haven't forgotten anything :)
Thanks
Steps To Reproduce
Configure redis DB
phpredis as client
serializer option 2 (Redis::SERIALIZER_IGBINARY)
compression option set to 2 (Redis::COMPRESSION_ZSTD)
Create a simple job with tries set to 1 (I have also added the ShouldBeUnique to provocate the DB constraint error which from my point of view is a separate error)
Dispatch the job and wait for second execution
As described above this will result in multiple executions on the second worker run due to the job was not removed from the
reserved
queueThe text was updated successfully, but these errors were encountered: