fix: filter servers before applying reducers

In order to accurately reduce the set of viable servers for server selection, we need to filter the set by the read preference BEFORE we reduce by max staleness, tagSet and latency window. NODE-2407
mongodb · May 26, 2020 · 99b86b3 · 99b86b3
1 parent f40cffc
commit 99b86b3
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 51 deletions.
diff --git a/lib/sdam/server_selection.js b/lib/sdam/server_selection.js
@@ -47,7 +47,7 @@ function maxStalenessReducer(readPreference, topologyDescription, servers) {
   }
 
   if (topologyDescription.type === TopologyType.ReplicaSetWithPrimary) {
-    const primary = servers.filter(primaryFilter)[0];
+    const primary = Array.from(topologyDescription.servers.values()).filter(primaryFilter)[0];
     return servers.reduce((result, server) => {
       const stalenessMS =
         server.lastUpdateTime -
@@ -196,50 +196,32 @@ function readPreferenceServerSelector(readPreference) {
       return latencyWindowReducer(topologyDescription, servers.filter(knownFilter));
     }
 
-    if (readPreference.mode === ReadPreference.PRIMARY) {
+    const mode = readPreference.mode;
+    if (mode === ReadPreference.PRIMARY) {
       return servers.filter(primaryFilter);
     }
 
-    if (readPreference.mode === ReadPreference.SECONDARY) {
-      return latencyWindowReducer(
-        topologyDescription,
-        tagSetReducer(
-          readPreference,
-          maxStalenessReducer(readPreference, topologyDescription, servers)
-        )
-      ).filter(secondaryFilter);
-    } else if (readPreference.mode === ReadPreference.NEAREST) {
-      return latencyWindowReducer(
-        topologyDescription,
-        tagSetReducer(
-          readPreference,
-          maxStalenessReducer(readPreference, topologyDescription, servers)
-        )
-      ).filter(nearestFilter);
-    } else if (readPreference.mode === ReadPreference.SECONDARY_PREFERRED) {
-      const result = latencyWindowReducer(
-        topologyDescription,
-        tagSetReducer(
-          readPreference,
-          maxStalenessReducer(readPreference, topologyDescription, servers)
-        )
-      ).filter(secondaryFilter);
-
-      return result.length === 0 ? servers.filter(primaryFilter) : result;
-    } else if (readPreference.mode === ReadPreference.PRIMARY_PREFERRED) {
+    if (mode === ReadPreference.PRIMARY_PREFERRED) {
       const result = servers.filter(primaryFilter);
       if (result.length) {
         return result;
       }
+    }
+
+    const filter = mode === ReadPreference.NEAREST ? nearestFilter : secondaryFilter;
+    const selectedServers = latencyWindowReducer(
+      topologyDescription,
+      tagSetReducer(
+        readPreference,
+        maxStalenessReducer(readPreference, topologyDescription, servers.filter(filter))
+      )
+    );
 
-      return latencyWindowReducer(
-        topologyDescription,
-        tagSetReducer(
-          readPreference,
-          maxStalenessReducer(readPreference, topologyDescription, servers)
-        )
-      ).filter(secondaryFilter);
+    if (mode === ReadPreference.SECONDARY_PREFERRED && selectedServers.length === 0) {
+      return servers.filter(primaryFilter);
     }
+
+    return selectedServers;
   };
 }
 

diff --git a/test/spec/max-staleness/Unknown/SmallMaxStaleness.json b/test/spec/max-staleness/Unknown/SmallMaxStaleness.json
@@ -5,7 +5,8 @@
     "servers": [
       {
         "address": "a:27017",
-        "type": "Unknown"
+        "type": "Unknown",
+        "maxWireVersion": 5
       }
     ]
   },

diff --git a/test/spec/max-staleness/Unknown/SmallMaxStaleness.yml b/test/spec/max-staleness/Unknown/SmallMaxStaleness.yml
@@ -7,6 +7,7 @@ topology_description:
   - &1
     address: a:27017
     type: Unknown
+    maxWireVersion: 5
 read_preference:
   mode: Nearest
   maxStalenessSeconds: 1

diff --git a/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Nearest.json b/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Nearest.json
@@ -4,7 +4,7 @@
     "servers": [
       {
         "address": "b:27017",
-        "avg_rtt_ms": 5,
+        "avg_rtt_ms": 21,
         "type": "RSSecondary",
         "tags": {
           "data_center": "nyc"
@@ -20,11 +20,19 @@
       },
       {
         "address": "a:27017",
-        "avg_rtt_ms": 26,
+        "avg_rtt_ms": 37,
         "type": "RSPrimary",
         "tags": {
           "data_center": "nyc"
         }
+      },
+      {
+        "address": "d:27017",
+        "avg_rtt_ms": 5,
+        "type": "RSOther",
+        "tags": {
+          "data_center": "nyc"
+        }
       }
     ]
   },
@@ -40,15 +48,15 @@
   "suitable_servers": [
     {
       "address": "b:27017",
-      "avg_rtt_ms": 5,
+      "avg_rtt_ms": 21,
       "type": "RSSecondary",
       "tags": {
         "data_center": "nyc"
       }
     },
     {
       "address": "a:27017",
-      "avg_rtt_ms": 26,
+      "avg_rtt_ms": 37,
       "type": "RSPrimary",
       "tags": {
         "data_center": "nyc"
@@ -66,7 +74,7 @@
   "in_latency_window": [
     {
       "address": "b:27017",
-      "avg_rtt_ms": 5,
+      "avg_rtt_ms": 21,
       "type": "RSSecondary",
       "tags": {
         "data_center": "nyc"

diff --git a/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Nearest.yml b/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Nearest.yml
@@ -3,7 +3,7 @@ topology_description:
   servers:
   - &1
     address: b:27017
-    avg_rtt_ms: 5
+    avg_rtt_ms: 21
     type: RSSecondary
     tags:
       data_center: nyc
@@ -15,10 +15,15 @@ topology_description:
       data_center: nyc
   - &2
     address: a:27017
-    avg_rtt_ms: 26
+    avg_rtt_ms: 37
     type: RSPrimary
     tags:
       data_center: nyc
+  - address: d:27017
+    avg_rtt_ms: 5
+    type: RSOther
+    tags:
+      data_center: nyc
 operation: read
 read_preference:
   mode: Nearest

diff --git a/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Secondary.json b/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Secondary.json
@@ -4,7 +4,7 @@
     "servers": [
       {
         "address": "b:27017",
-        "avg_rtt_ms": 5,
+        "avg_rtt_ms": 21,
         "type": "RSSecondary",
         "tags": {
           "data_center": "nyc"
@@ -20,7 +20,7 @@
       },
       {
         "address": "a:27017",
-        "avg_rtt_ms": 26,
+        "avg_rtt_ms": 5,
         "type": "RSPrimary",
         "tags": {
           "data_center": "nyc"
@@ -40,7 +40,7 @@
   "suitable_servers": [
     {
       "address": "b:27017",
-      "avg_rtt_ms": 5,
+      "avg_rtt_ms": 21,
       "type": "RSSecondary",
       "tags": {
         "data_center": "nyc"
@@ -58,7 +58,7 @@
   "in_latency_window": [
     {
       "address": "b:27017",
-      "avg_rtt_ms": 5,
+      "avg_rtt_ms": 21,
       "type": "RSSecondary",
       "tags": {
         "data_center": "nyc"

diff --git a/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Secondary.yml b/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Secondary.yml
@@ -3,18 +3,18 @@ topology_description:
   servers:
   - &1
     address: b:27017
-    avg_rtt_ms: 5
+    avg_rtt_ms: 21 # outside the latency window if primary is considered
     type: RSSecondary
     tags:
       data_center: nyc
   - &2
     address: c:27017
-    avg_rtt_ms: 100
+    avg_rtt_ms: 100 # outside the latency window if both secondaries are considered
     type: RSSecondary
     tags:
       data_center: nyc
   - address: a:27017
-    avg_rtt_ms: 26
+    avg_rtt_ms: 5
     type: RSPrimary
     tags:
       data_center: nyc