Design considerations in dynamic keys

[fhwang]

Hello, I'm considering moving a project from sidekiq-throttler to sidekiq-throttled. One thing that is holding me back is that sidekiq-throttled currently does not support dynamic keys, for example:

sidekiq_options throttle: { threshold: 20, period: 1.day, key: ->(user_id){ user_id } }

I'm happy to fork and submit a pull request, but before I do that I wanted to check in with you about how you were thinking about the design.

The Threshold and Concurrency strategy classes both support count and reset! methods that don't apply very well to a dynamic keys use-case. (I'm not sure if they can be implemented in a way that doesn't involve an in-memory cache.) Looks like reset! is called in Strategy, where count doesn't seem to referenced anywhere else in this repo.

How do you think I should proceed? Would it be acceptable to simply say reset! and count are not supported if you're using dynamic keys?

[ixti]

Yeah. I'm absolutely OK to add dynamic keys support as I said it somewhere earlier.
I think reset! and count should be able to receive params passed to key:

def reset!(*args)
  key = base_key
  key = "#{key}:" << @dynamic_key.call(*args) if @dynamic_key
  # ...
end

That's just an idea. I'm totally open to simply say it's not supported for dynamic keys ;))

[fhwang]

Having reset! and count receive params makes sense to me. I'll play around with this and have a PR in a bit.