New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActiveRecord::Base.establish_connection does not use the connection pool #7019

Closed
SamSaffron opened this Issue Jul 10, 2012 · 12 comments

Comments

Projects
None yet
7 participants
@SamSaffron
Contributor

SamSaffron commented Jul 10, 2012

Due to a new resolver being spun up and a new spec generated, ActiveRecord::Base.establish_connection does not make use of the connection pool at all.

Work around is to do:

resolver = ActiveRecord::Base::ConnectionSpecification::Resolver.new config, ActiveRecord::Base.configurations
ActiveRecord::Base.connection_handler.establish_connection config, resolver.spec

@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva

carlosantoniodasilva Jul 10, 2012

Member

Sorry, can you please clarify the code (use Github Flavored Markdown to style it so we can better read it), and tell us the Rails version you're using? Thanks!

Member

carlosantoniodasilva commented Jul 10, 2012

Sorry, can you please clarify the code (use Github Flavored Markdown to style it so we can better read it), and tell us the Rails version you're using? Thanks!

@SamSaffron

This comment has been minimized.

Show comment
Hide comment
@SamSaffron

SamSaffron Jul 10, 2012

Contributor

Using Rails 3.2.6

# this works and uses the pool
resolver = ActiveRecord::Base::ConnectionSpecification::Resolver.new "production", ActiveRecord::Base.configurations
ActiveRecord::Base.connection_handler.establish_connection "production", resolver.spec

# this works but does not use the pool
ActiveRecord::Base.establish_connection "production" 
Contributor

SamSaffron commented Jul 10, 2012

Using Rails 3.2.6

# this works and uses the pool
resolver = ActiveRecord::Base::ConnectionSpecification::Resolver.new "production", ActiveRecord::Base.configurations
ActiveRecord::Base.connection_handler.establish_connection "production", resolver.spec

# this works but does not use the pool
ActiveRecord::Base.establish_connection "production" 
@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva
Member

carlosantoniodasilva commented Jul 10, 2012

@SamSaffron

This comment has been minimized.

Show comment
Hide comment
@SamSaffron

SamSaffron Jul 10, 2012

Contributor

It actually is a bit worse ... "seems" the correct thing to do is:

# ActiveRecord::Base.remove_connection # don't ever call this cause it nukes the pool 
resolver = ActiveRecord::Base::ConnectionSpecification::Resolver.new "someconfig", ActiveRecord::Base.configurations
spec = resolver.spec # cache this across calls ... 
ActiveRecord::Base.connection_handler.establish_connection "ActiveRecord::Base", spec
Contributor

SamSaffron commented Jul 10, 2012

It actually is a bit worse ... "seems" the correct thing to do is:

# ActiveRecord::Base.remove_connection # don't ever call this cause it nukes the pool 
resolver = ActiveRecord::Base::ConnectionSpecification::Resolver.new "someconfig", ActiveRecord::Base.configurations
spec = resolver.spec # cache this across calls ... 
ActiveRecord::Base.connection_handler.establish_connection "ActiveRecord::Base", spec
@BMorearty

This comment has been minimized.

Show comment
Hide comment
@BMorearty

BMorearty Sep 22, 2012

Contributor

@SamSaffron is correct. If multiple tables call establish_connection independently, but using the same connection key, each table will end up with its own connection pool. Starting in 3.2, errors will occur.

I wrote up some details of it in a blog post where I tried to explain how people can make their establish_connection calls work again in 3.2.

The culprit seems to be the first line of this method in ActiveRecord::ConnectionAdapters::ConnectionHandler:

      def establish_connection(name, spec)
        @connection_pools[spec] ||= ConnectionAdapters::ConnectionPool.new(spec)
        @class_to_pool[name] = @connection_pools[spec]
      end

@connection_pools[spec] will always be nil because every time ActiveRecord::Base::establish_connection is called, a new ConnectionSpecification::Resolver is created. That resolver gets passed in as the spec param, so you're guaranteed that @connection_pools[some newly created spec] will be nil...and it will create a new pool.

Contributor

BMorearty commented Sep 22, 2012

@SamSaffron is correct. If multiple tables call establish_connection independently, but using the same connection key, each table will end up with its own connection pool. Starting in 3.2, errors will occur.

I wrote up some details of it in a blog post where I tried to explain how people can make their establish_connection calls work again in 3.2.

The culprit seems to be the first line of this method in ActiveRecord::ConnectionAdapters::ConnectionHandler:

      def establish_connection(name, spec)
        @connection_pools[spec] ||= ConnectionAdapters::ConnectionPool.new(spec)
        @class_to_pool[name] = @connection_pools[spec]
      end

@connection_pools[spec] will always be nil because every time ActiveRecord::Base::establish_connection is called, a new ConnectionSpecification::Resolver is created. That resolver gets passed in as the spec param, so you're guaranteed that @connection_pools[some newly created spec] will be nil...and it will create a new pool.

@fingermark

This comment has been minimized.

Show comment
Hide comment
@fingermark

fingermark Sep 27, 2012

Just got bit by this pretty hard. So, I applied the suggestions by @BMorearty to subclass AR::Base. And got bit by #5872. So I'm on rails 3-2-stable. So far so good: no prepared statement errors and no connections going out of control (with BMorearty's code change suggestions). Weird thing is I have a pool size of 1 with 4 unicorn workers. My default database has 4 db connections (also w/ a pool size of 1). My non-default db only has 1. I was expecting 4 like I have with my default, since I have 4 workers.

fingermark commented Sep 27, 2012

Just got bit by this pretty hard. So, I applied the suggestions by @BMorearty to subclass AR::Base. And got bit by #5872. So I'm on rails 3-2-stable. So far so good: no prepared statement errors and no connections going out of control (with BMorearty's code change suggestions). Weird thing is I have a pool size of 1 with 4 unicorn workers. My default database has 4 db connections (also w/ a pool size of 1). My non-default db only has 1. I was expecting 4 like I have with my default, since I have 4 workers.

@fingermark

This comment has been minimized.

Show comment
Hide comment
@fingermark

fingermark Sep 28, 2012

@jonleighton do you know if your recent connection pool and connection handler refactoring addressed this?

fingermark commented Sep 28, 2012

@jonleighton do you know if your recent connection pool and connection handler refactoring addressed this?

@jonleighton

This comment has been minimized.

Show comment
Hide comment
@jonleighton

jonleighton Sep 28, 2012

Member

I don't know. Please feel free to try master and update the ticket, but this sounds bad so I'll try to find time to take a look.

Member

jonleighton commented Sep 28, 2012

I don't know. Please feel free to try master and update the ticket, but this sounds bad so I'll try to find time to take a look.

@jonleighton

This comment has been minimized.

Show comment
Hide comment
@jonleighton

jonleighton Oct 19, 2012

Member

Looking into this a bit more, I don't think this is actually a bug. What's described here is how it's designed to work: every time you call establish_connection you create a new pool. Therefore, if you want to share pools between classes you should use an abstract superclass.

Member

jonleighton commented Oct 19, 2012

Looking into this a bit more, I don't think this is actually a bug. What's described here is how it's designed to work: every time you call establish_connection you create a new pool. Therefore, if you want to share pools between classes you should use an abstract superclass.

@BMorearty

This comment has been minimized.

Show comment
Hide comment
@BMorearty

BMorearty Oct 19, 2012

Contributor

Thanks for looking at it, @jonleighton. Much appreciated. I guess the fact that it seemed to "work" (before 3.2) was a bug--those of us who didn't know we were doing it wrong were just getting lucky.

Contributor

BMorearty commented Oct 19, 2012

Thanks for looking at it, @jonleighton. Much appreciated. I guess the fact that it seemed to "work" (before 3.2) was a bug--those of us who didn't know we were doing it wrong were just getting lucky.

@himanshu-saxena

This comment has been minimized.

Show comment
Hide comment
@himanshu-saxena

himanshu-saxena Sep 27, 2013

Hi All,
Can you please guide me , how can I use two database in same development environment

Thanks

himanshu-saxena commented Sep 27, 2013

Hi All,
Can you please guide me , how can I use two database in same development environment

Thanks

@robin850

This comment has been minimized.

Show comment
Hide comment
@robin850

robin850 Sep 27, 2013

Member

@Mobiloitte : Could you ask your question on the rubyonrails-talk mailing list please ? This is not really the place for such questions. Thank you!

Member

robin850 commented Sep 27, 2013

@Mobiloitte : Could you ask your question on the rubyonrails-talk mailing list please ? This is not really the place for such questions. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment