brendon / acts_as_list

An ActiveRecord plugin for managing lists.
http://brendon.github.io/acts_as_list/
MIT License
2.04k stars 355 forks source link

Deadlocking in update_positions count query #285

Closed cmentch closed 6 years ago

cmentch commented 6 years ago

I have a larger database where I am using acts_as_list on various models. I have started seeing increased deadlocking on a specific count query. The stack dump revealed it was from the method "update_positions". This is being called from an after_update trigger on the model. Even if I am not updating the ordering column, this method is still firing off which seems unnecessary. Is there a need to trigger this even if the ordering column is not updated?

brendon commented 6 years ago

Hi @cmentch, Thanks for picking this up. I've committed a change to master that checks if the position has changed first before going through with the callback. It tests out well locally, and let's see if the CI passes. Can you have a go using master and see if you get the deadlocks still?

brendon commented 6 years ago

There was some trouble with Timecop. I think I've fixed it, so we'll see :)

cmentch commented 6 years ago

Thank you for the quick response. I have pointed this to master in my Gemfile. It looks like it is no longer performing the count query when the ordering column is not updated. Perfect! I won't be able to try this out in production yet. When do you formally publish a new version of the gem?

brendon commented 6 years ago

I'll just get @swanandp to take a quick look. @swanandp, I can't see any problems. The tests pass.

brendon commented 6 years ago

@cmentch, ping me back if we don't hear anything from @swanandp within a few days.

swanandp commented 6 years ago

@brendon https://github.com/swanandp/acts_as_list/commit/75ec5b9b3a7e72a35bc5bd95ca9061d44ece2a76

Looks good. 👍

brendon commented 6 years ago

Roger. I'll release a new version @cmentch :)