NODE-2474: update equal server description (#2260)

Ensure the server description is updated even when equal to the previous one, to keep client-tracked attributes fresh. NODE-2474
mongodb · Mar 11, 2020 · 0c36a32 · 0c36a32
1 parent 6b3b64d
commit 0c36a32
Show file tree

Hide file tree

Showing 4 changed files with 267 additions and 21 deletions.
diff --git a/lib/core/sdam/topology.js b/lib/core/sdam/topology.js
@@ -519,10 +519,10 @@ class Topology extends EventEmitter {
     }
 
     // If we already know all the information contained in this updated description, then
-    // we don't need to update anything or emit SDAM events
-    if (previousServerDescription && previousServerDescription.equals(serverDescription)) {
-      return;
-    }
+    // we don't need to emit SDAM events, but still need to update the description, in order
+    // to keep client-tracked attributes like last update time and round trip time up to date
+    const equalDescriptions =
+      previousServerDescription && previousServerDescription.equals(serverDescription);
 
     // first update the TopologyDescription
     this.s.description = this.s.description.update(serverDescription);
@@ -532,15 +532,17 @@ class Topology extends EventEmitter {
     }
 
     // emit monitoring events for this change
-    this.emit(
-      'serverDescriptionChanged',
-      new events.ServerDescriptionChangedEvent(
-        this.s.id,
-        serverDescription.address,
-        previousServerDescription,
-        this.s.description.servers.get(serverDescription.address)
-      )
-    );
+    if (!equalDescriptions) {
+      this.emit(
+        'serverDescriptionChanged',
+        new events.ServerDescriptionChangedEvent(
+          this.s.id,
+          serverDescription.address,
+          previousServerDescription,
+          this.s.description.servers.get(serverDescription.address)
+        )
+      );
+    }
 
     // update server list from updated descriptions
     updateServers(this, serverDescription);
@@ -550,14 +552,16 @@ class Topology extends EventEmitter {
       processWaitQueue(this);
     }
 
-    this.emit(
-      'topologyDescriptionChanged',
-      new events.TopologyDescriptionChangedEvent(
-        this.s.id,
-        previousTopologyDescription,
-        this.s.description
-      )
-    );
+    if (!equalDescriptions) {
+      this.emit(
+        'topologyDescriptionChanged',
+        new events.TopologyDescriptionChangedEvent(
+          this.s.id,
+          previousTopologyDescription,
+          this.s.description
+        )
+      );
+    }
   }
 
   auth(credentials, callback) {

diff --git a/test/functional/core/replset_state.test.js b/test/functional/core/replset_state.test.js
@@ -12,6 +12,7 @@ describe('ReplicaSet state', function() {
 
   fs.readdirSync(path)
     .filter(x => x.indexOf('.json') !== -1)
+    .filter(x => !x.includes('repeated'))
     .forEach(x => {
       var testData = require(f('%s/%s', path, x));
 

diff --git a/test/spec/server-discovery-and-monitoring/rs/repeated.json b/test/spec/server-discovery-and-monitoring/rs/repeated.json
@@ -0,0 +1,140 @@
+{
+  "description": "Repeated ismaster response must be processed",
+  "uri": "mongodb://a,b/?replicaSet=rs",
+  "phases": [
+    {
+      "responses": [
+        [
+          "a:27017",
+          {
+            "ok": 1,
+            "ismaster": false,
+            "secondary": true,
+            "hidden": true,
+            "hosts": [
+              "a:27017",
+              "c:27017"
+            ],
+            "setName": "rs",
+            "minWireVersion": 0,
+            "maxWireVersion": 6
+          }
+        ]
+      ],
+      "outcome": {
+        "servers": {
+          "a:27017": {
+            "type": "RSOther",
+            "setName": "rs"
+          },
+          "b:27017": {
+            "type": "Unknown"
+          },
+          "c:27017": {
+            "type": "Unknown"
+          }
+        },
+        "topologyType": "ReplicaSetNoPrimary",
+        "logicalSessionTimeoutMinutes": null,
+        "setName": "rs"
+      }
+    },
+    {
+      "responses": [
+        [
+          "c:27017",
+          {
+            "ok": 1,
+            "ismaster": true,
+            "minWireVersion": 0,
+            "maxWireVersion": 6
+          }
+        ]
+      ],
+      "outcome": {
+        "servers": {
+          "a:27017": {
+            "type": "RSOther",
+            "setName": "rs"
+          },
+          "b:27017": {
+            "type": "Unknown"
+          }
+        },
+        "topologyType": "ReplicaSetNoPrimary",
+        "logicalSessionTimeoutMinutes": null,
+        "setName": "rs"
+      }
+    },
+    {
+      "responses": [
+        [
+          "a:27017",
+          {
+            "ok": 1,
+            "ismaster": false,
+            "secondary": true,
+            "hidden": true,
+            "hosts": [
+              "a:27017",
+              "c:27017"
+            ],
+            "setName": "rs",
+            "minWireVersion": 0,
+            "maxWireVersion": 6
+          }
+        ]
+      ],
+      "outcome": {
+        "servers": {
+          "a:27017": {
+            "type": "RSOther",
+            "setName": "rs"
+          },
+          "b:27017": {
+            "type": "Unknown"
+          },
+          "c:27017": {
+            "type": "Unknown"
+          }
+        },
+        "topologyType": "ReplicaSetNoPrimary",
+        "logicalSessionTimeoutMinutes": null,
+        "setName": "rs"
+      }
+    },
+    {
+      "responses": [
+        [
+          "c:27017",
+          {
+            "ok": 1,
+            "ismaster": true,
+            "hosts": [
+              "a:27017",
+              "c:27017"
+            ],
+            "setName": "rs",
+            "minWireVersion": 0,
+            "maxWireVersion": 6
+          }
+        ]
+      ],
+      "outcome": {
+        "servers": {
+          "a:27017": {
+            "type": "RSOther",
+            "setName": "rs"
+          },
+          "c:27017": {
+            "type": "RSPrimary",
+            "setName": "rs"
+          }
+        },
+        "topologyType": "ReplicaSetWithPrimary",
+        "logicalSessionTimeoutMinutes": null,
+        "setName": "rs"
+      }
+    }
+  ]
+}
diff --git a/test/spec/server-discovery-and-monitoring/rs/repeated.yml b/test/spec/server-discovery-and-monitoring/rs/repeated.yml
@@ -0,0 +1,101 @@
+description: Repeated ismaster response must be processed
+
+uri: "mongodb://a,b/?replicaSet=rs"
+
+phases:
+  # Phase 1 - a says it's not primary and suggests c may be the primary
+  - responses:
+    -
+      - "a:27017"
+      - ok: 1
+        ismaster: false
+        secondary: true
+        hidden: true
+        hosts: ["a:27017", "c:27017"]
+        setName: "rs"
+        minWireVersion: 0
+        maxWireVersion: 6
+    outcome:
+      servers:
+        "a:27017":
+          type: "RSOther"
+          setName: "rs"
+
+        "b:27017":
+          type: Unknown
+
+        "c:27017":
+          type: Unknown
+      topologyType: "ReplicaSetNoPrimary"
+      logicalSessionTimeoutMinutes: ~
+      setName: "rs"
+
+  # Phase 2 - c says it's a standalone, is removed
+  - responses:
+    -
+      - "c:27017"
+      - ok: 1
+        ismaster: true
+        minWireVersion: 0
+        maxWireVersion: 6
+    outcome:
+      servers:
+        "a:27017":
+          type: "RSOther"
+          setName: "rs"
+
+        "b:27017":
+          type: Unknown
+      topologyType: "ReplicaSetNoPrimary"
+      logicalSessionTimeoutMinutes: ~
+      setName: "rs"
+
+  # Phase 3 - response from a is repeated, and must be processed; c added again
+  - responses:
+    -
+      - "a:27017"
+      - ok: 1
+        ismaster: false
+        secondary: true
+        hidden: true
+        hosts: ["a:27017", "c:27017"]
+        setName: "rs"
+        minWireVersion: 0
+        maxWireVersion: 6
+    outcome:
+      servers:
+        "a:27017":
+          type: "RSOther"
+          setName: "rs"
+
+        "b:27017":
+          type: Unknown
+
+        "c:27017":
+          type: Unknown
+      topologyType: "ReplicaSetNoPrimary"
+      logicalSessionTimeoutMinutes: ~
+      setName: "rs"
+
+  # Phase 4 - c is now a primary
+  - responses:
+    -
+      - "c:27017"
+      - ok: 1
+        ismaster: true
+        hosts: ["a:27017", "c:27017"]
+        setName: "rs"
+        minWireVersion: 0
+        maxWireVersion: 6
+    outcome:
+      servers:
+        "a:27017":
+          type: "RSOther"
+          setName: "rs"
+
+        "c:27017":
+          type: RSPrimary
+          setName: rs
+      topologyType: "ReplicaSetWithPrimary"
+      logicalSessionTimeoutMinutes: ~
+      setName: "rs"