I'm investigating a sidekiq deadlock and it appears that several threads are blocked on the Redis gem's internal Monitor, with stack traces ending as this one does: https://gist.github.com/3152780 If I understand things correctly, since sidekiq uses a connection pool, a Redis instance should never be shared between concurrent threads, and thus should never block on this monitor. Can anyone explain why this would be happening?
Your understanding is correct. No one's ever reported such a deadlock and I have no idea what might cause it. On Fri, Jul 20, 2012 at 12:43 PM, John Firebaugh <john.firebaugh@gmail.com> wrote: > I'm investigating a sidekiq deadlock and it appears that several > threads are blocked on the Redis gem's internal Monitor, with stack > traces ending as this one does: > > https://gist.github.com/3152780 > > If I understand things correctly, since sidekiq uses a connection > pool, a Redis instance should never be shared between concurrent > threads, and thus should never block on this monitor. > > Can anyone explain why this would be happening?
On Fri, Jul 20, 2012 at 1:29 PM, Mike Perham <mperham@gmail.com> wrote: > Your understanding is correct. No one's ever reported such a deadlock > and I have no idea what might cause it. It turned out to be a misuse of Redis::Semaphore on my part: semaphore = Sidekiq.redis {|redis| Redis::Semaphore.new(:my_semaphore, redis) } semaphore.lock do # ... end Since Redis::Semaphore stores the passed in connection, this isn't safe. It needs to be written: Sidekiq.redis do |redis| semaphore = Redis::Semaphore.new(:my_semaphore, redis) semaphore.lock do # ... end end But note that this consumes a connection for the duration of the lock; you could easily drain the connection pool this way. And in either case, if the sidekiq process crashes or gets kill -9'd, you risk stranding the lock. In short, I'd not recommend trying to combine Redis::Semaphore and Sidekiq.
Distributed locking in general is a terrible idea. It's a constant source of pain in my experience. On Fri, Jul 20, 2012 at 3:28 PM, John Firebaugh <john.firebaugh@gmail.com> wrote: > On Fri, Jul 20, 2012 at 1:29 PM, Mike Perham <mperham@gmail.com> wrote: >> Your understanding is correct. No one's ever reported such a deadlock >> and I have no idea what might cause it. > > It turned out to be a misuse of Redis::Semaphore on my part: > > semaphore = Sidekiq.redis {|redis| > Redis::Semaphore.new(:my_semaphore, redis) } > semaphore.lock do > # ... > end > > Since Redis::Semaphore stores the passed in connection, this isn't > safe. It needs to be written: > > Sidekiq.redis do |redis| > semaphore = Redis::Semaphore.new(:my_semaphore, redis) > semaphore.lock do > # ... > end > end > > But note that this consumes a connection for the duration of the lock; > you could easily drain the connection pool this way. And in either > case, if the sidekiq process crashes or gets kill -9'd, you risk > stranding the lock. > > In short, I'd not recommend trying to combine Redis::Semaphore and Sidekiq.