arq icon indicating copy to clipboard operation
arq copied to clipboard

Sentinel timout not exposed

Open meseta opened this issue 4 years ago • 2 comments

Sentinel timeout argument is not exposed. This setting we need to be able to adjust because the default aioredis 1.3.1 timeout value is 0.2s, too short for a lot of use-cases

The relevant code is here. https://github.com/samuelcolvin/arq/blob/88afd6183283dcdf506f9331fd202f9f39ef83ac/arq/connections.py#L216-L236

I think settings.conn_timeout could be passed to aioredis.sentinel.create_sentinel_pool for the timeout argument. Internally, aioredis 1.3.1 passes this value onto the pools that it creates as well, making it work similarly to settings.conn_timeout when not using sentinel. We're currently working around this issue by monkey-patching the default argument in aioredis prior to any arq code.

In the future aioredis 2.0.0, it seems that sentinels can be instantiated with a socket_timeout argument. It would be worth exposing this to RedisSettings in arq to help with clusters that have higher latency. It may be best to wait for aioredis 2.0.0 before addressing this issue

meseta avatar Apr 05 '21 17:04 meseta

PR welcome to fix this.

samuelcolvin avatar Apr 24 '21 18:04 samuelcolvin

happy to help if we're still using arq in a couple months (nothing wrong with it, we're just switching a lot of stuff to kafka). I suggest waiting for aioredis 2.0.0, which releases imminently (see: https://github.com/aio-libs/aioredis-py/issues/930#issuecomment-814957888), which seems to have substantially changed how sentinel is handled. Happy to take a look if we're still using arq at that point

meseta avatar Apr 25 '21 19:04 meseta