Sidekiq 7 limit not enforced

[JoshuaMart]

Hi, Thanks for your work, the problem of #142 still seems to be present, here is my configuration.

Gemfile :

gem 'sidekiq'
gem 'sidekiq-limit_fetch'

Gemfile.lock :

sidekiq (7.0.2)
      concurrent-ruby (< 2)
      connection_pool (>= 2.3.0)
      rack (>= 2.2.4)
      redis-client (>= 0.11.0)
    sidekiq-limit_fetch (4.4.0)
      sidekiq (>= 6)

Sidekiq.yml :

:concurrency: 10
:max_retries: 0

:queues:
  - default
  - scan

:limits:
  default: 5
  scan: 2

The limit is well at 2 :

irb(main):001:0> Sidekiq::Queue['scan'].limit
2023-01-06T11:07:51.322Z pid=26 tid=13e6 INFO: Sidekiq 7.0.2 connecting to Redis with options {:url=>"redis://redis:6379/1", :size=>5, :pool_name=>"internal"}
=> 2

But if I launch 10 jobs, 10 workers are launched. It works as expected with Sidekiq 6 (6.5.8)

Let me know if you need more informations or if I can make more tests. Regards

[phlegx]

Maybe because of this: https://github.com/sidekiq/sidekiq/blob/main/docs/7.0-Upgrade.md#capsules

[deanpcmad]

Ah, interesting. So it looks like Sidekiq basically has limiting built in, which is great to see.

[kreintjes]

Will capsules also work when having multiple Heroku workers each running a separate Sidekiq instance with concurrency 1? Since limiting through capsules seems to be focussed on threads within one Sidekiq instance, while with sidekiq-limit_fetch you can set a maximum amount of workers per queue.

[kreintjes]

Will capsules also work when having multiple Heroku workers each running a separate Sidekiq instance with concurrency 1? Since limiting through capsules seems to be focussed on threads within one Sidekiq instance, while with sidekiq-limit_fetch you can set a maximum amount of workers per queue.

@deanpcmad can you say anything about this? We are currently wondering whether to update sidekiq-limit_fetch or switch to Sidekiq Capsules.

[deanpcmad]

Personally, I've moved to Sidekiq Capsules in one of my apps as it does what I need, which is limiting the number of jobs that execute in a queue

[paul-at]

Would've been nice to change >= 6 sidekiq dependency to ~> 6 if v7 support is broken and the gem is no longer maintained.

Ran into a nasty race condition in production after upgrading to latest sidekiq and sidekiq-limit_fetch as the limit silently stopped working.

I checked the commit history before updating, which made an impression that v7 is supported but didn't think to look into the Issues.

[kreintjes]

Personally, I've moved to Sidekiq Capsules in one of my apps as it does what I need, which is limiting the number of jobs that execute in a queue

Thanks for the reply. I think your needs are different then than ours. Am I correct in assuming you only have a single worker/server with a single Sidekiq process (but multiple threads) then? And you use capsules to limit the amount of threads certain queues have (while other queues have more threads available)?

In our situation we run multiple Heroku workers (amount is autoscaled) with a (single) Sidekiq process with the same configuration (concurrency 1 so a single thread per Heroku worker). We removed the limit_fetch gem and tried Sidekiq 7's capsules to see if we could limit the amount of concurrent jobs for some specific queues, but I did not work. As I feared, capsules only work to limit the concurrency within a single Sidekiq process, but not over multiple Sidekiq processes (on different servers). When I used capsules to say for example the heavy queue has concurrency 1, each of the Heroku workers had single thread to pick up jobs from the heavy queue next to a number of threads (default 5) to pick up jobs from the other queues. So the concurrency for the heavy queue would equal the amount of Heroku workers present at that moment, meaning we could still have multiple heavy jobs running at the same time (on different workers). So I'm afraid Capsules isn't a feasible solution for us, at least not without rethinking our complete Heroku sidekiq configuration and autoscaling set-up and maybe introducing extra (read) databases to limit/spread the load on our database.

Long story short: we still need something like sidekiq-limit_fetch to limit the concurrency of certain queues over all processes/servers. I tried updating both sidekiq (7.2.0) and sidekiq-limit_fetch (4.4.1) to the latest versions and it seems to still work for us. It makes sure only one Heroku worker picks up jobs from the super_heavy queue, while we have some available for the heavy queue and more for the other queues.

@JoshuaMart @paul-at Not sure why it doesn't work in your set-up. Maybe it only works for us because we have concurrency of 1 in our sidekiq.yml for our global configuration (and use multiple workers to scale horizontally instead of threads).

@deanpcmad could you say anything about the continuity of the sidekiq-limit_fetch gem? Are you the only maintainer currently? And are you stopping maintenance now that you no longer need it?

Ps. I also found the sidekiq-throttled gem which also seems to solve this problem. Unfortunately however, it has no ActiveJob integration at all it seems. We use ActiveJob so it is not really an option for us, but maybe it is for others who are (or want) to use Sidekiq::Job classes.

[deanpcmad]

Thanks for the reply. I think your needs are different then than ours. Am I correct in assuming you only have a single worker/server with a single Sidekiq process (but multiple threads) then? And you use capsules to limit the amount of threads certain queues have (while other queues have more threads available)?

Yep, I just use a single server. And with SolidQueue being released, I may end up moving to that instead.

I'm still happy to maintain this gem though. I just am unable to reproduce the issue. If anyone can reproduce it and create a PR, I'd be happy to merge and create a release.

[paul-at]

@kreintjes I think your hypothesis is correct. My use case consists of a single Sidekiq process with multiple threads, processing several queues and limit_fetch was used to process one of the queues strictly sequentially (by limiting concurrency to 1 with limit_fetch) without having to run multiple Sidekiq processes.

This worked with Sidekiq 6 and broke in Sidekiq 7, so ended moving to the capsules.

Thank you for the tips and your work on this project guys!