Skip to content

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.

  1. I had troubles connecting to unix socket. I tried URL = "unix:///var/opt/gitlab/redis/redis.socket" and URL = "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
  1. Is there a reason why ReadTimeout is an integer, not a time.Duration as we do it in other places?
  2. I had ReadTimeout = 1, which introduced 1ms read timeout on subscribing,
  3. ReadTimeout for PubSub.Receive doesn't make sense. With default being 1s we basically have i/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 the ReadTimeout. Depending on network conditions we loose roughly 10ms of notifications. I propose to introduce WatchTimeout, to be separate from ReadTimeout that would be used for GET. WatchTimeout should be a number, with the minimum being 60s. Probably something like 5m or 10m. It is still susceptible to notification loose, but this reduces the risk from roughly 1% to 0.001%.
  4. We have ReadTimeout, but I don't see equivalents for ConnectTimeout and WriteTimeout. Is this intended?

@bkc Thoughts?