ixti / sidekiq-throttled

Concurrency and rate-limit throttling for Sidekiq
MIT License
710 stars 76 forks source link

Feature request: allow throttled workers to have different cost #45

Open RKushnir opened 6 years ago

RKushnir commented 6 years ago

Let's say an API has a limit of 100 calls per minute. And there's 2 kinds of jobs — one makes 2 calls, the other makes 3.

To calculate the guaranteed limit I need to use the higher value, so the threshold will be 33 jobs/min. But this is obviously inefficient when most of the jobs are of the first kind(2 calls), in the worst case reducing the effective limit to 66 calls/min instead of 100.

So, it would be nice to be able to specify how much each job costs. Ideally, with an option to dynamically calculate the value.

ixti commented 6 years ago

Oh. That sounds pretty interesting! I'll need to think about it. In fact I'm already slowly working on changing the throttling code (making it separated of sidekiq sot that sidekiq-throttled will use it eventually) - but that's a slowly going process as here in SensorTower we don't need this ATM.

But the idea you described seems pretty interesting to me. At least at this very moment. :D

ixti commented 6 years ago

Can you describe in pseudo code how do you see it?

RKushnir commented 6 years ago

I haven't ever written a sidekiq middleware, so I have no idea what I'm talking about :)

Anyway, as I understand the current implementation, it keeps a list of the timestamps when previous jobs where started. And when there's a new job, it counts the timestamps to check if the limit has not been exceeded, in pseudo-SQL:

if (select count(*) from jobs where jobs.timestamp > 1.minute.ago) < 100 then execute

With my suggestion, it would store a pair (timestamp, job cost), and the calculation would be:

if (select sum(jobs.cost) from jobs where jobs.timestamp > 1.minute.ago) + new_job.cost <= 100 then execute