mysql2 doesn't seem to be threadsafe #461
Comments
|
What version of mysql2, what version of libmysqlclient? |
libmariadbclient 5.5.34-3 same problem with: ii libmysqlclient18 5.5.32-0ubuntu0.12.04.1 MySQL database client library |
|
Huh. That should just work. Could you show me the output of |
|
On the mariadb machine: On the other one: |
|
According to this line in extconf.rb, MySQL 5.5+ is already thread-safe without the _r variant: https://github.com/brianmario/mysql2/blob/master/ext/mysql2/extconf.rb#L59 I wonder if this isn't true or if there is some other nuance about it. |
|
In 5.5 the _r versions are just symlinked to the normal ones. |
|
That's what I thought, so we're not missing anything obvious there :) mysql2 is designed and intended to be thread-safe, so if your test case can help turn up bugs in that, I'm happy to take patches and work on it! |
|
Well, I've come up with a smaller test case, without puma, etc, which makes debugging significantly easier. The code is at https://github.com/bwalex/mysql2_461 To try it, you'll have to adjust the path of the mysql2 gem in the Gemfile as it's currently set to use my local copy. Run with It fails quite miserably with the code that is checked in. However, commenting out the following line in test.rb #ActiveRecord::Base.connection_pool.connections.map(&:verify!)seems to make it work. That's also consistent with the fact that most backtraces end up including nogvl_ping. Someone familiar with the code should be able to track down what's going on. My guess is that nogvl_ping needs to be locking or holding a structure that is getting ripped out from under it when running a query just after the verify/ping. As expected, it also fails using &:active? instead of &:verify! |
|
Some more details below. In this case, mysql ends up with a client structure with a NULL vio, which later blows up. Not too familiar with all the internals at work here, but it looks like the same client structure is being reused between threads, so it's blowing up because some other thread clears the vio. I see how that might happen if ActiveRecord::Base.connection_pool.connections.map() operates on all connections, including connections used by other threads. If that's the case, at the very least the 'ping' operation (and possibly also connect, disconnect, etc) would have to interlock against any other operation with that same mysql client structure. It's probably fair enough not to, but a warning about this scenario would be appropriate. |
|
Nice debugging! Each mysql2 client instance has self-contained resources, so in that respect there's no thread-safety problem. But we're not protecting the user from grabbing handles across threads, so far we've left that to be an application or connection pool implementation issue. |
|
I'm also running into a similar issue when running Puma against MySql2. The returned error varies which would imply a threading issue. Here is the lowest level stack we've encountered: This stack was caught by our CI server on Semaphore. |
|
@dimroc You are using mysql2 0.3.11. Please upgrade. |
|
@bwalex I missed out on reading your test case code when you posted it, sorry. I'm just reading it now. My comments: The mysql2 gem does not handle connection locking; we're leaving that up to the connection pool implementation. The Active Record Connection Pool provides the tools for this, per http://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/ConnectionPool.html
So this code fails, as expected, because you're not using the thread checkout functions: def run_test
puts "Thread #{Thread.current.object_id} reporting for duty."
loop do
#ActiveRecord::Base.connection_pool.with_connection do
ActiveRecord::Base.connection_pool.connections.map(&:verify!)
Test.all.to_a
ActiveRecord::Base.clear_active_connections!
#end
end
end
t = []
(1..32).each do |i|
t << Thread.new { run_test }
end
t.each { |thread| thread.join } |
|
Pasted that call stack to show the similarity with OP. Continue to see spurious errors on mysql2 0.3.15 with the following callstacks. Seems to succeed 50% of the time. This does not repro when running on a single thread via unicorn as opposed to puma. This is assuming that the rails stack (activerecord) is handling the connection pool you previously mentioned. |
|
This is weird. I would expect that Rails itself uses its own connection pooling correctly. Do you know if Puma is using a mix of threads and processes? I wonder if the fix that landed in a2bcfd6 would help here, if Puma uses a connection pool shared across processes (as well as threads). |
|
Run locally, puma's a single process running approximately 16 threads. If you would like, I can try a particular SHA on mysql2 to help isolate the issue. Both 0.3.11 and 0.3.15 showed the issue. Let me know if there's anything particular you would like me to try. |
|
Thank you! Yes, please try revision a2bcfd6 as well. |
|
Hi @sodabrew , |
|
#530 may be a full resolution to this issue. |
Running the code in https://github.com/bwalex/ar_threads with the mysql2 adapter (just adapt config/database.yml) and puma results in a fair number of random-looking failures:
Sometimes even a segfault:
Running the same with unicorn (single-threaded webserver) everything's fine.
The text was updated successfully, but these errors were encountered: