viafintech / gcra-ruby

Generic cell rate algorithm (leaky bucket) implementation for rate limiting
MIT License
51 stars 10 forks source link

Problem with quantity #11

Closed morgoth closed 2 years ago

morgoth commented 2 years ago

Here is my simple throttling code:

limiter = GCRA::RateLimiter.new(
      GCRA::RedisStore.new($redis, "test"),
      1 / 10.0,
      0
    )
limiter.limit('all', 1)
=> [false, #<struct GCRA::RateLimitInfo limit=1, remaining=0, reset_after=0.1, retry_after=nil>]
limiter.limit('all', 2)
=> [true, #<struct GCRA::RateLimitInfo limit=1, remaining=1, reset_after=0.0, retry_after=nil>]

It all works fine with quantity 1, but when I set different value, it returns inconsistent result. The "true" is returned as exceeded, but the retry_after is nil and also "remaining" is a positive value.

tobischo commented 2 years ago

Hi @morgoth, that is because you did not allow for any burst.

The max burst basically defines the size of the bucket. The rate period defines how fast the bucket fills up again.

You defined the max burst as 0. That means you can basically just take 1 at a time, however since the reset time is so quick, it also does not bother calculating retry_after: https://github.com/viafintech/gcra-ruby/blob/122a2b18a75ba20bd645bf06130c329f8f9f1e72/lib/gcra/rate_limiter.rb#L63-L67

If you try it with a higher value for burst, you'll see something different.

The case, while seeming inconsistent, was defined intentional

morgoth commented 2 years ago

@tobischo Thank you for the quick reply.

I think I don't understand how does it work exactly. Let's say i have rate period 0.1 (ten requests per second). Is this working in a way that I can:

Why the max_burst has to be set to make it work? In my case the quantity is dynamic and I don't know the actual limit.

I would expect the limiter.limit('all', 20) to throttle it to 2 seconds (2x10).

Is there any difference in behaviour when I set max_burst or not and use only 1-quantitty?

tobischo commented 2 years ago

If you would want to throttle something for 2 seconds, with a single request you would probably rather go with defining a rate_period of 2, which would mean 1 per 2 seconds.

Then you could limit with limiter.limit('all', 20) and the next one is available 2s later.

As for imagining it: the rate period basically defines the rate at which new capacity becomes available again. So image a bucket of coins. With a rate period of 1/10 that would mean that every 0.1s a new coin is thrown in. With a rate period of 2 that would mean that every 2s a new coin is thrown in.

The max_burst defines the size of the bucket and how many coins can be in there before it overflows. So if you have a bucket size of 0, there is really nothing in there. If you have a bucket size of 10, you could collect 10 coins before new ones are lost.

The limiting is taking coins from the bucket. If you have no burst, you can only take 1 at a time to make it work. (That's why burst 0 causes issues when limiting more than 1) If you have burst you can also take more than 1 at a time and then wait until enough coins have been added to the bucket again. However if you have a burst defined, meaning a bigger bucket, and if you always take just 1 coin, you can temporarily do that at a higher rate than you might be able to handle. That is, until the bucket is empty. After that you can only take coins at the rate at which it has been refilled.

If you could better describe why the quantity is dynamic, I might be able to suggest a solution that should work.

We have a similar usecase though, for a 24h limit. That means we defined it 86400.0 / limit with limit being the number within 24h, e.g. for 1000 per day. That would then be 86400.0 / 1000, which would result in 1 per 86.4s, additionally we defined a burst of equal size, so that you could use up 1000 and then you'd have to wait for 86.4s before the next 1, or if you needed to limit for 2, 3, or whatever it would be N * 86.4s. With that setup the number of limitations can vary.

morgoth commented 2 years ago

Thank you for the explanation.

I'm implementing throttling for the Google DistanceMatrix API which is: "1000 elements per second (EPS)". So it's not really requests per seconds, but rather "volume" of payload in requests. So I can have 1 request with 10 elements, 1 request with 1 element, etc. This cannot be bigger than 1000 elements per second.

I think I could set max_burst to 1000 (I won't have such big requests as there are other factors limiting it) and then it should work for me.

Just for the reference, here is my full code:

require "gcra/rate_limiter"
require "gcra/redis_store"

class Throttler
  COUNTER_KEY = "all"

  def initialize(name, rps:)
    @name = name
    @rps = rps
    @limiter = GCRA::RateLimiter.new(
      GCRA::RedisStore.new($redis, name),
      1 / @rps.to_f,
      @rps
    )
  end

  def perform(count: 1)
    logged = false
    while (result = @limiter.limit(COUNTER_KEY, count))[0]
      unless logged
        Rails.logger.tagged("throttler").info("Throttling #{@name} to #{@rps} RPS")
        logged = true
      end
      sleep result.last.retry_after
    end
  end
end

# testing it with the great async gem:
require "async"
start = Time.now
res = Async do |task|
  100.times do |i|
    task.async do
      throttler = Throttler.new("dm", rps: 1_000)
      throttler.perform(count: (rand * 100).round)
      sleep(1)
    end
  end
end.wait
puts Time.now - start
tobischo commented 2 years ago

That should be working, unless you would for some reason need to have the 1000 elements evenly spaced.

Theoretically you can also define the bucket size as the single largest set of elements you expect at one time. Then the rate period will refill it as it goes. That would allow for a more evenly spaced set of elements and reduces the chances for 2000 within 1 second, which the 1000 burst would allow.

tobischo commented 2 years ago

Assuming the question about the behaviour to be answered, I will close the issue