fix(NODE-3712,NODE-4546): electionId should be ordered before setVers…

…ion (mongodb#3174)
ZLY201 · Nov 5, 2022 · b9a22f1 · b9a22f1
1 parent 6f7b942
commit b9a22f1
Show file tree

Hide file tree

Showing 25 changed files with 1,890 additions and 112 deletions.
diff --git a/src/sdam/topology_description.ts b/src/sdam/topology_description.ts
@@ -373,31 +373,56 @@ function updateRsFromPrimary(
     return [checkHasPrimary(serverDescriptions), setName, maxSetVersion, maxElectionId];
   }
 
-  const electionId = serverDescription.electionId ? serverDescription.electionId : null;
-  if (serverDescription.setVersion && electionId) {
-    if (maxSetVersion && maxElectionId) {
-      if (
-        maxSetVersion > serverDescription.setVersion ||
-        compareObjectId(maxElectionId, electionId) > 0
-      ) {
-        // this primary is stale, we must remove it
-        serverDescriptions.set(
-          serverDescription.address,
-          new ServerDescription(serverDescription.address)
-        );
+  if (serverDescription.maxWireVersion >= 17) {
+    const electionIdComparison = compareObjectId(maxElectionId, serverDescription.electionId);
+    const maxElectionIdIsEqual = electionIdComparison === 0;
+    const maxElectionIdIsLess = electionIdComparison === -1;
+    const maxSetVersionIsLessOrEqual =
+      (maxSetVersion ?? -1) <= (serverDescription.setVersion ?? -1);
+
+    if (maxElectionIdIsLess || (maxElectionIdIsEqual && maxSetVersionIsLessOrEqual)) {
+      // The reported electionId was greater
+      // or the electionId was equal and reported setVersion was greater
+      // Always update both values, they are a tuple
+      maxElectionId = serverDescription.electionId;
+      maxSetVersion = serverDescription.setVersion;
+    } else {
+      // Stale primary
+      // replace serverDescription with a default ServerDescription of type "Unknown"
+      serverDescriptions.set(
+        serverDescription.address,
+        new ServerDescription(serverDescription.address)
+      );
 
-        return [checkHasPrimary(serverDescriptions), setName, maxSetVersion, maxElectionId];
-      }
+      return [checkHasPrimary(serverDescriptions), setName, maxSetVersion, maxElectionId];
     }
+  } else {
+    const electionId = serverDescription.electionId ? serverDescription.electionId : null;
+    if (serverDescription.setVersion && electionId) {
+      if (maxSetVersion && maxElectionId) {
+        if (
+          maxSetVersion > serverDescription.setVersion ||
+          compareObjectId(maxElectionId, electionId) > 0
+        ) {
+          // this primary is stale, we must remove it
+          serverDescriptions.set(
+            serverDescription.address,
+            new ServerDescription(serverDescription.address)
+          );
+
+          return [checkHasPrimary(serverDescriptions), setName, maxSetVersion, maxElectionId];
+        }
+      }
 
-    maxElectionId = serverDescription.electionId;
-  }
+      maxElectionId = serverDescription.electionId;
+    }
 
-  if (
-    serverDescription.setVersion != null &&
-    (maxSetVersion == null || serverDescription.setVersion > maxSetVersion)
-  ) {
-    maxSetVersion = serverDescription.setVersion;
+    if (
+      serverDescription.setVersion != null &&
+      (maxSetVersion == null || serverDescription.setVersion > maxSetVersion)
+    ) {
+      maxSetVersion = serverDescription.setVersion;
+    }
   }
 
   // We've heard from the primary. Is it the same primary as before?

diff --git a/src/utils.ts b/src/utils.ts
@@ -1254,7 +1254,7 @@ export function getMongoDBClientEncryption(): {
  * - `-1 = oid1 is less than oid2`
  * - `+0 = oid1 is equal oid2`
  */
-export function compareObjectId(oid1?: ObjectId, oid2?: ObjectId): 0 | 1 | -1 {
+export function compareObjectId(oid1?: ObjectId | null, oid2?: ObjectId | null): 0 | 1 | -1 {
   if (oid1 == null && oid2 == null) {
     return 0;
   }

diff --git a/test/spec/server-discovery-and-monitoring/rs/electionId_precedence_setVersion.json b/test/spec/server-discovery-and-monitoring/rs/electionId_precedence_setVersion.json
@@ -0,0 +1,92 @@
+{
+  "description": "ElectionId is considered higher precedence than setVersion",
+  "uri": "mongodb://a/?replicaSet=rs",
+  "phases": [
+    {
+      "responses": [
+        [
+          "a:27017",
+          {
+            "ok": 1,
+            "helloOk": true,
+            "isWritablePrimary": true,
+            "hosts": [
+              "a:27017",
+              "b:27017"
+            ],
+            "setName": "rs",
+            "setVersion": 1,
+            "electionId": {
+              "$oid": "000000000000000000000001"
+            },
+            "minWireVersion": 0,
+            "maxWireVersion": 17
+          }
+        ],
+        [
+          "b:27017",
+          {
+            "ok": 1,
+            "helloOk": true,
+            "isWritablePrimary": true,
+            "hosts": [
+              "a:27017",
+              "b:27017"
+            ],
+            "setName": "rs",
+            "setVersion": 2,
+            "electionId": {
+              "$oid": "000000000000000000000001"
+            },
+            "minWireVersion": 0,
+            "maxWireVersion": 17
+          }
+        ],
+        [
+          "a:27017",
+          {
+            "ok": 1,
+            "helloOk": true,
+            "isWritablePrimary": true,
+            "hosts": [
+              "a:27017",
+              "b:27017"
+            ],
+            "setName": "rs",
+            "setVersion": 1,
+            "electionId": {
+              "$oid": "000000000000000000000002"
+            },
+            "minWireVersion": 0,
+            "maxWireVersion": 17
+          }
+        ]
+      ],
+      "outcome": {
+        "servers": {
+          "a:27017": {
+            "type": "RSPrimary",
+            "setName": "rs",
+            "setVersion": 1,
+            "electionId": {
+              "$oid": "000000000000000000000002"
+            }
+          },
+          "b:27017": {
+            "type": "Unknown",
+            "setName": null,
+            "setVersion": null,
+            "electionId": null
+          }
+        },
+        "topologyType": "ReplicaSetWithPrimary",
+        "logicalSessionTimeoutMinutes": null,
+        "setName": "rs",
+        "maxSetVersion": 1,
+        "maxElectionId": {
+          "$oid": "000000000000000000000002"
+        }
+      }
+    }
+  ]
+}
diff --git a/test/spec/server-discovery-and-monitoring/rs/electionId_precedence_setVersion.yml b/test/spec/server-discovery-and-monitoring/rs/electionId_precedence_setVersion.yml
@@ -0,0 +1,62 @@
+description: ElectionId is considered higher precedence than setVersion
+uri: "mongodb://a/?replicaSet=rs"
+phases:
+  - responses:
+      - - "a:27017"
+        - ok: 1
+          helloOk: true
+          isWritablePrimary: true
+          hosts:
+            - "a:27017"
+            - "b:27017"
+          setName: rs
+          setVersion: 1
+          electionId:
+            $oid: "000000000000000000000001"
+          minWireVersion: 0
+          maxWireVersion: 17
+      - - "b:27017"
+        - ok: 1
+          helloOk: true
+          isWritablePrimary: true
+          hosts:
+            - "a:27017"
+            - "b:27017"
+          setName: rs
+          setVersion: 2 # Even though "B" reports the newer setVersion, "A" will report the newer electionId which should allow it to remain the primary
+          electionId:
+            $oid: "000000000000000000000001"
+          minWireVersion: 0
+          maxWireVersion: 17
+      - - "a:27017"
+        - ok: 1
+          helloOk: true
+          isWritablePrimary: true
+          hosts:
+            - "a:27017"
+            - "b:27017"
+          setName: rs
+          setVersion: 1
+          electionId:
+            $oid: "000000000000000000000002"
+          minWireVersion: 0
+          maxWireVersion: 17
+    outcome:
+      servers:
+        "a:27017":
+          type: RSPrimary
+          setName: rs
+          setVersion: 1
+          electionId:
+            $oid: "000000000000000000000002"
+        "b:27017":
+          type: Unknown
+          setName: null
+          setVersion: null
+          electionId: null
+      topologyType: ReplicaSetWithPrimary
+      logicalSessionTimeoutMinutes: null
+      setName: rs
+      maxSetVersion: 1
+      maxElectionId:
+        $oid: "000000000000000000000002"