clue / reactphp-redis

Async Redis client implementation, built on top of ReactPHP.
https://clue.engineering/2019/introducing-reactphp-redis
MIT License
268 stars 48 forks source link

RESP3 support (Redis 6+) #98

Open WyriHaximus opened 4 years ago

WyriHaximus commented 4 years ago

Looks like Redis 6 is likely only supporting the RESP3 protocol: http://antirez.com/news/125

clue commented 4 years ago

[EDIT! I'm reconsidering all this because Marc Gravell from Stack Overflow suggested that we could just switch protocol for backward compatibility per-connection, sending a command to enable RESP3. That means no longer need for a global configuration that switches the behavior of the server. Put in that way it is a lot more acceptable for me, and I'm reconsidering the essence of the blog post]

Thanks for reporting here, this is interesting! It looks like there won't be a hard switch any time soon, so Redis 6 will continue working as-is. But we should definitely look into providing native RESP3 support for Redis 6 in the future! :+1:

There are currently no immediate plans to build this from my end (no demand at the moment and more important outstanding issues currently), but I would be really happy to accept PRs :+1:

(If you need this for a commercial project and if you want to help sponsor this feature, please reach out and can work out the details)

WyriHaximus commented 4 years ago

There are currently no immediate plans to build this from my end (no demand at the moment and more important outstanding issues currently), but I would be really happy to accept PRs 👍

Yup that's why I filed it 👍

clue commented 3 years ago

Supporting the RESP3 protocol requires non-trivial but reasonable amount of work. The major issue I'm seeing is that it might change the semantics and return values of some commands, so this would be considered a BC break. This would for instance change how HGETALL should return a map (associative array) instead of a multi bulk reply (array/list with key value pairs). Accordingly, we may want to make sure this is the default behavior irrespective of the RESP protocol version, so this would require some kind of manual mapping for RESP2 already (which I view as reasonable, see also related tickets #17 and #103).