Further understanding of Sidekiq.redis and Sidekiq.redis_pool

[jeffbax]

Apologies if this is a dumb question, I have been cycling around the issues, discussions, and docs for a few gems trying to navigate some redis -> redis-client upgrades and general connection pooling issues in our app... without a clear answer and a lot of trial and error :)

Some of the gems in question:

• ActiveJob Uniqueness (Redlock)
• Flipper
• Rack-Attack
• Searchkick
• Sidekiq

In Sidekiq 6 one was able to provide a pool, and now Sidekiq builds one internally. The upgrade guide mentions:

If you use Sidekiq.redis to access Redis connections, that API is now exposing a connection based on redis-client

Does this mean that we should be able to piggyback off of the pool that Sidekiq initializes and provide it to other gems that are also using RedisClient, or are those helpers primarily for internal Sidekiq extensions and components only?

ie. init Searchkick.redis = Sidekiq.redis

I don't mean this as a Searchkick Q&A, so much as understand more about why Sidekiq chose to make their own pool and strategies for avoiding having to initialize so many pools across the application as I currently am having to (due to some gems needing redis-client, others still redis-rb, and yet others taking only URLs as they manage things internally such as action cable)

I know there is a question of "why are you using one Redis instance for so much" but we are generally not taxing things all that much yet outside of connections as servers scale up and down (and implementing some other layers to limit things like Flipper using it at all) but figured the question was worth asking if not for others that may have similar questions.

[mperham]

Does this mean that we should be able to piggyback off of the pool that Sidekiq initializes and provide it to other gems that are also using RedisClient, or are those helpers primarily for internal Sidekiq extensions and components only?

The pool has never been something Sidekiq has advertised as available for use. We locked it down because people were initializing their own connection pool way too small and causing severe performance issues with Sidekiq. You are welcome to use that pool in your own code if you understand the ramifications but this has always been a matter of "If you know what you are doing, I'm not going to stop you" but I reserve the right to make breaking changes, like the pool lockdown, if the API is causing support issues.

Searchkick.redis = Sidekiq.redis

Don't do this. Sidekiq's connections should remain under Sidekiq's control, you cannot steal a connection and hand it elsewhere safely. Create your own pools and connections for other subsystems, this helps keep them logically separate too.

[jeffbax]

Hey Mike, thanks for the quick response! (and sorry for my delay)

Yeah I wasn't quite sure given in the past it seemed like one was able to provide one, but it makes sense to move it internally if it was causing persistent issues for people. I imagine performance demands are a lot higher for Sidekiq than most frameworks which may be a little safer to initialize a shared pool for (such as for us, Flipper, which Redis is only a backup for memcached)

That said, is it safe to tap Sidekiq.redis within a Sidekiq::Job instance procedure? I noticed some code of ours currently doing that while managing batch key handling (Sidekiq jobs, not ActiveRecord jobs) and want to confirm whether that code is doing something it shouldn't or not.

We haven't noticed issues yet, but that is not to say it isn't on the wrong path.

Thanks again, appreciate all the work and support.

[mperham]

Yes, it's safe to use Sidekiq.redis in your own jobs, just try to minimize the amount of time you hold the connection.

Bad

def perform
  Sidekiq.redis do |conn|
    # connection is held during this entire loop
    User.find_each do |u|
      conn.set("uid:#{u.id}", u.email)
    end
  end
end

Good

def perform
  User.find_each do |u|
    # connection only held to perform the actual network operation
    Sidekiq.redis do |conn|
      conn.set("uid:#{u.id}", u.email)
    end
  end
end

[jeffbax]

got it, yup we aren't doing anything like that anywhere but will keep it in mind in the rare cases I see that getting accessed

Thanks!