[13.x] Make the Redis queue driver cluster-safe (bulk() node-less MULTI; allQueueNames() uses KEYS) - #61198
Open
Orrison wants to merge 1 commit into
Open
[13.x] Make the Redis queue driver cluster-safe (bulk() node-less MULTI; allQueueNames() uses KEYS)#61198Orrison wants to merge 1 commit into
bulk() node-less MULTI; allQueueNames() uses KEYS)#61198Orrison wants to merge 1 commit into
Conversation
Signed-off-by: Kevin Ullyott <kevin.ullyott@canyongbs.com>
Member
|
How did you run into this on your app? |
Contributor
Author
We use an AWS Redis serverless cache and I recently adopted the https://github.com/cboxdk/laravel-queue-autoscale package to set up an auto scaling queue worker process in ECS. So ran into this right away when setting that up, as Redis serverless is always a cluster behind a proxy. And we use queues and batches quite heavily. I've gotten around it and am running well in production by overriding these internal classes to apply these fixes in the interim. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes two Redis-Cluster issues in the queue driver, both gated to phpredis cluster connections so non-cluster behavior is unchanged:
RedisQueue::bulk()silently drops batched jobs. On aPhpRedisClusterConnectionit wraps its pushes intransaction(), which callsRedisCluster::multi()with no node. A cluster transaction must bind to one node, but the pushes route to the queue's{hash tag}slot. Soexec()returnsfalsewith no exception, the jobs never enqueue, and the connection is poisoned.Bus::batch()therefore reports success and recordspending_jobs, but the batch is stuck forever. Only real multi-shard clusters hit it (a single-node cluster hides it). Fix: push each job directly on the cluster branch (the pushes are independent and share one slot) the same "special-case the cluster connection" pattern the framework already uses inCache\RedisStore::putMany(),RedisTaggedCache::flush(), andRedisBroadcaster.RedisQueue::allQueueNames()usesKEYS('queues:*'), which is unavailable on managed clusters (e.g. ElastiCache Serverless) and, where available, enumerates per-node (partial on multi-shard). Fix: on a cluster connection,SCANacross all master nodes instead. (Only reached viaQueue::all*Jobs(), used byQueueFakein core, latent but worth closing for cluster completeness.)Non-cluster and Predis paths are untouched. Adds cluster + non-cluster tests for both.