Configured clustered rated limiters

[steveh]

In Sidekiq Ent 7, you can configure a rate limiter, which is described as cluster-safe. It accepts either a ConnectionPool or a Hash of config.

If you pass a Hash, it's not cluster-aware, as it uses redis-client, and the cluster functionality has been moved to redis-cluster-client, i.e. this doesn't work:

Sidekiq::Limiter.redis = { nodes: ["redis://localhost:6379"] }
> ArgumentError: unknown keyword: :nodes

Which leads me to think I need to pass my own ConnectionPool, e.g.

Sidekiq::Limiter.redis = ConnectionPool.new(size: 5) do
  RedisClient.cluster(nodes: ["redis://localhost:6379"], size: 5).new_client
end

But I feel that means doubling up on pools - both ConnectionPool and RedisClient::Cluster's have pools.

What's the best way to do this?

[mperham]

Right, the redis config assumes you want a pool of basic redis-client instances. Sidekiq isn't cluster-compatible so Sidekiq::Config::redis= doesn't allow :nodes but Sidekiq::Limiter needs a spot of logic to allow cluster-compatibility.

Put this monkeypatch at the top of your app's initializer with the simple Hash config below it and let me know if it solves your problem.

module Sidekiq
  class RedisClientAdapter
    def initialize(options)
      opts = client_opts(options)
      @config = if opts.key?(:sentinels)
        RedisClient.sentinel(**opts)
      elsif opts.key?(:nodes)
        # Sidekiq does not support Redis clustering but Sidekiq Enterprise's
        # rate limiters are cluster-safe so we can scale to millions
        # of rate limiters using a Redis cluster.
        # Sidekiq::Limiter.redis = { nodes: [...] }
        RedisClient.cluster(**opts)
      else
        RedisClient.config(**opts)
      end
    end
  end
end

Sidekiq::Limiter.redis = { nodes: ["redis://localhost:6379"] }

[mperham]

Sidekiq needs a standard Redis pool. You'll have a separate Pool for the clustered instances used by rate limiting. There's nothing you can do to prevent this.

I don't think I understand your questions.

[steveh]

I mean when Limiter goes to use Sidekiq, it will check out a RedisClient::Cluster from the ConnectionPool.
But each of those RedisClient::Cluster's maintains its own connection pool, via the concurrency parameter.
So with the defaults, we'll have a ConnectionPool of size 5, and a RedisClient::Cluster of concurrency 5 = 25 connections.

I'm wondering if you think we should either:

1. set RedisClient::Cluster concurrency=1 (or disable it) delegating pool management to ConnectionPool
2. set ConnectionPool size=1, delegating pool management to RedisClient::Cluster
3. split the desired total concurrency between them. say if we wanted 50 total, give size=10 and concurrency=5
4. do nothing as they serve different purposes

[mperham]

I think you're misunderstanding me. You don't need to provide a ConnectionPool if you have that monkeypatch in place. You just put this in your initializer:

Sidekiq::Limiter.redis = { nodes: ["redis://localhost:6379", ...] }

[steveh]

I understand not needing to provide a pool, but Sidekiq::Limiter.redis= passes that Hash to Sidekiq::RedisConnection.create, which creates a ConnectionPool around the result of Sidekiq::RedisClientAdapter.

Am I just overthinking this? Does it not matter that a ConnectionPool has many cluster connections, each with its own concurrency pool?

[mperham]

Yeah, I think you are overthinking it. Sidekiq::Limiter's ConnectionPool will have a set of RedisClient objects, each of which might hold a single connection to each cluster member. Pools should be lazy so if the Pool only needs one connection, it will only create one connection.