Better DNS resolution handling:

When DNS cannot resolve on Node initialization, the Node will now be marked as "down" instead of raising a SocketError. Moped will attempt to re-resolve the DNS on subsequent refreshes on the timer, and continue to mark the node as down if it cannot be resolved. [ close mongoid#71 ]
mattparlane · Sep 16, 2012 · d8aa898 · d8aa898
1 parent 90b465e
commit d8aa898
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -28,6 +28,10 @@
 * \#72 Reauthenticate properly when an `rs.stepDown()` occurs in the middle of
   cursor execution.
 
+* \#71 When DNS cannot resolve on node initialization, the node will be flagged
+  as down instead of raising a `SocketError`. On subsequent refreshes Moped will
+  attempt to resolve the DNS again to determine if the node can be brought up.
+
 ## 1.2.1
 
 ### Resolved Issues

diff --git a/lib/moped/node.rb b/lib/moped/node.rb
@@ -215,17 +215,12 @@ def hash
     def initialize(address, options = {})
       @address = address
       @options = options
-
-      host, port = address.split(":")
-      @ip_address = ::Socket.getaddrinfo(host, nil, ::Socket::AF_INET, ::Socket::SOCK_STREAM).first[3]
-      @port = port.to_i
-      @resolved_address = "#{@ip_address}:#{@port}"
-
       @timeout = 5
       @down_at = nil
       @refreshed_at = nil
       @primary = nil
       @secondary = nil
+      resolve_address
     end
 
     # Insert documents into the database.
@@ -380,25 +375,27 @@ def query(database, collection, selector, options = {})
     #
     # @since 1.0.0
     def refresh
-      info = command("admin", ismaster: 1)
-
-      @refreshed_at = Time.now
-      primary = true if info["ismaster"]
-      secondary = true if info["secondary"]
-
-      peers = []
-      peers.push(info["primary"]) if info["primary"]
-      peers.concat(info["hosts"]) if info["hosts"]
-      peers.concat(info["passives"]) if info["passives"]
-      peers.concat(info["arbiters"]) if info["arbiters"]
-
-      @peers = peers.map { |peer| Node.new(peer) }
-      @primary, @secondary = primary, secondary
-      @arbiter = info["arbiterOnly"]
-      @passive = info["passive"]
-
-      if !primary && Threaded.executing?(:ensure_primary)
-        raise Errors::ReplicaSetReconfigured, "#{inspect} is no longer the primary node."
+      if resolve_address
+        info = command("admin", ismaster: 1)
+
+        @refreshed_at = Time.now
+        primary = true if info["ismaster"]
+        secondary = true if info["secondary"]
+
+        peers = []
+        peers.push(info["primary"]) if info["primary"]
+        peers.concat(info["hosts"]) if info["hosts"]
+        peers.concat(info["passives"]) if info["passives"]
+        peers.concat(info["arbiters"]) if info["arbiters"]
+
+        @peers = peers.map { |peer| Node.new(peer) }
+        @primary, @secondary = primary, secondary
+        @arbiter = info["arbiterOnly"]
+        @passive = info["passive"]
+
+        if !primary && Threaded.executing?(:ensure_primary)
+          raise Errors::ReplicaSetReconfigured, "#{inspect} is no longer the primary node."
+        end
       end
     end
 
@@ -565,5 +562,28 @@ def log_operations(logger, ops, duration_ms)
         logger.debug indent + last.log_inspect + runtime
       end
     end
+
+    def resolve_address
+      unless ip_address
+        begin
+          parse_address and true
+        rescue SocketError
+          if logger = Moped.logger
+            logger.warn " MOPED: Could not resolve IP address for #{address}"
+          end
+          @down_at = Time.new
+          false
+        end
+      else
+        true
+      end
+    end
+
+    def parse_address
+      host, port = address.split(":")
+      @port = port.to_i
+      @ip_address = ::Socket.getaddrinfo(host, nil, ::Socket::AF_INET, ::Socket::SOCK_STREAM).first[3]
+      @resolved_address = "#{@ip_address}:#{@port}"
+    end
   end
 end
diff --git a/spec/moped/node_spec.rb b/spec/moped/node_spec.rb
@@ -94,4 +94,37 @@
       end
     end
   end
+
+  describe "#initialize" do
+
+    let(:node) do
+      described_class.new("iamnota.mongoid.org")
+    end
+
+    context "when dns cannot resolve the address" do
+
+      it "flags the node as being down" do
+        node.should be_down
+      end
+
+      it "sets the down_at time" do
+        node.down_at.should be_within(1).of(Time.now)
+      end
+
+      context "when attempting to refresh the node" do
+
+        before do
+          node.refresh
+        end
+
+        it "keeps the node flagged as down" do
+          node.should be_down
+        end
+
+        it "updates the down_at time" do
+          node.down_at.should be_within(1).of(Time.now)
+        end
+      end
+    end
+  end
 end