aio-libs-abandoned / aioredis-py

asyncio (PEP 3156) Redis support
https://aioredis.readthedocs.io/
MIT License
2.3k stars 336 forks source link

Health check fails when a pubsub has no subscriptions #1206

Open bmerry opened 2 years ago

bmerry commented 2 years ago

Describe the bug

When a PubSub needs to issue a PING due to the health check feature, it does not consider that there might be no subscriptions at the moment. Redis responds differently to PING depending on whether there are active subscriptions or not: if there are no subscriptions it just returns the argument as a bulk response, instead of a multi-bulk with "pong" and the response. This breaks the code that detects the health check response, and instead the individual bytes of the aioredis-py-health-check string get inserted into the returned message.

To Reproduce

  1. Install aioredis 2.0.0
  2. Run this code:
    
    #!/usr/bin/env python3

import asyncio

import aioredis

async def poll(ps): while True: message = await ps.get_message(timeout=1) if message is not None: print(message)

async def main(): r = aioredis.Redis.from_url("redis://localhost", health_check_interval=2) ps = r.pubsub() await ps.subscribe("foo") poller = asyncio.create_task(poll(ps)) await asyncio.sleep(5) await ps.unsubscribe("foo") await asyncio.sleep(5) await ps.subscribe("baz") poller.cancel() try: await poller except asyncio.CancelledError: pass

asyncio.run(main())


### Expected behavior

Expected all messages printed to have proper types.

### Logs/tracebacks

```python-traceback
{'type': 'subscribe', 'pattern': None, 'channel': b'foo', 'data': 1}
{'type': 'unsubscribe', 'pattern': None, 'channel': b'foo', 'data': 0}
{'type': 97, 'pattern': None, 'channel': 105, 'data': 111}
{'type': 97, 'pattern': None, 'channel': 105, 'data': 111}

Note that 97, 105, 111 are the result of indexing b"aioredis-py-health-check" with indices 0, 1, 2.


### Python Version

```console
$ python --version
Python 3.8.10

aioredis Version

$ python -m pip show aioredis
Name: aioredis
Version: 2.0.0

Additional context

redis-py seems to have a similar bug with the interaction between health checks and pub-sub, but the failure mode is not the same (in redis-py it seems to be some sort of race condition, whereas in aioredis it appears reliably reproducible), so this might need an aioredis-specific fix.

Code of Conduct

Andrew-Chen-Wang commented 2 years ago

Thanks for the report! PR #1207 created

bmerry commented 2 years ago

Thanks, that was fast. I should be able to do a review on Monday.