search

redis / ioredis

🚀 A robust, performance-focused, and full-featured Redis client for Node.js.
MIT License
14.44k stars 1.2k forks source link

[Feature Request]: High integrity mode (already implemented in another client/s) #1925

Open avislavkin opened 1 month ago

avislavkin commented 1 month ago

This is a client-library alternative suggestion to https://github.com/redis/redis/issues/13240, to address the issue of protocol desync going undetected for at least "some" commands and causing mayhem. Full details of suggested resolution and implementation are here: https://github.com/StackExchange/StackExchange.Redis/issues/2706

uglide commented 1 month ago

@avislavkin Could you share more details about the use-case and environment where you experienced RESP "desync" issues?

avislavkin commented 1 month ago

@uglide, I haven't come across those issues. You can find all the details I have here. @mgravell might be able to share some use cases he's encountered.

mgravell commented 1 month ago

The short version here would be "heck knows". This has been reported stupendously rarely; cause could be anything from a rare edge-case bug at any layer, including things like the client, the framework, TLS, the CPU/RAM, the NIC/FPGA, any intermediary gateway/proxy, or the server. I do know that on at least one occasion VeryBadThingsTM have been observed, and there is no intended finger-pointing at any specific layer, least of all Redis itself. The point of the feature in SE.Redis is as a defence-in-depth safeguard: "Impossible things happened? We should at least detect that there was a problem,and not propagate incorrect results". SE.Redis is arguably a lot more vulnerable to this problem because it makes much higher use of multiplexing than most other clients as the default and primary mechanism. If that isn't true for ioredis (you can't really desync on lease/request/response/return cycle): you might not need this.

avislavkin commented 1 month ago

@uglide, considering the problem statement that @mgravell mentioned earlier, could ioredis encounter the same issue? Does ioredis require this feature as well?