Why `Thread.exclusive` ?

[DanielVartanov]

Hi,

I looked through the code and I wonder why do you use Thread.exclusive when dealing with the counter?

My first concern is that Redis itself has awesome atomic operations like INCR, SETNX and others. Calls to Redis does not require any critical section.

My second concern is that if Thread.exclusive was added for the sake of memory storage, then how do you share limit counter between processes?

Thanks, Daniel.

[gevans]

Hey Daniel,

Thanks for opening this issue. This is left over from the original implementation. Originally, memory storage was the only available backend. There's no shared counter between processes when using memory storage.

@lsimoneau, even though that was authored a little over a year ago, thoughts/recommendation on the Redis backend's thread-safety without the use of Thread.exclusive?

[lsimoneau]

Hey, so it's a little vague since it's been so long, but looking back over my commits, I didn't pay any attention to the Thread.exclusive, I just abstracted out the business of the in-memory storage and then wrote the Redis one to conform to the same interface.

Looking at the Redis implementation, it appears I've taken some fairly careful precautions to avoid any potential concurrency issues (https://github.com/gevans/sidekiq-throttler/blob/master/lib/sidekiq/throttler/storage/redis.rb#L35-L53), so I doubt that the lock is required for the Redis backend to behave correctly in multithreaded environments.