enova / sidekiq-rate-limiter

Redis-backed, per-worker rate limits for job processing.
http://rubygems.org/gems/sidekiq-rate-limiter
MIT License
193 stars 43 forks source link

Looks like this is no longer working #12

Closed ryana closed 9 years ago

ryana commented 9 years ago

Hey guys,

It looks like this is no longer working on Sidekiq 3.2.1 & Rails 4.2.3

# app/workers/test_worker.rb
class TestWorker
  include Sidekiq::Worker
  sidekiq_options queue: :test_queue, throttle: {threshold: 20, period: 1.minute}

  def perform(options)
  end
end
# config/sidekiq.yml

---
:concurrency: 10
:pidfile: tmp/pids/sidekiq.pid
development:
  :concurrency: 2
:queues:
  - test_queue

Gem versions

Spike showing way more throughput than should be there

It should only be doing 40 jobs/minute (2 workers x 20 jobs/minute) but it's just going full blast.

I haven't dug into this at all yet, but wanted to bring to your attention. I will report back if I find anything.

ryana commented 9 years ago

False alarm. Plz ignore. Will report back with what I did wrong in a little bit :)

jeljer-te-wies commented 9 years ago

Hi Ryana,

Do you remember what you did wrong. It's not working for us!

ryana commented 9 years ago

I'm pretty sure that I completely forgot to require 'sidekiq-rate-limiter/server' and set Sidekiq.options[:fetch] = Sidekiq::RateLimiter::Fetch -- embarrassing :)

jeljer-te-wies commented 9 years ago

Haha lol! I din't forget those things, but it's not working for me sadly (no errors, just not doing the rate limiting)

ryana commented 9 years ago

Can you drop your Worker file code and your sidekiq.yml in here?

jeljer-te-wies commented 9 years ago

Sure!

Here is my worker:

class ShopifyWorker
  include Sidekiq::Worker  
  sidekiq_options throttle: { threshold: 10, period: 1.hour }

  def self.perform
    sleep 1.seconds
    p 'Boooom!'
  end
end

Initializer for server:

require 'sidekiq-rate-limiter/version'
require 'sidekiq-rate-limiter/fetch'
Sidekiq.configure_client do |config|
  config.redis = { url: "redis://127.0.0.1:6379/1", namespace: "shopify" }
end
Sidekiq.configure_server do |config|
  config.redis = { url: "redis://127.0.0.1:6379/1", namespace: "shopify" }
  Sidekiq.options[:fetch] = Sidekiq::RateLimiter::Fetch
end

When i run the following example it will perform the 50 jobs at once.

50.times { ShopifyWorker.delay.perform() } 
ryana commented 9 years ago

Hmm. Well there are some differences between my setup and yours, but I'm not sure if any of them cause the problem you're seeing.

I'm on sidekiq-rate-limiter (0.1.0)

jeljer-te-wies commented 9 years ago

Awesome Ryana! You never know. I will try it out immediately!

jeljer-te-wies commented 9 years ago

That worked! awesome! thanxs Ryana!

ryana commented 9 years ago

@reloadseo do you know which line was the culprit? Be good to throw it in here for posterity.

jeljer-te-wies commented 9 years ago

You are right Ryana, So the setup we have now (and is working).

In your worker:

class MyAwesomeWorker
  include Sidekiq::Worker  
  sidekiq_options queue: 'shopify',
    rate: {
      limit: 2,
      period: 2
    }

  def perform
    p 'Hello world'
  end
end

In your code, use it with the perform_async, DON'T use:

MyAwesomeWorker.delay.my_method()

We had the problem that when you have a lot of concurrent jobs this does not work. So we decided to limit our job to 1, you can do it like this:

bundle exec sidekiq -q shopify -c 1

Only with these setting we get nice output like this:

screen shot 2015-10-30 at 16 51 21
mookie5dc commented 8 years ago

Does this gem work with concurrent threads? When I tried it, the workers exceeded the limits I specified.