Skip to content

Commit ca51fec

Browse files
nbbeekennbbeeken
authored
fix(NODE-3712,NODE-4546): electionId should be ordered before setVersion (#3174)
1 parent f6b56a1 commit ca51fec

25 files changed

+1890
-112
lines changed

‎src/sdam/topology_description.ts

+46-21
Original file line numberDiff line numberDiff line change
@@ -373,31 +373,56 @@ function updateRsFromPrimary(
373373
return [checkHasPrimary(serverDescriptions), setName, maxSetVersion, maxElectionId];
374374
}
375375

376-
const electionId = serverDescription.electionId ? serverDescription.electionId : null;
377-
if (serverDescription.setVersion && electionId) {
378-
if (maxSetVersion && maxElectionId) {
379-
if (
380-
maxSetVersion > serverDescription.setVersion ||
381-
compareObjectId(maxElectionId, electionId) > 0
382-
) {
383-
// this primary is stale, we must remove it
384-
serverDescriptions.set(
385-
serverDescription.address,
386-
new ServerDescription(serverDescription.address)
387-
);
376+
if (serverDescription.maxWireVersion >= 17) {
377+
const electionIdComparison = compareObjectId(maxElectionId, serverDescription.electionId);
378+
const maxElectionIdIsEqual = electionIdComparison === 0;
379+
const maxElectionIdIsLess = electionIdComparison === -1;
380+
const maxSetVersionIsLessOrEqual =
381+
(maxSetVersion ?? -1) <= (serverDescription.setVersion ?? -1);
382+
383+
if (maxElectionIdIsLess || (maxElectionIdIsEqual && maxSetVersionIsLessOrEqual)) {
384+
// The reported electionId was greater
385+
// or the electionId was equal and reported setVersion was greater
386+
// Always update both values, they are a tuple
387+
maxElectionId = serverDescription.electionId;
388+
maxSetVersion = serverDescription.setVersion;
389+
} else {
390+
// Stale primary
391+
// replace serverDescription with a default ServerDescription of type "Unknown"
392+
serverDescriptions.set(
393+
serverDescription.address,
394+
new ServerDescription(serverDescription.address)
395+
);
388396

389-
return [checkHasPrimary(serverDescriptions), setName, maxSetVersion, maxElectionId];
390-
}
397+
return [checkHasPrimary(serverDescriptions), setName, maxSetVersion, maxElectionId];
391398
}
399+
} else {
400+
const electionId = serverDescription.electionId ? serverDescription.electionId : null;
401+
if (serverDescription.setVersion && electionId) {
402+
if (maxSetVersion && maxElectionId) {
403+
if (
404+
maxSetVersion > serverDescription.setVersion ||
405+
compareObjectId(maxElectionId, electionId) > 0
406+
) {
407+
// this primary is stale, we must remove it
408+
serverDescriptions.set(
409+
serverDescription.address,
410+
new ServerDescription(serverDescription.address)
411+
);
412+
413+
return [checkHasPrimary(serverDescriptions), setName, maxSetVersion, maxElectionId];
414+
}
415+
}
392416

393-
maxElectionId = serverDescription.electionId;
394-
}
417+
maxElectionId = serverDescription.electionId;
418+
}
395419

396-
if (
397-
serverDescription.setVersion != null &&
398-
(maxSetVersion == null || serverDescription.setVersion > maxSetVersion)
399-
) {
400-
maxSetVersion = serverDescription.setVersion;
420+
if (
421+
serverDescription.setVersion != null &&
422+
(maxSetVersion == null || serverDescription.setVersion > maxSetVersion)
423+
) {
424+
maxSetVersion = serverDescription.setVersion;
425+
}
401426
}
402427

403428
// We've heard from the primary. Is it the same primary as before?

‎src/utils.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1254,7 +1254,7 @@ export function getMongoDBClientEncryption(): {
12541254
* - `-1 = oid1 is less than oid2`
12551255
* - `+0 = oid1 is equal oid2`
12561256
*/
1257-
export function compareObjectId(oid1?: ObjectId, oid2?: ObjectId): 0 | 1 | -1 {
1257+
export function compareObjectId(oid1?: ObjectId | null, oid2?: ObjectId | null): 0 | 1 | -1 {
12581258
if (oid1 == null && oid2 == null) {
12591259
return 0;
12601260
}

‎test/spec/server-discovery-and-monitoring/rs/electionId_precedence_setVersion.json

+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
{
2+
"description": "ElectionId is considered higher precedence than setVersion",
3+
"uri": "mongodb://a/?replicaSet=rs",
4+
"phases": [
5+
{
6+
"responses": [
7+
[
8+
"a:27017",
9+
{
10+
"ok": 1,
11+
"helloOk": true,
12+
"isWritablePrimary": true,
13+
"hosts": [
14+
"a:27017",
15+
"b:27017"
16+
],
17+
"setName": "rs",
18+
"setVersion": 1,
19+
"electionId": {
20+
"$oid": "000000000000000000000001"
21+
},
22+
"minWireVersion": 0,
23+
"maxWireVersion": 17
24+
}
25+
],
26+
[
27+
"b:27017",
28+
{
29+
"ok": 1,
30+
"helloOk": true,
31+
"isWritablePrimary": true,
32+
"hosts": [
33+
"a:27017",
34+
"b:27017"
35+
],
36+
"setName": "rs",
37+
"setVersion": 2,
38+
"electionId": {
39+
"$oid": "000000000000000000000001"
40+
},
41+
"minWireVersion": 0,
42+
"maxWireVersion": 17
43+
}
44+
],
45+
[
46+
"a:27017",
47+
{
48+
"ok": 1,
49+
"helloOk": true,
50+
"isWritablePrimary": true,
51+
"hosts": [
52+
"a:27017",
53+
"b:27017"
54+
],
55+
"setName": "rs",
56+
"setVersion": 1,
57+
"electionId": {
58+
"$oid": "000000000000000000000002"
59+
},
60+
"minWireVersion": 0,
61+
"maxWireVersion": 17
62+
}
63+
]
64+
],
65+
"outcome": {
66+
"servers": {
67+
"a:27017": {
68+
"type": "RSPrimary",
69+
"setName": "rs",
70+
"setVersion": 1,
71+
"electionId": {
72+
"$oid": "000000000000000000000002"
73+
}
74+
},
75+
"b:27017": {
76+
"type": "Unknown",
77+
"setName": null,
78+
"setVersion": null,
79+
"electionId": null
80+
}
81+
},
82+
"topologyType": "ReplicaSetWithPrimary",
83+
"logicalSessionTimeoutMinutes": null,
84+
"setName": "rs",
85+
"maxSetVersion": 1,
86+
"maxElectionId": {
87+
"$oid": "000000000000000000000002"
88+
}
89+
}
90+
}
91+
]
92+
}

‎test/spec/server-discovery-and-monitoring/rs/electionId_precedence_setVersion.yml

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
description: ElectionId is considered higher precedence than setVersion
2+
uri: "mongodb://a/?replicaSet=rs"
3+
phases:
4+
- responses:
5+
- - "a:27017"
6+
- ok: 1
7+
helloOk: true
8+
isWritablePrimary: true
9+
hosts:
10+
- "a:27017"
11+
- "b:27017"
12+
setName: rs
13+
setVersion: 1
14+
electionId:
15+
$oid: "000000000000000000000001"
16+
minWireVersion: 0
17+
maxWireVersion: 17
18+
- - "b:27017"
19+
- ok: 1
20+
helloOk: true
21+
isWritablePrimary: true
22+
hosts:
23+
- "a:27017"
24+
- "b:27017"
25+
setName: rs
26+
setVersion: 2 # Even though "B" reports the newer setVersion, "A" will report the newer electionId which should allow it to remain the primary
27+
electionId:
28+
$oid: "000000000000000000000001"
29+
minWireVersion: 0
30+
maxWireVersion: 17
31+
- - "a:27017"
32+
- ok: 1
33+
helloOk: true
34+
isWritablePrimary: true
35+
hosts:
36+
- "a:27017"
37+
- "b:27017"
38+
setName: rs
39+
setVersion: 1
40+
electionId:
41+
$oid: "000000000000000000000002"
42+
minWireVersion: 0
43+
maxWireVersion: 17
44+
outcome:
45+
servers:
46+
"a:27017":
47+
type: RSPrimary
48+
setName: rs
49+
setVersion: 1
50+
electionId:
51+
$oid: "000000000000000000000002"
52+
"b:27017":
53+
type: Unknown
54+
setName: null
55+
setVersion: null
56+
electionId: null
57+
topologyType: ReplicaSetWithPrimary
58+
logicalSessionTimeoutMinutes: null
59+
setName: rs
60+
maxSetVersion: 1
61+
maxElectionId:
62+
$oid: "000000000000000000000002"

0 commit comments

Comments
 (0)