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

Error-prone behaviour of SELECT command in combination with connection pool

Open nj-vs-vh opened this issue 5 months ago • 1 comments

Version: redis-py 5.0.1, Redis 7.2.4

Platform: Python 3.10.13 on MacOS 13.3.1

Description: Hi! I realize this is not exactly an issue with the library, but I still want to point this out after spending several hours debugging. Consider the following code:

import asyncio
import os
from redis.asyncio import Redis

async def main() -> None:
    r = Redis.from_url(os.environ["REDIS_URL"])
    await r.select(1)
    await r.set("key", b"hello world")

    async def read():
        print(await r.get("key"))

    await asyncio.gather(
        read(),
        read(),
    )

asyncio.run(main())

That code produces (for me, very consistently)

b'hello world'
None

What happens is:

  • the client runs SELECT, which sets db to 1 for one of the connections in the pool (probably the first one)
  • the subsequent SET goes through the same connection and creates "key" in db 1
  • to serve parallel call, an other connection is pulled from the pool, having default db 0 set, and the GET produces no result

Just like that, in a couple lines of code, I have produced an UB and created a bunch of hard-to-find bugs down the line. The root of the problem is that the Redis object abstraction hides the underlying multi-connection complexity. I have read the SELECT documentation page and used the mental model from there to write my Python code, which, it appears, does not apply here.

I suggest possible enhancements:

  • add this caveat to the documentation for select and other stateful connection-level methods (or even create a dedicated section)
  • add helper methods like select_for_all that would run the command for all connections in the pool and set the new default value for the db as new default -- thus making it compatible with single-connection mental model
  • emit a warning or even raise an exception when calling select (and similar stateful connection-level functions), if the client is not in single connection mode; they create an inherent UB since the caller can't know which connection will be modified as a result of the call.

nj-vs-vh avatar Jan 27 '24 22:01 nj-vs-vh

I'll be happy to make a PR, given a green light and some guidance from maintainers.

nj-vs-vh avatar Jan 27 '24 22:01 nj-vs-vh