From d652fd91102fbbe2967b82847b90d050330778d9 Mon Sep 17 00:00:00 2001 From: thomas morgan Date: Sat, 4 Oct 2014 13:06:21 -0600 Subject: [PATCH 1/5] Don't try to disconnect if address never resolved to begin with Call to disconnect triggers call to pool, which uses address.resolved as a hash key. This in turn causes pollution of the connection pool. --- lib/moped/node.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/moped/node.rb b/lib/moped/node.rb index 0ede953..0fdafff 100644 --- a/lib/moped/node.rb +++ b/lib/moped/node.rb @@ -137,7 +137,7 @@ def down? # # @since 1.2.0 def disconnect - connection{ |conn| conn.disconnect } + connection{ |conn| conn.disconnect } if address.resolved true end From 4f27e3fbe0d153e7981fa9e73178906f47e60f08 Mon Sep 17 00:00:00 2001 From: thomas morgan Date: Sat, 4 Oct 2014 13:07:37 -0600 Subject: [PATCH 2/5] Don't append nodes to seeds until DNS resolves Test for equality of Node (==,#eql?) depends on node.address.resolved. Therefore avoid such tests (such as .include?() ) until address is resolved. This fixes unbounded growth (read: memory leak) of the seeds array in unstable network environments. --- lib/moped/cluster.rb | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/moped/cluster.rb b/lib/moped/cluster.rb index b239c03..fa19d96 100644 --- a/lib/moped/cluster.rb +++ b/lib/moped/cluster.rb @@ -174,7 +174,13 @@ def refresh(nodes_to_refresh = seeds) seen = {} # Set up a recursive lambda function for refreshing a node and it's peers. refresh_node = ->(node) do - unless seen[node] + unless node.address.resolved + begin + node.refresh + rescue Errors::ConnectionFailure + end + end + unless seen[node] || !node.address.resolved seen[node] = true # Add the node to the global list of known nodes. seeds.push(node) unless seeds.include?(node) @@ -369,8 +375,10 @@ def initialize_copy(_) # @since 1.0.0 def refresh_peers(node, &block) node.peers.each do |node| - block.call(node) unless seeds.include?(node) - peers.push(node) unless peers.include?(node) + if node.address.resolved + block.call(node) unless seeds.include?(node) + peers.push(node) unless peers.include?(node) + end end end end From 004f7379879f75bcbe81b974b04239eda78a2e48 Mon Sep 17 00:00:00 2001 From: thomas morgan Date: Sat, 4 Oct 2014 13:09:40 -0600 Subject: [PATCH 3/5] Prevent ensure_connected from returning half-setup connection on reentry --- lib/moped/node.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/moped/node.rb b/lib/moped/node.rb index 0fdafff..dd512e9 100644 --- a/lib/moped/node.rb +++ b/lib/moped/node.rb @@ -175,9 +175,9 @@ def ensure_connected(&block) begin connection do |conn| - stack(:connection) << conn connect(conn) unless conn.connected? conn.apply_credentials(@credentials) + stack(:connection) << conn yield(conn) end rescue Exception => e From a7f9ec016c6cd519dc5a69075bcd68d5dc36bf5c Mon Sep 17 00:00:00 2001 From: thomas morgan Date: Sat, 4 Oct 2014 13:10:24 -0600 Subject: [PATCH 4/5] Test for a connection should include non-alive socket --- lib/moped/node.rb | 2 +- spec/moped/node_spec.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/moped/node.rb b/lib/moped/node.rb index dd512e9..4431470 100644 --- a/lib/moped/node.rb +++ b/lib/moped/node.rb @@ -175,7 +175,7 @@ def ensure_connected(&block) begin connection do |conn| - connect(conn) unless conn.connected? + connect(conn) unless conn.alive? conn.apply_credentials(@credentials) stack(:connection) << conn yield(conn) diff --git a/spec/moped/node_spec.rb b/spec/moped/node_spec.rb index ee641cb..df034ee 100644 --- a/spec/moped/node_spec.rb +++ b/spec/moped/node_spec.rb @@ -215,6 +215,7 @@ before do node.connection do |conn| conn.stub(:connected?).and_return(true) + conn.stub(:alive?).and_return(false) conn.instance_variable_set(:@sock, nil) end end From 695971d5470c21c9021295b4be861a7bea4671d0 Mon Sep 17 00:00:00 2001 From: thomas morgan Date: Sat, 4 Oct 2014 13:11:38 -0600 Subject: [PATCH 5/5] Ensure mongodb credentials are always sent upon (re)connection If existing socket is reset, perhaps due to mongod failure, credentials hash isn't reset. This causes them to not be sent upon reconnection. Changed to reset at connect() instead of relying on a disconnect() that may or may not happen. Also cache last set of credentials so they can be reused when connection loss is detected by with_connection(). --- lib/moped/authenticatable.rb | 1 + lib/moped/connection.rb | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/moped/authenticatable.rb b/lib/moped/authenticatable.rb index 06d4ac3..de99a7f 100644 --- a/lib/moped/authenticatable.rb +++ b/lib/moped/authenticatable.rb @@ -28,6 +28,7 @@ def apply_credentials(logins) login(database, username, password) end end + @original_credentials = credentials.dup end self end diff --git a/lib/moped/connection.rb b/lib/moped/connection.rb index 0041867..08aa576 100644 --- a/lib/moped/connection.rb +++ b/lib/moped/connection.rb @@ -46,6 +46,7 @@ def alive? # # @since 1.0.0 def connect + credentials.clear @sock = if !!options[:ssl] Socket::SSL.connect(host, port, timeout) else @@ -74,7 +75,6 @@ def connected? # # @since 1.0.0 def disconnect - credentials.clear @sock.close rescue ensure @@ -217,7 +217,10 @@ def read_data(socket, length) # # @since 1.3.0 def with_connection - connect if @sock.nil? || !@sock.alive? + if @sock.nil? || !@sock.alive? + connect + apply_credentials(@original_credentials || {}) + end yield @sock end end