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

Errors when using with redis-rb > 4.1.0 #30

Closed lavaturtle closed 5 years ago

lavaturtle commented 5 years ago

When I install the latest versions of everything with bundle install, and then run the specs, I see these errors:

..F.F..

Failures:

  1) Sidekiq::RateLimiter::Fetch should retrieve work
     Failure/Error: lim.add(klass)

     Redis::CommandError:
       ERR value is not an integer or out of range
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:156:in `value'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:148:in `_set'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:75:in `block in finish'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:74:in `each'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:74:in `each_with_index'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:74:in `each'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:74:in `map'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:74:in `finish'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:98:in `finish'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/client.rb:164:in `block in call_pipeline'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/client.rb:306:in `with_reconnect'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/client.rb:162:in `call_pipeline'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis.rb:2462:in `block in multi'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis.rb:52:in `block in synchronize'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis.rb:52:in `synchronize'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis.rb:2454:in `multi'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis_rate_limiter-0.1.0/lib/redis_rate_limiter.rb:35:in `add'
     # ./lib/sidekiq-rate-limiter/fetch.rb:39:in `block in limit'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/sidekiq-5.2.7/lib/sidekiq.rb:97:in `block in redis'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/connection_pool-2.2.2/lib/connection_pool.rb:65:in `block (2 levels) in with'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/connection_pool-2.2.2/lib/connection_pool.rb:64:in `handle_interrupt'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/connection_pool-2.2.2/lib/connection_pool.rb:64:in `block in with'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/connection_pool-2.2.2/lib/connection_pool.rb:61:in `handle_interrupt'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/connection_pool-2.2.2/lib/connection_pool.rb:61:in `with'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/sidekiq-5.2.7/lib/sidekiq.rb:94:in `redis'
     # ./lib/sidekiq-rate-limiter/fetch.rb:33:in `limit'
     # ./lib/sidekiq-rate-limiter/fetch.rb:11:in `retrieve_work'
     # ./spec/sidekiq-rate-limiter/fetch_spec.rb:57:in `block (2 levels) in <top (required)>'

  2) Sidekiq::RateLimiter::Fetch should accept procs for limit, name, and period config keys
     Failure/Error: lim.add(klass)

     Redis::CommandError:
       ERR value is not an integer or out of range
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:156:in `value'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:148:in `_set'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:75:in `block in finish'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:74:in `each'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:74:in `each_with_index'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:74:in `each'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:74:in `map'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:74:in `finish'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/pipeline.rb:98:in `finish'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/client.rb:164:in `block in call_pipeline'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/client.rb:306:in `with_reconnect'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis/client.rb:162:in `call_pipeline'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis.rb:2462:in `block in multi'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis.rb:52:in `block in synchronize'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis.rb:52:in `synchronize'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis-4.1.1/lib/redis.rb:2454:in `multi'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/redis_rate_limiter-0.1.0/lib/redis_rate_limiter.rb:35:in `add'
     # ./lib/sidekiq-rate-limiter/fetch.rb:39:in `block in limit'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/sidekiq-5.2.7/lib/sidekiq.rb:97:in `block in redis'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/connection_pool-2.2.2/lib/connection_pool.rb:65:in `block (2 levels) in with'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/connection_pool-2.2.2/lib/connection_pool.rb:64:in `handle_interrupt'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/connection_pool-2.2.2/lib/connection_pool.rb:64:in `block in with'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/connection_pool-2.2.2/lib/connection_pool.rb:61:in `handle_interrupt'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/connection_pool-2.2.2/lib/connection_pool.rb:61:in `with'
     # /home/jacinda/.rvm/gems/ruby-2.5.3@sidekiq-rate-limiter/gems/sidekiq-5.2.7/lib/sidekiq.rb:94:in `redis'
     # ./lib/sidekiq-rate-limiter/fetch.rb:33:in `limit'
     # ./lib/sidekiq-rate-limiter/fetch.rb:11:in `retrieve_work'
     # ./spec/sidekiq-rate-limiter/fetch_spec.rb:93:in `block (2 levels) in <top (required)>'

Finished in 0.27527 seconds (files took 0.16476 seconds to load)
7 examples, 2 failures

Failed examples:

rspec ./spec/sidekiq-rate-limiter/fetch_spec.rb:54 # Sidekiq::RateLimiter::Fetch should retrieve work
rspec ./spec/sidekiq-rate-limiter/fetch_spec.rb:84 # Sidekiq::RateLimiter::Fetch should accept procs for limit, name, and period config keys

If I update the gemspec to pin redis to 4.1.0, the specs pass. But they fail with 4.1.1 or 4.1.2.

This matches some errors I've seen in production, so it's not just a spec issue.

jpaas commented 5 years ago

Is there any chance of merging @lavaturtle 's PR? We've been running it in production for 4 months now.

jpaas commented 5 years ago

Thank-you @packrat386 !