ixti / sidekiq-throttled

Concurrency and rate-limit throttling for Sidekiq
MIT License
698 stars 75 forks source link

Observer is called continuously #157

Closed stefanoc closed 9 months ago

stefanoc commented 11 months ago

Since I need to upgrade Sidekiq to version 7, I'm testing the 1.x branch (I've been using version 0.18 so far). Everything looks fine except for one thing: the observer callback is invoked much more frequently than before. I'm talking about hundreds of calls per second. This is my test job:

class TestJob < ApplicationJob
  include Sidekiq::Throttled::Job

  OBSERVER = lambda do |strategy, *args|
    puts "THROTTLING: #{strategy} #{args.inspect}"
  end

  sidekiq_throttle(
    concurrency: { limit: 10 },
    threshold: { limit: 10, period: 1.hour },
    observer: OBSERVER
  )

  def perform
    sleep 1
  end
end

Queueing 20 jobs:

 20.times { |n| TestJob.perform_async }

and letting Sidekiq run for 2-3 seconds results in >2000 log lines printed in the terminal. Is this expected behavior?

ixti commented 10 months ago

I've removed expireable list that was casuing issues for one group of users. But removing it - causes now issues for the other group. So, I'm planning on brining it back with more obvious ways to configure the behaviour.

akarimcheese commented 10 months ago

I've removed expireable list that was casuing issues for one group of users. But removing it - causes now issues for the other group. So, I'm planning on brining it back with more obvious ways to configure the behaviour.

In bringing this back, would this include the behavior of fetching from lower queues while a higher one is in the expirable list?

ixti commented 10 months ago

Yeah, expirable set will be responsible to exclude queues that got too many throttled jobs in a row for a fetcher for the duration of configurable time.

PS: The work has began, thanks @Connorhd: https://github.com/ixti/sidekiq-throttled/pull/160

ixti commented 9 months ago

Will merge the fix today!