search

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

Make limit check and additions an atomic operation #29

Closed bbonamin closed 1 year ago

bbonamin commented 6 years ago

Hi! We were having problems with a very limited API that only allows us to do a couple of requests per second and tried to use this gem, but we've been seeing issues with a highly loaded queue.

We found that due to the fact that checking whether the limit is exceeded and adding a new item to the list are different operations, if two sidekiq jobs are performed at the same time and check for the limit at the same time, both can be returned true, and proceed with executing the work.

The addition of a semaphore for the check & add to limit operations ensures that it's only done by one job at the time, and fixes the race condition.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.5%) to 98.507% when pulling 66a3a156906c3753b19cf284e414a9e18d4069d9 on bbonamin:atomic-limit-check-and-add into c163145c160f4a56d6b03bf12994b3eedb62101c on enova:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-9.0%) to 91.045% when pulling ca8f05eac873a14172a611875f5892744c771fa1 on bbonamin:atomic-limit-check-and-add into c163145c160f4a56d6b03bf12994b3eedb62101c on enova:master.

krthi commented 5 years ago

Isn't this issue fixed?

bf4 commented 4 years ago

@packrat386 PR is stale. Close or review and rebase?