Skip to content
This repository has been archived by the owner on Jan 15, 2024. It is now read-only.

ConnectionPool checkin should put connection back into the pool #228

Merged
merged 1 commit into from Nov 5, 2013

Conversation

arthurnn
Copy link
Contributor

@arthurnn arthurnn commented Nov 3, 2013

Replaced #227 with this, so we PR against master.

We need to checkin the connection(put back in the pool) after use.
We cannot only depend on the reaper to get rid of connection, as threads could be sleeping or blocked by something else. If we make the connections rotate in the pool, we could have more threads using less connections.

just for an example, other gems are going the same thing:
https://github.com/rails/rails/blob/master/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb#L370
https://github.com/zk-ruby/zk/blob/master/lib/zk/pool.rb#L203
https://github.com/mperham/connection_pool/blob/master/lib/connection_pool.rb#L79

[fixes #223]

review @durran

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 6acc591 on conn_pool_checkin into 8433b87 on master.

durran added a commit that referenced this pull request Nov 5, 2013
ConnectionPool checkin should put connection back into the pool
@durran durran merged commit cd4989b into master Nov 5, 2013
@arthurnn arthurnn deleted the conn_pool_checkin branch November 5, 2013 17:29
@thibaudgg
Copy link

With this commit (using Moped master instead of beta3) I got a lot of Moped::Errors::AuthenticationFailure, running with Sidekiq and pool_size equals to concurrency (20).

Moped::Errors::AuthenticationFailure: The operation: #<Moped::Protocol::Commands::Authenticate @length=155 @request_id=1 @response_to=0 @op_code=2004 @flags=[] @full_collection_name="app13829477.$cmd" @skip=0 @limit=-1 @selector={:authenticate=>1, :user=>"heroku", :nonce=>"ec0e22bfbeb9454c", :key=>"20307c131d89639536c59da74595c481"} @fields=nil> failed with error 18: "auth fails" See https://github.com/mongodb/mongo/blob/master/docs/errors.md for details about this error.

[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/authenticatable.rb:68:in `block in login`
[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/node.rb:130:in `block in connection`
[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/connection/pool.rb:159:in `with_connection`
[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/node.rb:129:in `connection`
[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/authenticatable.rb:65:in `login`
[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/authenticatable.rb:28:in `block in apply_credentials`
[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/authenticatable.rb:26:in `each`
[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/authenticatable.rb:26:in `apply_credentials`
[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/cluster.rb:227:in `block in with_primary`
[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/node.rb:212:in `block in ensure_primary`
[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/executable.rb:25:in `execute`
[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/node.rb:211:in `ensure_primary`
[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/cluster.rb:226:in `with_primary`
[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/query.rb:379:in `update`
[PROJECT_ROOT]/vendor/bundle/ruby/2.0.0/bundler/gems/moped-a87255b0dd17/lib/moped/query.rb:420:in `upsert`

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connection Timeout - Waited for item but none was pushed.
4 participants