Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

redis.asyncio.Sentinel doesn't provide a proper way to clear its child connection pools #3201

Open
evgenybf opened this issue Apr 9, 2024 · 0 comments

Comments

@evgenybf
Copy link

evgenybf commented Apr 9, 2024

Version: What redis-py and what redis version is the issue happening on?
redis-py=-5.0.3, master

Platform: What platform / version? (For example Python 3.5.1 on Windows 7 / Ubuntu 15.10 / Azure)
python 3.9
Windows 10/ Ubuntu 22.04

Description: Description of your issue, stack traces from errors and code that reproduces the issue

from redis.asyncio.sentinel import Sentinel

# Sentinel.__init__ creates a list of Redis clients - one for each host. Every Redis client contains its own connection pool.
# https://github.com/redis/redis-py/blob/07fc339b4a4088c1ff052527685ebdde43dfc4be/redis/asyncio/sentinel.py#L215
# https://github.com/redis/redis-py/blob/07fc339b4a4088c1ff052527685ebdde43dfc4be/redis/asyncio/client.py#L168C9-L168C65
sentinel = Sentinel([('host1', 26379), ('host2', 26379)], socket_timeout=0.1)

# master_for() creates a new SentinelConnectionPool
# https://github.com/redis/redis-py/blob/07fc339b4a4088c1ff052527685ebdde43dfc4be/redis/asyncio/sentinel.py#L350
master = sentinel.master_for('mymaster', socket_timeout=0.1)

await master.set('foo', 'bar')

# Closes a single Redis ConnectionPool instance
# https://github.com/redis/redis-py/blob/07fc339b4a4088c1ff052527685ebdde43dfc4be/redis/asyncio/client.py#L576
await master.aclose(close_connection_pool=True)

Since, not all connection pools are closed, testing it withIsolatedAsyncioTestCase we get the following warning when sentinel is being garbage collected:

/usr/local/lib/python3.9/site-packages/_pytest/unraisableexception.py:78: PytestUnraisableExceptionWarning: Exception ignored in: <function AbstractConnection.__del__ at 0x772f81b31550>
  
  Traceback (most recent call last):
    File "/usr/local/lib/python3.9/site-packages/redis/asyncio/connection.py", line 216, in __del__
      self._close()
    File "/usr/local/lib/python3.9/site-packages/redis/asyncio/connection.py", line 223, in _close
      self._writer.close()

A workaround is to close all connections in the sentinel.sentinels list too:

for s in sentinel.sentinels:
    await s.aclose()
@evgenybf evgenybf changed the title asyncio Sentinel doesn't provide a proper way to clear its child connection pools redis.asyncio.Sentinel doesn't provide a proper way to clear its child connection pools Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant