From 3e3f05813af7190a174dd2e99823f0b106984aae Mon Sep 17 00:00:00 2001 From: Kyle Banker Date: Thu, 15 Sep 2011 17:46:59 -0400 Subject: [PATCH] minor: cleanup and test fix --- lib/mongo/repl_set_connection.rb | 32 ++++++++------- lib/mongo/util/node.rb | 66 +++++++++++++++---------------- test/replica_sets/refresh_test.rb | 14 +++---- test/unit/pool_manager_test.rb | 4 +- 4 files changed, 59 insertions(+), 57 deletions(-) diff --git a/lib/mongo/repl_set_connection.rb b/lib/mongo/repl_set_connection.rb index cc9bb59..1bbd10c 100644 --- a/lib/mongo/repl_set_connection.rb +++ b/lib/mongo/repl_set_connection.rb @@ -23,7 +23,8 @@ module Mongo # Instantiates and manages connections to a MongoDB replica set. class ReplSetConnection < Connection attr_reader :nodes, :secondaries, :arbiters, :secondary_pools, - :replica_set_name, :read_pool, :seeds, :tags_to_pools, :refresh_interval, :auto_refresh + :replica_set_name, :read_pool, :seeds, :tags_to_pools, + :refresh_interval, :background_refresh # Create a connection to a MongoDB replica set. # @@ -56,11 +57,12 @@ module Mongo # @option opts [Float] :connect_timeout (nil) The number of seconds to wait before timing out a # connection attempt. # @option opts [Boolean] :ssl (false) If true, create the connection to the server using SSL. - # @option opts [Boolean] :auto_refresh (false) Set this to true to enable a background thread that + # @option opts [Boolean] :background_refresh (false) Set this to true to enable a background thread that # periodically updates the state of the connection. If, for example, you initially connect while a secondary - # is down, :auto_refresh will reconnect to that secondary behind the scenes to - # prevent you from having to reconnect manually. - # @option opts [Integer] :refresh_interval (90) If :auto_refresh is enabled, this is the number of seconds + # is down, :background_refresh will reconnect to that secondary behind the scenes to + # prevent you from having to reconnect manually. If set to +false+, background refresh will happen + # synchronously. + # @option opts [Integer] :refresh_interval (90) If :background_refresh is enabled, this is the number of seconds # that the background thread will sleep between calls to check the replica set's state. # @option opts [Boolean] :require_primary (true) If true, require a primary node for the connection # to succeed. Otherwise, connection will succeed as long as there's at least one secondary. @@ -118,7 +120,7 @@ module Mongo @arbiters = [] # Refresh - @auto_refresh = opts.fetch(:auto_refresh, false) + @background_refresh = opts.fetch(:background_refresh, false) @refresh_interval = opts[:refresh_interval] || 90 # Are we allowing reads from secondaries? @@ -151,7 +153,7 @@ module Mongo end # Require a primary node to connect? - @require_primary = opts.fetch(:require_primary, false) + @require_primary = opts.fetch(:require_primary, true) setup(opts) end @@ -163,14 +165,14 @@ module Mongo # Initiate a connection to the replica set. def connect - log(:debug, "Connecting.") + log(:debug, "Connecting...") sync_synchronize(:EX) do return if @connected manager = PoolManager.new(self, @seeds) manager.connect update_config(manager) - initiate_auto_refresh + initiate_background_refresh if @require_primary && @primary.nil? #TODO: in v2.0, we'll let this be optional and do a lazy connect. raise ConnectionFailure, "Failed to connect to primary node." @@ -241,7 +243,8 @@ module Mongo end def nodes - warn "DEPRECATED" + warn "ReplSetConnection#nodes is DEPRECATED and will be removed in v2.0. " + + "Please use ReplSetConnection#seeds instead." @seeds end @@ -261,7 +264,6 @@ module Mongo end # Close the connection to the database. - # TODO: we should get an exclusive lock here. def close sync_synchronize(:EX) do @connected = false @@ -331,8 +333,8 @@ module Mongo private - def initiate_auto_refresh - if @auto_refresh + def initiate_background_refresh + if @background_refresh return if @refresh_thread && @refresh_thread.alive? @refresh_thread = Thread.new do while true do @@ -441,7 +443,9 @@ module Mongo end end - if !@auto_refresh && + # Refresh synchronously every @refresh_interval seconds + # if @background_refresh is false. + if !@background_refresh && ((Time.now - @last_refresh) > @refresh_interval) refresh end diff --git a/lib/mongo/util/node.rb b/lib/mongo/util/node.rb index ef9d7cf..8402808 100644 --- a/lib/mongo/util/node.rb +++ b/lib/mongo/util/node.rb @@ -4,15 +4,15 @@ module Mongo attr_accessor :host, :port, :address, :config, :connection, :socket def initialize(connection, data) - self.connection = connection + @connection = connection if data.is_a?(String) - self.host, self.port = split_nodes(data) + @host, @port = split_nodes(data) else - self.host = data[0] - self.port = data[1].nil? ? Connection::DEFAULT_PORT : data[1].to_i + @host = data[0] + @port = data[1].nil? ? Connection::DEFAULT_PORT : data[1].to_i end - self.address = "#{host}:#{port}" - self.config = nil + @address = "#{host}:#{port}" + @config = nil end def eql?(other) @@ -34,12 +34,12 @@ module Mongo def connect begin socket = nil - if self.connection.connect_timeout - Mongo::TimeoutHandler.timeout(self.connection.connect_timeout, OperationTimeout) do - socket = self.connection.socket_class.new(self.host, self.port) + if @connection.connect_timeout + Mongo::TimeoutHandler.timeout(@connection.connect_timeout, OperationTimeout) do + socket = @connection.socket_class.new(@host, @port) end else - socket = self.connection.socket_class.new(self.host, self.port) + socket = @connection.socket_class.new(@host, @port) end if socket.nil? @@ -48,29 +48,29 @@ module Mongo socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) end rescue OperationTimeout, OperationFailure, SocketError, SystemCallError, IOError => ex - self.connection.log(:debug, "Failed connection to #{host_string} with #{ex.class}, #{ex.message}.") + @connection.log(:debug, "Failed connection to #{host_string} with #{ex.class}, #{ex.message}.") socket.close if socket return nil end - self.socket = socket + @socket = socket end def close - if self.socket - self.socket.close - self.socket = nil - self.config = nil + if @socket + @socket.close + @socket = nil + @config = nil end end def connected? - self.socket != nil + @socket != nil end def active? begin - result = self.connection['admin'].command({:ping => 1}, :socket => self.socket) + result = @connection['admin'].command({:ping => 1}, :socket => @socket) return result['ok'] == 1 rescue OperationFailure, SocketError, SystemCallError, IOError => ex return nil @@ -82,28 +82,28 @@ module Mongo # matches with the name provided. def set_config begin - self.config = self.connection['admin'].command({:ismaster => 1}, :socket => self.socket) + @config = @connection['admin'].command({:ismaster => 1}, :socket => @socket) - if self.config['msg'] && @logger - self.connection.log(:warn, "#{config['msg']}") + if @config['msg'] && @logger + @connection.log(:warn, "#{config['msg']}") end check_set_membership(config) check_set_name(config) rescue ConnectionFailure, OperationFailure, SocketError, SystemCallError, IOError => ex - self.connection.log(:warn, "Attempted connection to node #{host_string} raised " + + @connection.log(:warn, "Attempted connection to node #{host_string} raised " + "#{ex.class}: #{ex.message}") return nil end - self.config + @config end # Return a list of replica set nodes from the config. # Note: this excludes arbiters. def node_list connect unless connected? - set_config unless self.config + set_config unless @config return [] unless config @@ -115,7 +115,7 @@ module Mongo def arbiters connect unless connected? - set_config unless self.config + set_config unless @config return [] unless config['arbiters'] config['arbiters'].map do |arbiter| @@ -125,22 +125,22 @@ module Mongo def tags connect unless connected? - set_config unless self.config + set_config unless @config return {} unless config['tags'] && !config['tags'].empty? config['tags'] end def primary? - self.config['ismaster'] == true || self.config['ismaster'] == 1 + @config['ismaster'] == true || @config['ismaster'] == 1 end def secondary? - self.config['secondary'] == true || self.config['secondary'] == 1 + @config['secondary'] == true || @config['secondary'] == 1 end def host_port - [self.host, self.port] + [@host, @port] end def hash @@ -168,13 +168,13 @@ module Mongo # Ensure that this node is part of a replica set of the expected name. def check_set_name(config) - if self.connection.replica_set_name + if @connection.replica_set_name if !config['setName'] - self.connection.log(:warn, "Could not verify replica set name for member #{host_string} " + + @connection.log(:warn, "Could not verify replica set name for member #{host_string} " + "because ismaster does not return name in this version of MongoDB") - elsif self.connection.replica_set_name != config['setName'] + elsif @connection.replica_set_name != config['setName'] message = "Attempting to connect to replica set '#{config['setName']}' on member #{host_string} " + - "but expected '#{self.connection.replica_set_name}'" + "but expected '#{@connection.replica_set_name}'" raise ReplicaSetConnectionError, message end end diff --git a/test/replica_sets/refresh_test.rb b/test/replica_sets/refresh_test.rb index 7368574..ecad881 100644 --- a/test/replica_sets/refresh_test.rb +++ b/test/replica_sets/refresh_test.rb @@ -20,12 +20,12 @@ class ReplicaSetRefreshTest < Test::Unit::TestCase x.report("Connect") do 10.times do ReplSetConnection.new([RS.host, RS.ports[0]], [RS.host, RS.ports[1]], - [RS.host, RS.ports[2]], :auto_refresh => false) + [RS.host, RS.ports[2]], :background_refresh => false) end end @con = ReplSetConnection.new([RS.host, RS.ports[0]], [RS.host, RS.ports[1]], - [RS.host, RS.ports[2]], :auto_refresh => false) + [RS.host, RS.ports[2]], :background_refresh => false) x.report("manager") do man = Mongo::PoolManager.new(@con, @con.seeds) @@ -41,7 +41,7 @@ class ReplicaSetRefreshTest < Test::Unit::TestCase rescue_connection_failure do @conn = ReplSetConnection.new([RS.host, RS.ports[0]], [RS.host, RS.ports[1]], - [RS.host, RS.ports[2]], :auto_refresh => false) + [RS.host, RS.ports[2]], :background_refresh => false) end assert_equal [], @conn.secondaries @@ -70,7 +70,7 @@ class ReplicaSetRefreshTest < Test::Unit::TestCase rescue_connection_failure do @conn = ReplSetConnection.new([RS.host, RS.ports[0]], [RS.host, RS.ports[1]], - [RS.host, RS.ports[2]], :refresh_interval => 2, :auto_refresh => true) + [RS.host, RS.ports[2]], :refresh_interval => 2, :background_refresh => true) end assert_equal [], @conn.secondaries @@ -87,7 +87,7 @@ class ReplicaSetRefreshTest < Test::Unit::TestCase def test_automated_refresh_with_removed_node @conn = ReplSetConnection.new([RS.host, RS.ports[0]], [RS.host, RS.ports[1]], - [RS.host, RS.ports[2]], :refresh_interval => 2, :auto_refresh => true) + [RS.host, RS.ports[2]], :refresh_interval => 2, :background_refresh => true) assert_equal 2, @conn.secondary_pools.length assert_equal 2, @conn.secondaries.length @@ -103,13 +103,13 @@ class ReplicaSetRefreshTest < Test::Unit::TestCase def test_adding_and_removing_nodes @conn = ReplSetConnection.new([RS.host, RS.ports[0]], [RS.host, RS.ports[1]], - [RS.host, RS.ports[2]], :refresh_interval => 2, :auto_refresh => true) + [RS.host, RS.ports[2]], :refresh_interval => 2, :background_refresh => true) RS.add_node sleep(5) @conn2 = ReplSetConnection.new([RS.host, RS.ports[0]], [RS.host, RS.ports[1]], - [RS.host, RS.ports[2]], :refresh_interval => 2, :auto_refresh => true) + [RS.host, RS.ports[2]], :refresh_interval => 2, :background_refresh => true) assert @conn2.secondaries == @conn.secondaries assert_equal 3, @conn.secondary_pools.length diff --git a/test/unit/pool_manager_test.rb b/test/unit/pool_manager_test.rb index fd08ac7..def8cb1 100644 --- a/test/unit/pool_manager_test.rb +++ b/test/unit/pool_manager_test.rb @@ -5,7 +5,7 @@ class PoolManagerTest < Test::Unit::TestCase context "Initialization: " do - def setup + should "populate pools correctly" do TCPSocket.stubs(:new).returns(new_mock_socket) @db = new_mock_db @@ -14,9 +14,7 @@ class PoolManagerTest < Test::Unit::TestCase @connection.stubs(:pool_size).returns(2) @connection.stubs(:socket_class).returns(TCPSocket) @connection.stubs(:[]).returns(@db) - end - should "populate pools correctly" do @connection.stubs(:replica_set_name).returns(nil) @connection.stubs(:log) @arbiters = ['localhost:27020']