`:blpop` returns an exception when using sentinels

[jlledom]

The documentation says :blpop should return nil when timed out. However when connecting to sentinels, timing out raises a RedisClient::ReadTimeoutError.

I wrote this script to reproduce it:

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem 'redis'
  gem 'hiredis-client'
end

config = {
  url: 'redis://redis-master',
  name: 'redis-master',
  sentinels: [
    { host: 'localhost', port: 26379},
    { host: 'localhost', port: 26380},
    { host: 'localhost', port: 26381}
  ],
}
client = Redis.new(config)

while true
  begin
    sleep 1
    result = client.blpop('foo', timeout: 1)
    puts result || 'No result'
  rescue RedisClient::ReadTimeoutError => e
    puts e.message
    retry
  end
end

The problem comes from the hiredis-client gem which actually raises an exception for sentinels but returns nil for single redis instances. So this is probably affecting other blocking commands.

[stanhu]

How timely. I just ran into this issue as well, but I'm not using the hiredis-client gem:

/Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client/ruby_connection/buffered_io.rb:214:in `block in fill_buffer': Waited 2 seconds (RedisClient::ReadTimeoutError)
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client/ruby_connection/buffered_io.rb:197:in `loop'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client/ruby_connection/buffered_io.rb:197:in `fill_buffer'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client/ruby_connection/buffered_io.rb:187:in `ensure_remaining'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client/ruby_connection/buffered_io.rb:152:in `getbyte'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client/ruby_connection/resp3.rb:113:in `parse'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client/ruby_connection/resp3.rb:50:in `load'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client/ruby_connection.rb:98:in `block in read'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client/ruby_connection/buffered_io.rb:114:in `with_timeout'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client/ruby_connection.rb:98:in `read'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client/connection_mixin.rb:31:in `call'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client.rb:360:in `block (2 levels) in blocking_call_v'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client/middlewares.rb:16:in `call'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client.rb:359:in `block in blocking_call_v'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client.rb:699:in `ensure_connected'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-client-0.22.1/lib/redis_client.rb:358:in `blocking_call_v'
        from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/redis-5.2.0/lib/redis.rb:160:in `block in send_blocking_command'

[stanhu]

Notice how _client is different depending on whether Sentinels are in use:

irb(main):001> require 'redis'
=> true
irb(main):002> redis_sentinel = Redis.new(url: 'redis://mymaster', name: 'mymaster', sentinels: [ { host: 'localhost', port: 26379 }])
=> #<Redis client v5.2.0 for redis://127.0.0.1:6381>
irb(main):003> redis_sentinel._client
=> #<RedisClient redis://127.0.0.1:6381>

irb(main):004> redis = Redis.new(url: 'redis://localhost:6381')
=> #<Redis client v5.2.0 for redis://localhost:6381>
irb(main):005> redis._client
=> #<Redis::Client redis://localhost:6381>

In the Redis Sentinel case, RedisClient from the redis-client library is used, but in the standalone Redis case, Redis::Client is used: https://github.com/redis/redis-rb/blob/13f324667841d5a923e90aa9fc6a7a540a6ecc96/lib/redis/client.rb#L22-L28

The latter increments the timeout in Redis::Client#blocking_call_v:

https://github.com/redis/redis-rb/blob/13f324667841d5a923e90aa9fc6a7a540a6ecc96/lib/redis/client.rb#L95-L101

However, this doesn't happen in RedisClient#blocking_call_v: https://github.com/redis-rb/redis-client/blob/a2f16fc7dbdaff3f8d3d396e8ec7c3826d2f9fcc/lib/redis_client.rb#L355-L364

I've validated that when I added a similar timeout mechanism to RedisClient, the issue appears to go away.

@byroot What do you think? Should _client be consistent?

[stanhu]

https://github.com/redis-rb/redis-client/pull/197 should fix this.