Deadlock in preload_indices #1132
Comments
|
I'd like to hesitantly propose this as a fix... diff --git a/lib/thinking_sphinx/configuration.rb b/lib/thinking_sphinx/configuration.rb
index 7ce15dd7..e983e8a5 100644
--- a/lib/thinking_sphinx/configuration.rb
+++ b/lib/thinking_sphinx/configuration.rb
@@ -10,7 +10,7 @@ class ThinkingSphinx::Configuration < Riddle::Configuration
delegate :environment, :to => :framework
- @@mutex = Mutex.new
+ @@mutex = ActiveSupport::Concurrency::LoadInterlockAwareMonitor.new
def initialize
superWhich appears to fix my deadlocks. I don't understand enough of AS::Dependencies' locking insanity to say for sure it's the correct fix, though. |
|
I'm glad to know you've found a fix! And I'm a little surprised the issue you've hit hasn't been noted by anyone else… I've used Thinking Sphinx and Puma together in my apps, and I'm sure others have too. 🤔 I'll have a look into the ActiveSupport monitor class soon to confirm it's a good fit - but the fact it's working for you is a great sign. |
|
I think the reason I'm consistently running into it is that we have a before_filter that creates an AuditLog activerecord object on every single request, and pages that fire multiple simultaneous ajax calls, so I'm probably doing more concurrent inserts than most people. I've created a demo repository here - https://github.com/jdelStrother/thinkingsphinx-deadlock. The readme has instructions on reproducing the problem, but the money shot is here: https://github.com/jdelStrother/thinkingsphinx-deadlock/blob/master/app/controllers/application_controller.rb#L32. Let me know if I can do anything to help. |
This seems to be related to some changes with Rails' database locking behaviour, which was added in Rails 5.1. This monitor class was added in response to similar issues - and is only present since 5.1.5. The underlying issue noted in #1132 may possibly crop up in earlier Rails 5.1.x releases, but this should resolve things for anything newer. Thanks to @jdelStrother for doing all the hard work on this!
|
Just pushed a commit that should resolve this - essentially, your solution, but only when that ActiveSupport class is available (which seems to be from 5.1.5 onwards, for an issue that was introduced at some point in the 5.1.x releases). Thanks so much for the test app too, that was a great way to review the problem! 👏 |
|
This fix is available in the newly-released v4.3.0, with credit to you in the release notes/changelog 😄 |
Otherwise concurrent threads (e.g. sidekiq, puma) can deadlock while racing to obtain access to the mutex block at https://github.com/pat/thinking-sphinx/blob/v3.4.2/lib/thinking_sphinx/configuration.rb#L78. This bug was fixed in ThinkingSphinx v4.3.0+. See - pat/thinking-sphinx#1051 - pat/thinking-sphinx#1132
I run multithreaded puma in development, and get fairly frequent deadlocks if two concurrent requests try to create a record.
The problem appears to be due to the mutex handling in
Configuration#preload_indices:I hoped I could fix this by wrapping the mutex in
permit_concurrent_loads, but that doesn't appear to help. Any other suggestions I might try?The text was updated successfully, but these errors were encountered: