steve0511 / resty-redis-cluster

Openresty lua client for redis cluster.
Apache License 2.0
376 stars 134 forks source link

feat: add max_connection_timeout configuration parameter #85

Closed NivLipetz closed 3 years ago

NivLipetz commented 3 years ago

closes #84

incase we want to be in control of how long it takes us to connect to a redis cluster, we should be able to control the timeout time, this PR includes this functionality.

This is the error message that is logged when a timeout happens:

msg:2021/06/21 20:20:40 [error] 50#0: *9280 [lua] rediscluster.lua:124: try_hosts_slots(): max_connection_timeout of 10000ms reached., client: 127.0.0.1, server: kong, request: "GET /mickey/health HTTP/1.1", host: "mickey.co"
NivLipetz commented 3 years ago

Hi @NivLipetz. The PR looks good. we also use this client and need this feature ! did you test it locally under load ?

Thanks @shyimo , we tested this under load with 1000RPS and it acts as expected, instead of taking 20+seconds for each request, the requests timeout after 10-12seconds with the 10second max_connection_timeout configuration

steve0511 commented 3 years ago

LGTM, thanks for contributing.

NivLipetz commented 3 years ago

Happy to contribute 🙂

@steve0511 do you have any estimations as to when you will release this to luarocks? We would like to install this pkg with the fix for our deployments. Thx again!

steve0511 commented 3 years ago

@NivLipetz updated luarocks.