monospice / laravel-redis-sentinel-drivers

Redis Sentinel integration for Laravel and Lumen.
MIT License
101 stars 48 forks source link

Add Standalone option #30

Closed dreadkopp closed 4 years ago

dreadkopp commented 4 years ago

There might be cases where a total overwrite of the redis connection is not working.

i.e. Session,Cache,Queue is handled internally with sentinel. However when there are i.e. external Queues using redis as well /wo sentinel. For those we need some option to get a non-redis connection from the manager

cyrossignol commented 4 years ago

Hi @dreadkopp, I'm not sure that I understand what this achieves. Can you please explain your use case a bit more?

This package doesn't force an application to use Sentinel for all Redis connections. It's designed so that standard Redis connections and Sentinel connections can coexist. If you'd like to use both in the same application, you can skip the step that sets the REDIS_DRIVER configuration directive to redis-sentinel. Then, add any standard Redis connections to the redis array in config/database.php and use those connections like you would without this package.

dreadkopp commented 4 years ago

@cyrossignol

nevermind. I guess i did not understand laravel global switches correctly.

I set ENV REDIS_DRIVER to 'redis-sentinel' and was using that environment variable in config/*.php

However that switch seems to go much deeper denying access to 'simple' redis driver even when explicitly set in the config ('driver' => 'redis')

Thus this was my apporach to use a 'simple' redis connection for some queueseven with env REDIS_DRIVER set to 'redis-sentinel'

Rewrote the configs and everything is happy now except for a few 'No sentinel server available for autodiscovery.' fails

cyrossignol commented 4 years ago

However that switch seems to go much deeper denying access to 'simple' redis driver even when explicitly set in the config ('driver' => 'redis')

Thanks for the feedback. The package shouldn't overwrite existing configuration values with environment variables. There's a test case that checks this. If it is, please open an issue.

Rewrote the configs and everything is happy now except for a few 'No sentinel server available for autodiscovery.' fails

This can happen on networks with occasional latency spikes because the default Predis timeout for Sentinel connections is 0.1 seconds. You can increase the package's sentinel_timeout configuration value if this becomes a problem. More information in this ticket.

dreadkopp commented 4 years ago

Thanks @cyrossignol ,

i saw that ticket when trying to get rid of that problem.

Using Predis 1.1.1 and a sentinel_timeout of 0.8 (which is seconds afair, ping to sentinels is < 1ms though). Problem still persists.

Anywho that shouldn't be part of this discussing or PR. I am closing this since it was a unnecessity in the first place ;)