convertkit / excess_flow Goto Github PK

If this exists already perhaps you can help me identify how to do it. I'm looking for a way to detect whether a particular key will overflow in the next call. Digging into the source it looks like it would be using this method
within_rate_limit? but would need to not increment the counter

def next_within_rate_limit?
      ExcessFlow::GlobalMutex.locked(lock_key: configuration.lock_key) do
        cleanup_stale_counters

        if current_requests < configuration.limit
          true
        else
          false
        end
      end
    end
end

The big difference is it wouldn't call the "bump_counter" method.

So I'm not sure if this belongs in this gem but I feel like it's a fairly common use case. If it doesn't belong directly in the gem perhaps an example in the readme could help someone?

So I needed to rate limit a particular method but rather than just skipping the execution if the limit was hit, I needed to always guarantee the method was eventually executed. So I combine ExcessFlow with a simple while (true) and a sleep such that eventually the method completes once it can without violating the limit.

Here is my code. Also if I missed something already built into the gem that does this easier please let me know!

module RateLimit
  module_function

  def limit(unique_throttle_name, rate, interval, &block)
    while true
      execution = ExcessFlow.throttle(options(unique_throttle_name, rate, interval)) do
        block.call
      end

      if execution.success?
        return execution.result
      else
        time_to_wait = interval + 1
        time_to_wait = rand(0.0..time_to_wait.to_f)
        puts "Rate limited - waiting a random amount of time #{time_to_wait}"
        sleep time_to_wait
      end
    end
  end

  def options(name, rate, interval)
    { key: name.to_s, limit: rate, ttl: interval }
  end
end

Usage:

RateLimit.limit("rate-limit-some-unique-name', 1, 10) do
   puts "Performing this block once, no more than every 10 seconds"
end

I run this code inside some sidekiq workers that run asynchronously so I can safely guarantee across processes that the block doesn't execute more than it should, but it still eventually executes.

ExcessFlow is a high precision Redis based rate limiter; it means that even with hundreds and even thousands requests coming in all at once it will not allow an occasional request slip over limit (causing potential race conditions or unwanted extra invocations of your code).

I just implemented a sliding window rate limiter myself, with redis in ruby so I just wanted to give some thoughts and ideas. Overall i'd say I really like this gem and the layout of the code is well done. My code is not publicly available for reasons I won't get into.

I'm also being pretty nitpicky, so sorry for that.

I think the statement "it means that even with hundreds and even thousands requests coming in all at once it will not allow an occasional request slip over limit" is false, because the Timestamps in a given ordered set are determined by the client, and not guaranteed to be unique.

You do

r.zadd(
          configuration.counter_key,
          current_timestamp,
          current_timestamp
        )

From the redis docs;
If a specified member is already a member of the sorted set, the score is updated and the element reinserted at the right position to ensure the correct ordering.

So if 2 clients have some clock drift (most clocks are slightly out), and when they each acquire the global mutex they both set the same value for current_timestamp, then the nature of the set is that it will update the same element to the same score, in effect not adding the current request to your set.

The fix I used in my code was setting the set value to a guid, similar to;

r.zadd(
          configuration.counter_key,
          current_timestamp,
          SecureRandom.uuid
        )

so it's always going to be unique and the rest of your operations work. timestamps set in the future (relative to your first instance) will still be counted by your zcount, so there is a slight chance you'll block an extra request occasionally, but the chance is small.

The second thing i just wanted to comment on (sorry for the nitpick), is the mutex is unnecessary. Instead you could use a redis transaction. Here is what I'm doing;

    ret = redis.pipelined do
      redis.zremrangebyscore full_key, 0, clear_before_timestamp # remove requests past period
      redis.zcard full_key # count of how many requests we've received
      redis.zadd full_key, current_timestamp, SecureRandom.uuid
      redis.expire full_key, period
    end

The redis docs say;
All the commands in a transaction are serialized and executed sequentially. It can never happen that a request issued by another client is served in the middle of the execution of a Redis transaction. This guarantees that the commands are executed as a single isolated operation.

https://redis.io/topics/transactions

Just wanted to point out another way to do it :)

Thank you for your library and your time. I might have got something wrong so please correct me if i have.

Per @ankopainting suggestions in #1:

The second thing i just wanted to comment on (sorry for the nitpick), is the mutex is unnecessary. Instead you could use a redis transaction. Here is what I'm doing;

    ret = redis.pipelined do
      redis.zremrangebyscore full_key, 0, clear_before_timestamp # remove requests past period
      redis.zcard full_key # count of how many requests we've received
      redis.zadd full_key, current_timestamp, SecureRandom.uuid
      redis.expire full_key, period
    end

The redis docs say;
All the commands in a transaction are serialized and executed sequentially. It can never happen that a request issued by another client is served in the middle of the execution of a Redis transaction. This guarantees that the commands are executed as a single isolated operation.

https://redis.io/topics/transactions

Need to investigate how viable this option is and if mutex can be replaced with a transaction it could clean up code a bit.