Really fiber-aware ActiveRecord connection pool and em_mysql2 adapter

[dmgtn]

This implementation supports concurrent running transactions. We tested it with active_record 3.2, but did not test with 4.0. We are using it in production and it is working well.

ActiveRecord have already had good connection pooling, that uses Thread.current[] for thread-aware (and fiber-aware) storage. We replaced original MonitorMixin, by fiber-aware one, and thats all.

[igrigorik]

Looks to be much simpler implementation, which is a good sign. That said, would be great to get 4.0 support! Any chance you could give it a shot and see what the delta is (if any)?

[marshall-lee]

@mrms-dos @igrigorik Are you still here ??

I have some questions

Am I right that everything should work without defining custom adapters if we just:

$VERBOSE.tap do |old_verbose|
  $VERBOSE = nil
  Mysql2::Client = Mysql2::EM::Client
  $VERBOSE = old_verbose
end

It's really dirty but doing copy-paste of some of the Mysql2Adapter implementation is dirtier :) And I don't think that someone would want both mysql2 and em_mysql2 in the same project.

[marshall-lee]

What was the motivation of stubbing clear_stale_cached_connections! ?

UPD: In ActiveRecord 4.x clear_stale_cached_connections! is replaced with reap.

• In 4.0 and 4.1 reap is not been calling by default from checkout like clear_stale_cached_connections!. It is been calling by separate Reaper thread that calls reap periodically but only if reaping_frequency is set in database.yml.
• In 4.2 it's been calling from both acquire_connection and Reaper thread (which is optional, again).

I think we should not stub reap in ActiveRecord 4.x because it's thread-agnostic now unlike clear_stale_cached_connections!.

We also can implement Reaper differently: use EM timers instead of thread with sleep.

module ActiveRecord
  module ConnectionAdapters
    class ConnectionPool
      if ActiveRecord::VERSION::MAJOR == 4
        class Reaper
          def run
            return unless frequency
            EM::Synchrony.add_periodic_timer(frequency) do
              pool.reap
            end
          end
        end
      end
    end
  end
end

It would be useful! But we cannot backport it to ActiveRecord 3.x, so clear_stale_cached_connections! must be just stubbed like in this PR.

[marshall-lee]

ActiveRecord::Base.connection_id ||= Fiber.current.object_id

Taking Fiber.current.object_id is right but Base.connection_id is still saved per thread. Is it potentially harmful?

UPD: sorry, i just realized that Thread.current is also fiber-local.

[marshall-lee]

Now i'm trying to use em-pg-client with ActiveRecord 4.2 and ConnectionPool with fiber aware MonitorMixin from this PR. Things are almost the same at first sight and it should work.

The little difference coming from 4.2 is:

module ActiveRecord
  module ConnectionAdapters
    class AbstractAdapter
      include EventMachine::Synchrony::MonitorMixin

      if [ActiveRecord::VERSION::MAJOR, ActiveRecord::VERSION::MINOR] == [4, 2]
        def lease
          synchronize do
            unless in_use?
              @owner = Fiber.current
            end
          end
        end
      end
    end
  end
end

For PostgreSQL users:

require 'em-pg-client'

$VERBOSE.tap do |old_verbose|
  $VERBOSE = nil
  PGconn = PG::EM::Client
  $VERBOSE = old_verbose
end

[marshall-lee]

Please see #190

[dgutov]

@mrms-dos @marshall-lee Please merge the current master in your pull request branches, or rebase on top of it, so that they build on Travis, and we can see how the specs pass on different ActiveRecord versions.

[marshall-lee]

@dgutov did it in #190

[igrigorik]

Looks like this and #190 are duplicates. Any objections to closing this one in favor of #190 -- seems like we're farther ahead there.