Fix RUBY-1911 Average round trip time may become nil during server selection (#1517)

Author: p-mongo

lib/mongo/server_selector/selectable.rb

@@ -193,6 +193,12 @@ def select_server(cluster, ping = nil, session = nil)
           servers = candidates(cluster)
           if Lint.enabled?
             servers.each do |server|
+              # It is possible for a server to have a nil average RTT here
+              # because the ARTT comes from description which may be updated
+              # by a background thread while server selection is running.
+              # Currently lint mode is not a public feature, if/when this
+              # changes (https://jira.mongodb.org/browse/RUBY-1576) the
+              # requirement for ARTT to be not nil would need to be removed.
               if server.average_round_trip_time.nil?
                 raise Error::LintError, "Server #{server.address} has nil average rtt"
               end
@@ -361,11 +367,34 @@ def secondaries(candidates)
       # @since 2.0.0
       def near_servers(candidates = [], local_threshold = nil)
         return candidates if candidates.empty?
-        nearest_server = candidates.min_by(&:average_round_trip_time)
+
+        # Average RTT on any server may change at any time by the server
+        # monitor's background thread. ARTT may also become nil if the
+        # server is marked unknown. Take a snapshot of ARTTs for the duration
+        # of this method.
+
+        candidates = candidates.map do |server|
+          {server: server, artt: server.average_round_trip_time}
+        end.reject do |candidate|
+          candidate[:artt].nil?
+        end
+
+        return candidates if candidates.empty?
+
+        nearest_candidate = candidates.min_by do |candidate|
+          candidate[:artt]
+        end
+
         # Default for legacy signarure
         local_threshold ||= self.local_threshold
-        threshold = nearest_server.average_round_trip_time + local_threshold
-        candidates.select { |server| server.average_round_trip_time <= threshold }.shuffle!
+
+        threshold = nearest_candidate[:artt] + local_threshold
+
+        candidates.select do |candidate|
+          candidate[:artt] <= threshold
+        end.map do |candidate|
+          candidate[:server]
+        end.shuffle!
       end
 
       # Select the servers matching the defined tag sets.

spec/mongo/server_selector_spec.rb

@@ -564,4 +564,49 @@ def make_server_with_staleness(last_write_date)
       it_behaves_like 'staleness filter'
     end
   end
+
+  describe '#candidates' do
+    let(:selector) { Mongo::ServerSelector::Primary.new(options) }
+
+    let(:cluster) { double('cluster') }
+
+    let(:options) { {} }
+
+    context 'sharded' do
+      let(:servers) do
+        [make_server(:mongos)]
+      end
+
+      before do
+        allow(cluster).to receive(:single?).and_return(false)
+        allow(cluster).to receive(:sharded?).and_return(true)
+        allow(cluster).to receive(:options).and_return({})
+        allow(cluster).to receive(:servers).and_return(servers)
+      end
+
+      it 'returns the servers' do
+        expect(selector.candidates(cluster)).to eq(servers)
+      end
+
+      context 'with local threshold' do
+        let(:options) do
+          {local_threshold: 1}
+        end
+
+        it 'returns the servers' do
+          expect(selector.candidates(cluster)).to eq(servers)
+        end
+
+        context 'when servers become unknown' do
+          let(:servers) do
+            [make_server(:unknown)]
+          end
+
+          it 'returns an empty list' do
+            expect(selector.candidates(cluster)).to eq([])
+          end
+        end
+      end
+    end
+  end
 end

spec/support/common_shortcuts.rb

@@ -74,7 +74,11 @@ def wait_for_all_servers(cluster)
 
     def make_server(mode, options = {})
       tags = options[:tags] || {}
-      average_round_trip_time = options[:average_round_trip_time] || 0
+      average_round_trip_time = if mode == :unknown
+        nil
+      else
+        options[:average_round_trip_time] || 0
+      end
 
       if mode == :unknown
         ismaster = {}