laminas / laminas-cache-storage-adapter-redis

BSD 3-Clause "New" or "Revised" License
8 stars 12 forks source link

[RFC]: `RedisCluster` with `shuffle` for nodes #80

Closed boesing closed 2 months ago

boesing commented 11 months ago

RFC

Q A
Proposed Version(s) 2.7.0
BC Break? No

Goal

Shuffle redis nodes before passing them to RedisCluster instantiation.

Background

I somehow stumbled over https://symfony.com/doc/current/configuration/env_var_processors.html and saw the example of shuffle in the documentation:

# config/packages/framework.yaml
parameters:
    env(REDIS_NODES): "127.0.0.1:6380,127.0.0.1:6381"
services:
    RedisCluster:
        class: RedisCluster
        arguments: [null, "%env(shuffle:csv:REDIS_NODES)%"]

I wonder if it has an actual performance impact for high-load projects when the clusters are actually shuffled. I haven't checked this already but wanted to dump this idea here as an RFC.

Considerations

This could have performance gains in case it turns out that it is helpful.

Proposal(s)

Add shuffle to RedisClusterResourceManager where the seeds are passed to the RedisCluster#__construct.

Appendix

Ref: https://github.com/phpredis/phpredis/discussions/2457#discussioncomment-9843874

boesing commented 2 months ago

Closing here as redis does that already under the hood as stated here: https://github.com/phpredis/phpredis/discussions/2457#discussioncomment-9847916