Skip to content

Conversation

@vadimonus
Copy link
Contributor

@vadimonus vadimonus commented Jan 16, 2025

Currently Laravel declares some support for Redis Cluster, but some features are not working (for example, job batches).
This PR adds Queue Integration Tests run with Redis Cluster (on both predis and phpredis). This will allow to check if new and existing features are compatible with Redis Cluster.

Some changes were made to run tests in new environment.

  • InteractsWithRedis trait extended, to provide clustered config for redis, if REDIS_CLUSTER_HOSTS_AND_PORTS env variable is passed. This is considered backward compatible change, as currently env with this name is not described anywhere.
  • InteractsWithRedis also sets RedisManager instance for application container. Without this change application in tests tries to use application configs leading to behaviour inconsistency (as config in InteractsWithRedis and in application differs). Added cache to config in InteractsWithRedis, to make configs in trait same as in app, and do not break tests, that previously used app configs. This change may be considered backward incompatible. On the other hand, behaviour of InteractsWithRedis is not described anywhere, and is intended to be used only in tests, and this change may be treated as not breaking contracts.
  • InteractsWithRedis trait added to RateLimitedWithRedisTest (unlike other tests that interacts with redis, that test missed this trait).
  • Redis cluster queue should use {default} queue name instead of queue. RedisQueueTest updated, to take queue name from config. This allow to pass right queue name when tested in cluster.
  • testBulkJobQueuedEvent is skipped with redis cluster connection (fix for bulk with cluster and unskipping test will be in separate PR, simular to Job Batches with Redis Cluster #54205)
  • redis-cluster example in docker-compose.yml to run local tests with cluster
@github-actions
Copy link

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

@vadimonus vadimonus marked this pull request as ready for review January 18, 2025 09:09
@taylorotwell taylorotwell marked this pull request as draft January 19, 2025 17:37
@vadimonus
Copy link
Contributor Author

@taylorotwell returned to draft. Please, clarify, if i should update something

@vadimonus vadimonus marked this pull request as ready for review February 4, 2025 16:30
@taylorotwell
Copy link
Member

Hey @vadimonus - can you let me know how to actually run the tests against Redis Cluster? Please mark as ready for review when answered.

@taylorotwell taylorotwell marked this pull request as draft February 5, 2025 11:53
@vadimonus
Copy link
Contributor Author

vadimonus commented Feb 5, 2025

@taylorotwell , sorry, lost one file while rebasing.

This PR adds new entry to queues.yml. It will create minimal Redis Cluster with 3 nodes and run tests with it (both drivers)
It you want to run test locally, you can uncomment redis-cluster containers in docker-compose.yml and specify in .env REDIS_CLUSTER_HOSTS_AND_PORTS=redis-cluster-0:7000,redis-cluster-1:7001,redis-cluster-2:7002 and REDIS_QUEUE='{default}'

Currently Laravel redis config does not allow to switch to redis-cluster only by providing some env. So fixes in InteractsWithRedis.php are made to switch to provide clustered config if REDIS_CLUSTER_HOSTS_AND_PORTS env is set.

@vadimonus vadimonus marked this pull request as ready for review February 5, 2025 15:42
@taylorotwell taylorotwell merged commit d30565a into laravel:11.x Feb 6, 2025
40 checks passed
@vadimonus vadimonus deleted the redis-cluster-queue-tests branch February 8, 2025 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants