Upgrading from 1.3.2 to 2.0.2, redis-rb gem support

[notarize-tgall]

Our codebase has some built in redis connection pooling, which utilizes the redis gem for connections. I see when you pass a URL, connections are now established with redis-client.

For cases where a user supplies a client, rather than a URL, should redis still be supported?

The one bump I'm running into is that script registration is occurring by rescuing RedisClient::CommandError, whereas the exception with the redis gem is Redis::CommandError. If I patch this locally, things seem to be working ok.

Is support for redis completely dropped? If not, I'm happy to open a PR to rescue Redis::CommandError in addition to RedisClient::CommandError.

[pboling]

I have a similar issue. We followed the same upgrade path and have complex Redis cluster/sentinel/role/host/port configurations across many environments. I have no idea how to translate all that to RedisClient, and if it can be avoided I'd prefer to stick with the existing configs, which are targeting Redis.new instead of RedisClient.new. Sending the Redis.new into Redlock is of course not working at all... but could it?

[notarize-tgall]

@pboling We ended up monkey patching Redlock::Client::RedisInstance#recover_from_script_flush to, on Redis::CommandError, re raise as RedisClient::CommandError.

Still willing to open a PR to address this issue, if the maintainers are open to it. We could have a configuration parameter that, ultimately, determines which error in recover_from_script_flush gets rescued.

[pboling]

Makes me wonder why Redis::CommandError doesn't inherit from RedisClient::CommandError...

Maybe that monkey patch would be another alternative.

[pboling]

Looking more into the super class structure, it doesn't seem like a good idea to patch the class hierarchy. :(

[notarize-tgall]

Here's a gist of the monkey patch to support redis-rb redlock_redis-rb_support.rb

[jpheos]

I have exactly the same problem, and for me it's was because I keep an old config after upgrade gem.

I had that:

ActiveJob::Uniqueness.configure do |config|
  config.on_conflict = :log
  config.redlock_servers = [
    Redis.new(url: ENV.fetch('REDIS_URL', 'redis://localhost:6379/0'), ssl_params: { verify_mode: OpenSSL::SSL::VERIFY_NONE })
  ]
end

And I change Redis to RedisClient

[jeffbax]

Hopefully not repeating myself from some other issues, but attempting to do this upgrade for activejob-uniqueness https://github.com/veeqo/activejob-uniqueness

After resolving the following errors:

1. Needing to change to RedisClient instances (EVAL error https://github.com/leandromoreira/redlock-rb/issues/124)
2. Needing to specify url as it didn't pick up REDIS_URL env var like I assume redis-rb did (seen in Heroku preview apps)
3. Needing to specify reconnect_attempts: 1 param (Broken Pipe https://github.com/veeqo/activejob-uniqueness/issues/69#issuecomment-1877140691)

Then noticed a large connection spike on our Redis instance once it went to prod:

With these errors generally starting to pop up.

Redlock::LockAcquisitionError: failed to acquire lock on 'Too many Redis errors prevented lock acquisition: RedisClient::ConnectionError: stream closed in another thread'

Ultimately rolling back this gem and Redlock back to 1.3.2 for the moment as I don't have a ton of time to debug at the moment, but figured it was worth sharing (given the OP here mentioned connection pooling concerns)

[leandromoreira]

https://github.com/leandromoreira/redlock-rb/issues/148#issuecomment-1864310235

[jeffbax]

#148 (comment)

Yeah, I did come across this but was hoping to avoid adding a monkey patch. But ty for sharing it.

[jeffbax]

#148 (comment)

FWIW, I have resolved this the right way by upgrading RedisClient and overhauling the app pooling and things seem to be going without a hitch now, although I am using reconnect_attempts: 1 in the pool I build for Redlock.

Thanks for your time, appreciate the work on the gem!

[bellef]

@jeffbax we're running into the exact same problem as you for activejob-uniqueness config, could you please tell us more about overhauling the app pooling?

• redlock (2.0.6)
• config:

  ActiveJob::Uniqueness.configure do |config|
  config.redlock_servers = [
  RedisClient.new(
    url:                ENV.fetch('REDIS_URL', 'redis://localhost:6379/0'),
    ssl_params:         { verify_mode: OpenSSL::SSL::VERIFY_NONE },
    reconnect_attempts: 1
  )
  ]
  end

[sebgrebe]

@bellef, I am facing the same problem. Did you figure this out?

[sebgrebe]

@bellef, I am facing the same problem. Did you figure this out?

Nevermind, figured it out. This comment above fixed it for me

[bellef]

@sebgrebe I increased reconnect_attempts to 3

[sebgrebe]

Thanks, @bellef. I ended doing the same (but to 1)