Skip to content

Ensure BLPOP/BRPOP returns nil instead of raising ReadTimeoutError

Stan Hu requested to merge sh-patch-redis-client-16-10 into 16-10-stable-ee

What does this MR do and why?

This backports !153282 (merged) to 16-10-stable-ee.

This addresses a regression in the redis-client gem, which was upgraded in GitLab 16.10 in !145203 (merged). This problem happens when Redis Sentinels are in use.

In https://github.com/redis/redis-rb/issues/1279, we discovered that when using BLPOP or BRPOP redis-rb properly returned nil when timeout was reached with no key present, but when connecting to Redis Sentinels, the client raised a ReadTimeoutTimeout error.

This occurred because of a subtle difference in how RedisClient (from redis-rb) and Redis::Client (from redis-client) behaved. The former, which is used with standalone Redis, returned nil because the socket read timeout was incremented to the command timeout value (https://github.com/redis/redis-rb/pull/1175). The latter did not have this, so the socket read timeout would get triggered before the actual Redis timeout hit.

To make the behavior consistent, increment the configured read timeout to the command timeout.

This commit includes the patch in https://github.com/redis-rb/redis-client/pull/197 to fix this issue until an official redis-client patch release can be made.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

  • This MR is backporting a bug fix, documentation update, or spec fix, previously merged in the default branch.
  • The MR that fixed the bug on the default branch has been deployed to GitLab.com (not applicable for documentation or spec changes).
  • This MR has a severity label assigned (if applicable).
  • Set the milestone of the merge request to match the target backport branch version.
  • This MR has been approved by a maintainer (only one approval is required).
  • Ensure the e2e:package-and-test-ee job has either succeeded or been approved by a Software Engineer in Test.

Note to the merge request author and maintainer

If you have questions about the patch release process, please:

Edited by Stan Hu

Merge request reports