arq
arq copied to clipboard
Allow `connection_pool` to be set in RedisSettings
@samuelcolvin @chrisguidry Thank you for a great library! It would be really grateful if this PR could be reviewed or a workaround for this issue provided. Many thanks.
We are using arq for our production app, and it appears that connection pool throws an error and arq worker crashes if the connection limit(max_connections) is reached.
Not only does worker but the enqueue_job method also throws an error under the same condition.
This seems to be caused by the default ConnectionPool of redis-py.
https://github.com/redis/redis-py/issues/2517
Code to reproduce:
import asyncio
from arq import ArqRedis, create_pool
from arq.connections import RedisSettings
REDIS_SETTINGS = RedisSettings(
max_connections=1
)
async def foo(ctx) -> None:
print('ok')
async def main() -> None:
redis: ArqRedis = await create_pool(REDIS_SETTINGS)
for _ in range(2):
await redis.enqueue_job('foo')
class Worker:
functions = [foo]
redis_settings = REDIS_SETTINGS
if __name__ == '__main__':
asyncio.run(main())
By allowing connection_pool to be set in RedisSettings, it seems to work as expected.
import asyncio
from arq import ArqRedis, create_pool
from arq.connections import RedisSettings
from redis.asyncio import BlockingConnectionPool
REDIS_SETTINGS = RedisSettings(
connection_pool=BlockingConnectionPool(max_connections=1)
)
async def foo(ctx) -> None:
print("ok")
async def main() -> None:
redis: ArqRedis = await create_pool(REDIS_SETTINGS)
for _ in range(10):
await redis.enqueue_job("foo")
class Worker:
functions = [foo]
redis_settings = REDIS_SETTINGS
if __name__ == "__main__":
asyncio.run(main())
It appears that Worker can take redis_pool as an argument, but I guess we can handle this case more concisely this way for both Worker and Enqueuer, provided it does not break anything else.
:warning: Please install the to ensure uploads and comments are reliably processed by Codecov.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
@@ Coverage Diff @@
## main #473 +/- ##
==========================================
- Coverage 96.27% 96.01% -0.27%
==========================================
Files 11 11
Lines 1074 1079 +5
Branches 209 190 -19
==========================================
+ Hits 1034 1036 +2
- Misses 19 21 +2
- Partials 21 22 +1
| Files | Coverage Δ | |
|---|---|---|
| arq/connections.py | 87.50% <100.00%> (-2.57%) |
:arrow_down: |
Continue to review full report in Codecov by Sentry.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 3f865a6...6048460. Read the comment docs.
I'd also really like smoother support for connection pooling. My Arq enqueuer falls over really quickly in a production setting.