Improve Redis key watcher
I did a spin on Redis key watcher by testing !128 (merged).
Everything works as expected, the code is behaving correctly, but there are some quirks.
- I had troubles connecting to unix socket. I tried
URL = "unix:///var/opt/gitlab/redis/redis.socket"
andURL = "unix:/var/opt/gitlab/redis/redis.socket"
. I would expect the first to work:
2017-03-02_17:32:46.77638 2017/03/02 17:32:46 error: Failed to connect to redis: dial unix: missing address
- Is there a reason why ReadTimeout is an integer, not a
time.Duration
as we do it in other places? - I had
ReadTimeout = 1
, which introduced 1ms read timeout on subscribing, -
ReadTimeout
forPubSub.Receive
doesn't make sense. With default being 1s we basically havei/o timeout
every one second on this method due to the reading deadline. This has bad consequences as the time needed to reconnect and setup new subscription is non-zero and during that we loosely published notifications. Of course, we have to not receive a message during theReadTimeout
. Depending on network conditions we loose roughly 10ms of notifications. I propose to introduceWatchTimeout
, to be separate fromReadTimeout
that would be used forGET
.WatchTimeout
should be a number, with the minimum being60s
. Probably something like5m
or10m
. It is still susceptible to notification loose, but this reduces the risk from roughly 1% to 0.001%. - We have
ReadTimeout
, but I don't see equivalents forConnectTimeout
andWriteTimeout
. Is this intended?
@bkc Thoughts?