Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(NODE-3712,NODE-4546): electionId should be ordered before setVersion #3174

Merged
merged 7 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 46 additions & 21 deletions src/sdam/topology_description.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
2 changes: 1 addition & 1 deletion src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1300,7 +1300,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 {
baileympearson marked this conversation as resolved.
Show resolved Hide resolved
if (oid1 == null && oid2 == null) {
return 0;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
]
}
Original file line number Diff line number Diff line change
@@ -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"
Loading