ixti / sidekiq-throttled

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

Add dynamic cooldown policies #174

Open ixti opened 8 months ago

ixti commented 8 months ago

Allow Config#cooldown_period and Config#cooldown_threshold to be specified as Proc:

Sidekiq::Throttled.configure do |c|
  c.cooldown_period = lambda do |queue_name|
    queue_name == "default" ? 2.0 : 10.0
  end

  c.cooldown_threshold = lambda do |queue_name|
    queue_name == "mostly-throttled" ? 0 : 10
  end
end

This will go very well in pair with planned push-back strategies PR.

codecov-commenter commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (02704ab) 98.92% compared to head (21136ba) 98.92%. Report is 7 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #174 +/- ## ======================================= Coverage 98.92% 98.92% ======================================= Files 17 17 Lines 371 372 +1 Branches 52 52 ======================================= + Hits 367 368 +1 Misses 4 4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

freemanoid commented 8 months ago
  c.cooldown_period = lambda do |unit_of_work|

I'm not sure about the unit_of_work in the interface. Maybe a queue name would be enough? I'm just thinking about how it can be done in a bit more explicit fashion.

ixti commented 8 months ago

I'm not sure about the unit_of_work in the interface. Maybe a queue name would be enough? I'm just thinking about how it can be done in a bit more explicit fashion.

My rationale behind unit of work was that one may want to have different period/threshold based on job class and/or args. I guess the most straight-forward API in this case will be:

c.cooldown_period = lambda do |queue_name, message|
  job_record = Sidekiq::JobRecord.new(message)

  if job_record.display_class == "FooJob"
    1.0
  elsif "default" == queue_name
    2.0
  else
    3.0
  end
end

Naturally, in this case, it will also support:

c.cooldown_period = lambda do |queue_name|
  if "default" == queue_name
    2.0
  else
    3.0
  end
end
ixti commented 8 months ago

After giving it another thought, I think we can keep it simple :D just queue name.

freemanoid commented 8 months ago

After giving it another thought, I think we can keep it simple :D just queue name.

I like the Sidekiq::JobRecord idea tho. 😄

ixti commented 7 months ago

I like the Sidekiq::JobRecord idea tho. 😄

I do like it too. But it will involve object creation even when user does not need it (e.g. just caring about queue_name. Thus passing queue_name where the message arrived from and message give full flexibility without sacrificing performance.

ixti commented 7 months ago

I'm inclined towards do |queue, message| approach.

ixti commented 7 months ago

Will wait with final decision until real case scenario appears.