Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

connections not released in rails 3

#5736

When a activerecord asks for a connection in a thread, this connections is not released after thread end. So, the connection pool can be exhausted. Definining a pool of 5 for the DB connection (mysql), this code fails after the 5th try:

puts "ruby vesion: #{RUBY_VERSION} rails version: #{Rails.version}"
1.upto(10) do |i|
     Thread.new() do
        puts "iteration #{i}"
        User.first
        puts "iteration #{i} after query"
        # ActiveRecord::Base.connection_pool.release_connection
    end.join
    # ActiveRecord::Base.connection_pool.clear_stale_cached_connections!() 
end

the result is:

$rails runner /tmp/test2.rb
ruby vesion: 1.9.2 rails version: 3.0.0
iteration 1
iteration 1 after query
iteration 2
iteration 2 after query
iteration 3
iteration 3 after query
iteration 4
iteration 4 after query
iteration 5
iteration 5 after query
iteration 6
/tmp/test2.rb:4:in join': deadlock detected (fatal)</code>
</pre>



Uncommenting one of the commented lines that manually forces to release connections makes it works but I guess it's not the expected behaviour since it worked without problems in previous version (tested with ruby 1.8.7 and rails 2.3.5)



It fails too with rails 3.1.0.beta.

Reported by luis.lopez (at branelabs) · September 29th, 2010 @ 09:27 PM

State: committed
Milestone: none
Assigned to: Aaron Patterson Aaron Patterson
Importance: none

Activity

  1. Hemant Kumar
    Hemant Kumar
    • Assigned user set to Aaron Patterson

    Verified on Ruby 1.9. The problem with checkout methods is, it relies on return value of wait method to reclaim connections from dead threads.

    But return value of wait has been changed and in 1.9 it will return always true, whether it returns after timeout period or it returns because it was signalled by singal. In fact, return value of wait can't be relied upon and I saw the code of Rubinius and JRuby implementations as well and behaviour is not analogous to 1.8.

    Attached patch fixes the problem and it no longer depends on return value of wait method. I have also added some missing tests. One more thing that I did ended up doing was setting timeout value on 1.9; original code had timeout set to nil in Ruby 1.9. Calling wait with nil parameter can trigger deadlock as OP got in original bug report.

    September 30th, 2010 @ 10:08 AM

  2. Hemant Kumar
    Hemant Kumar
    • Tag changed from rails 3.0.0, connection, pool, thread to rails 3.0.0, connection, patched, pool, thread

    September 30th, 2010 @ 10:19 AM

  3. luis.lopez (at branelabs)
    luis.lopez (at branelabs)
    • Tag cleared.

    The patch works. The only drawback is the waiting time to release connections. If you run the above code, it will freeze for 5 seconds in the 6th try. If you do the test more times (1.upto(20), for instance) it will freeze 5 seconds every 5 tries (since the pool is 5 for this test). Not sure if it is possible to check if there are connections to be reclaimed before wait.

    September 30th, 2010 @ 12:06 PM

  4. gnufied
    gnufied

    Well the patch does not modify original behaviour, it just makes it work on Ruby 1.9. One can of course check for connections held by dead threads before calling wait,but that would mean that we will have to call clear_stale_cached_connections twice:

    clear_stale_cached_connections!

if(@checked_out.size < @connections.size)
  next
else
  @queue.wait(@timeout)
end            
clear_stale_cached_connections!
if @size == @checked_out.size
  raise ConnectionTimeoutError, "could not obtain a database connection#{" within #{@timeout} seconds" if @timeout}.  The max pool size is currently #{@size}; consider increasing it."
end

    September 30th, 2010 @ 02:07 PM

  5. luis.lopez (at branelabs)
    luis.lopez (at branelabs)

    I personally think that it would be a better solution: the best moment to check for connections is probably before the wait and I don't think that call clear_stale_cached_connections twice would be big problem since there is a wait anyway. In the worst case there will be 2 calls to clear_stale_cached_connections (one more then in the current code) and in the best the wait would be avoided.

    However, I don't know if clear_stale_cached_connections! could have a big performance penalty in case the pool were big

    September 30th, 2010 @ 05:18 PM

  6. Repository
    Repository
    • State changed from new to committed

    October 6th, 2010 @ 09:43 PM

  7. Repository
    Repository

    October 6th, 2010 @ 09:46 PM