Skip to content
This repository has been archived by the owner on Feb 21, 2023. It is now read-only.

Health check fails when a pubsub has no subscriptions #1206

Open
1 task done
bmerry opened this issue Nov 17, 2021 · 2 comments · May be fixed by #1207
Open
1 task done

Health check fails when a pubsub has no subscriptions #1206

bmerry opened this issue Nov 17, 2021 · 2 comments · May be fixed by #1207
Labels

Comments

@bmerry
Copy link
Collaborator

bmerry commented Nov 17, 2021

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

  • I agree to follow the aio-libs Code of Conduct
@bmerry bmerry added the bug label Nov 17, 2021
@Andrew-Chen-Wang Andrew-Chen-Wang linked a pull request Nov 17, 2021 that will close this issue
5 tasks
@Andrew-Chen-Wang
Copy link
Collaborator

Andrew-Chen-Wang commented Nov 17, 2021

Thanks for the report! PR #1207 created

@bmerry
Copy link
Collaborator Author

bmerry commented Nov 18, 2021

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants