aioredis-py icon indicating copy to clipboard operation
aioredis-py copied to clipboard

Health check fails when a pubsub has no subscriptions

Open bmerry opened this issue 3 years ago • 2 comments

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

{'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

  • [X] I agree to follow the aio-libs Code of Conduct

bmerry avatar Nov 17 '21 07:11 bmerry

Thanks for the report! PR #1207 created

Andrew-Chen-Wang avatar Nov 17 '21 22:11 Andrew-Chen-Wang

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

bmerry avatar Nov 18 '21 05:11 bmerry