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.

Tweaks to make ActiveRecord Fiber-friendly

#4337

My work on Phat (http://www.mikeperham.com/2010/04/03/introducing-phat-an-asynchrono...) brought to light a few issues in making Rails working in a single Thread, multiple Fiber environment.

The full set of patches required for ActiveRecord is here:

http://github.com/mperham/em_postgresql/blob/master/lib/active_reco...

Some of these changes can't be easily rolled back into Rails but the ones that can IMO are:

  • The connection pool's checkout method should mark a connection as checked out before actually using it, otherwise another Fiber may try to use the same connection while the verify! operation is in progress.

In connection_pool.rb

def checkout_and_verify(c)
  @checked_out << c
  c.verify!
  c.run_callbacks :checkout
  c
end
  • The connection pool Mutex should be configurable. Since Fibers cause a single thread to check out many connections from the pool, the Mutex causes deadlocks, even though there is only a single Thread executing. Ideally, I should be able to supply my own Mutex and Condition classes which are Fiber-friendly.

Reported by Mike Perham · April 7th, 2010 @ 04:28 PM

State: open
Milestone: 3.1
Assigned to: Aaron Patterson Aaron Patterson
Importance: Medium

Activity

  1. Jeremy Kemper
    Jeremy Kemper
    • Milestone cleared.
    • State changed from new to open
    • Assigned user set to Jeremy Kemper

    Cool - thanks for working on this, Mike!

    Looks like the pool implementation itself could be configurable so you could pick a threaded or fibered variant.

    Patches (and tests) welcome, of course :)

    April 7th, 2010 @ 06:24 PM

  2. Mike Perham
    Mike Perham

    Any suggestions on the switching logic? I like the idea of config.fibersafe! which works in a similar fashion to config.threadsafe! It would be able to rejigger the Rails stack (AR, memcache-client, e.g.) to use Fiber-safe versions.

    April 7th, 2010 @ 06:56 PM

  3. Mike Perham
    Mike Perham
    • Tag set to patch

    Patch attached. I'm unhappy with a few areas that perhaps you can suggest alternatives:

    1. How do we cleanly get the concurrency_model value from the Rails::Application instance down to the ConnectionPool? My defined? code is definitely a hack.
    2. I had to pull in EventMachine into the Gemfile in order to test the Fibers actually executing. The wait queue uses EM to pause while waiting for a connection and I'm not sure we want to introduce that dependency into ActiveRecord, even if it's pulled in dynamically.
    3. There's no real way to implement remove_stale_cached_threads! since we can't get a list of Fibers in the system. I hacked it in em_postgresql by registering the application's FiberPool manually. I don't know if this is a showstopper but it's a concern.

    April 7th, 2010 @ 10:32 PM

  4. Dan Pickett
    Dan Pickett
    • Tag changed from patch to bugmash, patch

    Advanced bugmashin': Can some bugmashers critique the patch and revise/comment as necessary?

    May 15th, 2010 @ 01:54 AM

  5. Jeremy Kemper
    Jeremy Kemper
    • Milestone cleared.
    • Importance changed from to Medium

    August 30th, 2010 @ 04:10 AM

  6. Jeremy Kemper
    Jeremy Kemper
    • Milestone set to 3.0.2

    October 15th, 2010 @ 11:01 PM

  7. Ryan Bigg
    Ryan Bigg
    • Tag cleared.

    Automatic cleanup of spam.

    November 8th, 2010 @ 01:55 AM

  8. Santiago Pastorino
    Santiago Pastorino
    • Milestone cleared.

    November 15th, 2010 @ 09:04 PM

  9. wtn
    wtn

    +1

    Hope to see this patch get accepted.

    November 20th, 2010 @ 05:59 AM

  10. José Valim
    José Valim
    • Assigned user changed from Jeremy Kemper to Aaron Patterson

    November 20th, 2010 @ 10:08 AM

  11. Aaron Patterson
    Aaron Patterson
    • Milestone set to 3.1

    I will attempt to test / fix this. I don't like the architecture of the patch. I think we should have two subclasses of connection pool (one for threads and one for fibers) then switch the instantiation logic. Having initialize extend the current instance seems not good.

    We can have the spec contain configuration info. IIRC, the spec comes from the database configuration yaml file, so config could be something like:

    development:
  adapter: sqlite3
  database: db/development.sqlite3
  pool: 5
  timeout: 5000
  concurrency: fiber

    (I'm not sure this is exactly what the config file would look like)

    I'm marking this for Rails 3.1 as this seems like too much for a bugfix release.

    December 15th, 2010 @ 11:31 PM

  12. bingbing
    bingbing

    March 26th, 2011 @ 01:39 AM