From 2f645cdf2705a5d62069e01f5c95bd640889b33e Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 18 Jan 2022 17:41:06 -0500 Subject: [PATCH 01/14] DRIVERS-1954: SDAM should give priority to electionId over setVersion when updating topology --- .../server-discovery-and-monitoring.rst | 72 +++++++++------ .../tests/rs/electionId_setVersion.json | 92 +++++++++++++++++++ .../tests/rs/electionId_setVersion.yml | 62 +++++++++++++ .../rs/use_setversion_without_electionid.json | 12 +-- .../rs/use_setversion_without_electionid.yml | 12 +-- 5 files changed, 211 insertions(+), 39 deletions(-) create mode 100644 source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.json create mode 100644 source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.yml diff --git a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst index d81a1a4aab..59d033f872 100644 --- a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst +++ b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst @@ -293,10 +293,10 @@ Fields: * type: a `TopologyType`_ enum value. See `initial TopologyType`_. * setName: the replica set name. Default null. -* maxSetVersion: an integer or null. The largest setVersion ever reported by - a primary. Default null. * maxElectionId: an ObjectId or null. The largest electionId ever reported by a primary. Default null. +* maxSetVersion: an integer or null. The largest setVersion ever reported by + a primary. Default null. * servers: a set of ServerDescription instances. Default contains one server: "localhost:27017", ServerType Unknown. * stale: a boolean for single-threaded clients, whether the topology must @@ -350,10 +350,10 @@ Fields: The client `monitors all three types of servers`_ in a replica set. * (=) tags: map from string to string. Default empty. * (=) setName: string or null. Default null. -* (=) setVersion: integer or null. Default null. * (=) electionId: an ObjectId, if this is a MongoDB 2.6+ replica set member that - believes it is primary. See `using setVersion and electionId to detect stale primaries`_. + believes it is primary. See `using electionId and setVersion to detect stale primaries`_. Default null. +* (=) setVersion: integer or null. Default null. * (=) primary: an address. This server's opinion of who the primary is. Default null. * lastUpdateTime: when this server was last checked. Default "infinity ago". @@ -1101,29 +1101,38 @@ updateRSFromPrimary checkIfHasPrimary() return - if description.setVersion is not null and description.electionId is not null: + if description.electionId is not null and description.setVersion is not null: # Election ids are ObjectIds, see # "using setVersion and electionId to detect stale primaries" # for comparison rules. - if (topologyDescription.maxSetVersion is not null and - topologyDescription.maxElectionId is not null and ( - topologyDescription.maxSetVersion > description.setVersion or ( - topologyDescription.maxSetVersion == description.setVersion and - topologyDescription.maxElectionId > description.electionId + if ( + topologyDescription.maxElectionId is not null + and topologyDescription.maxSetVersion is not null + and ( + # electionId change is checked first to align tuple ordering with RSM + topologyDescription.maxElectionId > description.electionId + or ( + # If there is no difference in electionId, we check ordering based on setVersion + topologyDescription.maxElectionId == description.electionId + and topologyDescription.maxSetVersion > description.setVersion ) - ): - + ) + ): # Stale primary. replace description with a default ServerDescription of type "Unknown" checkIfHasPrimary() return + if description.electionId is not null and ( + topologyDescription.maxElectionId is null + or description.electionId > topologyDescription.maxElectionId + ): topologyDescription.maxElectionId = description.electionId - if (description.setVersion is not null and - (topologyDescription.maxSetVersion is null or - description.setVersion > topologyDescription.maxSetVersion)): - + if description.setVersion is not null and ( + topologyDescription.maxSetVersion is null + or description.setVersion > topologyDescription.maxSetVersion + ): topologyDescription.maxSetVersion = description.setVersion for each server in topologyDescription.servers: @@ -1159,10 +1168,10 @@ updateRSFromPrimary See `replica set monitoring with and without a primary`_. - If the server is primary with an obsolete setVersion or electionId, it is + If the server is primary with an obsolete electionId or setVersion, it is likely a stale primary that is going to step down. Mark it Unknown and let periodic monitoring detect when it becomes secondary. See - `using setVersion and electionId to detect stale primaries`_. + `using electionId and setVersion to detect stale primaries`_. A note on checking "me": Unlike `updateRSWithPrimaryFromMember`, there is no need to remove the server if the address is not equal to "me": since the server address will not be a member of either "hosts", "passives", or "arbiters", the server will already have been @@ -1960,7 +1969,7 @@ list are removed. .. _stale primaries: -Using setVersion and electionId to detect stale primaries +Using electionId and setVersion to detect stale primaries ''''''''''''''''''''''''''''''''''''''''''''''''''''''''' Replica set members running MongoDB 2.6.10+ or 3.0+ include an integer called @@ -1971,13 +1980,17 @@ protocol versions; electionIds from one protocol version must not be compared to electionIds from a different protocol. Because protocol version changes require replica set reconfiguration, -clients use the tuple (setVersion, electionId) to detect stale primaries. +clients use the tuple (electionId, setVersion) to detect stale primaries. +The tuple order comparison MUST be checked in the order of electionId followed +by setVersion since that order of comparison is guaranteed monotonicity. -The client remembers the greatest setVersion and electionId reported by a primary, +The client remembers the greatest electionId and setVersion reported by a primary, and distrusts primaries from older setVersions or from the same setVersion but with lesser electionIds. -It compares setVersions as integer values. -It compares electionIds as 12-byte big-endian integers. + +- It compares electionIds as 12-byte sequence i.e. memory comparison. +- It compares setVersions as integer values. + This prevents the client from oscillating between the old and new primary during a split-brain period, and helps provide read-your-writes consistency with write concern "majority" @@ -2018,15 +2031,18 @@ reads with WriteConcern Majority and ReadPreference Primary." Detecting a stale primary ````````````````````````` -To prevent this scenario, the client uses setVersion and electionId to +To prevent this scenario, the client uses electionId and setVersion to determine which primary was elected last. In this case, it would not consider -A primary, nor read from it, after receiving B's hello or legacy hello response with the -same setVersion and a greater electionId. +"A" a primary, nor read from it because server B will have a greater electionId +but the same setVersion. Monotonicity ```````````` -The electionId is an ObjectId compared bytewise in big-endian order. +The electionId is an ObjectId compared bytewise in order. + +(ie. 000000000000000000000001 > 000000000000000000000000, FF0000000000000000000000 > FE0000000000000000000000 etc.) + In some server versions, it is monotonic with respect to a particular servers' system clock, but is not globally monotonic across a deployment. However, if inter-server clock skews are small, it can be @@ -2434,6 +2450,8 @@ Bernie Hackett gently oversaw the specification process. Changes ------- +2021-01-17: Require clients to compare (electionId, setVersion) tuples. + 2015-12-17: Require clients to compare (setVersion, electionId) tuples. 2015-10-09: Specify electionID comparison method. diff --git a/source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.json b/source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.json new file mode 100644 index 0000000000..3b3cb014ff --- /dev/null +++ b/source/server-discovery-and-monitoring/tests/rs/electionId_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": 6 + } + ], + [ + "b:27017", + { + "ok": 1, + "helloOk": true, + "isWritablePrimary": true, + "hosts": [ + "a:27017", + "b:27017" + ], + "setName": "rs", + "setVersion": 2, + "electionId": { + "$oid": "000000000000000000000001" + }, + "minWireVersion": 0, + "maxWireVersion": 6 + } + ], + [ + "a:27017", + { + "ok": 1, + "helloOk": true, + "isWritablePrimary": true, + "hosts": [ + "a:27017", + "b:27017" + ], + "setName": "rs", + "setVersion": 1, + "electionId": { + "$oid": "000000000000000000000002" + }, + "minWireVersion": 0, + "maxWireVersion": 6 + } + ] + ], + "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": 2, + "maxElectionId": { + "$oid": "000000000000000000000002" + } + } + } + ] +} diff --git a/source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.yml b/source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.yml new file mode 100644 index 0000000000..18c58ac400 --- /dev/null +++ b/source/server-discovery-and-monitoring/tests/rs/electionId_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: 6 + - - "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: 6 + - - "a:27017" + - ok: 1 + helloOk: true + isWritablePrimary: true + hosts: + - "a:27017" + - "b:27017" + setName: rs + setVersion: 1 + electionId: + $oid: "000000000000000000000002" + minWireVersion: 0 + maxWireVersion: 6 + 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: 2 + maxElectionId: + $oid: "000000000000000000000002" diff --git a/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.json b/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.json index 421ff57c8d..d6b1cb4c65 100644 --- a/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.json +++ b/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.json @@ -115,14 +115,14 @@ "outcome": { "servers": { "a:27017": { + "type": "RSPrimary", + "setName": "rs", + "setVersion": 1 + }, + "b:27017": { "type": "Unknown", "setName": null, "electionId": null - }, - "b:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 2 } }, "topologyType": "ReplicaSetWithPrimary", @@ -130,7 +130,7 @@ "setName": "rs", "maxSetVersion": 2, "maxElectionId": { - "$oid": "000000000000000000000001" + "$oid": "000000000000000000000002" } } } diff --git a/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.yml b/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.yml index 300f9d9304..41eb447035 100644 --- a/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.yml +++ b/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.yml @@ -97,21 +97,21 @@ phases: [ outcome: { servers: { "a:27017": { + type: "RSPrimary", + setName: "rs", + setVersion: 1 + }, + "b:27017":{ type: "Unknown", setName: , electionId: - }, - "b:27017": { - type: "RSPrimary", - setName: "rs", - setVersion: 2 } }, topologyType: "ReplicaSetWithPrimary", logicalSessionTimeoutMinutes: null, setName: "rs", maxSetVersion: 2, - maxElectionId: {"$oid": "000000000000000000000001"}, + maxElectionId: {"$oid": "000000000000000000000002"}, } } ] From ce41552cec714a156e0c384a8e36e0d03c1eb2e7 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 4 Feb 2022 11:49:37 -0500 Subject: [PATCH 02/14] update comment, add electionId --- .../tests/rs/use_setversion_without_electionid.json | 5 ++++- .../tests/rs/use_setversion_without_electionid.yml | 11 ++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.json b/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.json index d6b1cb4c65..06b0faca11 100644 --- a/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.json +++ b/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.json @@ -117,7 +117,10 @@ "a:27017": { "type": "RSPrimary", "setName": "rs", - "setVersion": 1 + "setVersion": 1, + "electionId": { + "$oid": "000000000000000000000002" + } }, "b:27017": { "type": "Unknown", diff --git a/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.yml b/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.yml index 41eb447035..815ee96025 100644 --- a/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.yml +++ b/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.yml @@ -4,7 +4,7 @@ uri: "mongodb://a/?replicaSet=rs" phases: [ - # Primary A has setVersion and electionId, tells us about B. + # Primary A has electionId and setVersion, tells us about B. { responses: [ ["a:27017", { @@ -42,7 +42,7 @@ phases: [ } }, - # Reconfig the set and elect B, it has a new setVersion but no electionId. + # Reconfig, B reports as primary, B is missing the electionId but reports setVersion { responses: [ ["b:27017", { @@ -78,8 +78,8 @@ phases: [ } }, - # Delayed response from A, reporting its reelection. Its setVersion shows - # the election preceded B's so we ignore it. + # A reports as primary, A has been reelection (electionId greater than our recorded maxElectionId). + # A's setVersion is less than our maxSetVersion, but electionId takes precedence so B's primary claim is ignored { responses: [ ["a:27017", { @@ -99,7 +99,8 @@ phases: [ "a:27017": { type: "RSPrimary", setName: "rs", - setVersion: 1 + setVersion: 1, + electionId: {"$oid": "000000000000000000000002"} }, "b:27017":{ type: "Unknown", From 6f60d4e771a1e3dff6ac1302e6adc6709f8f5a9b Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 8 Feb 2022 15:37:48 -0500 Subject: [PATCH 03/14] test: add set version can rollback test --- .../tests/rs/set_version_can_rollback.json | 149 ++++++++++++++++++ .../tests/rs/set_version_can_rollback.yml | 101 ++++++++++++ 2 files changed, 250 insertions(+) create mode 100644 source/server-discovery-and-monitoring/tests/rs/set_version_can_rollback.json create mode 100644 source/server-discovery-and-monitoring/tests/rs/set_version_can_rollback.yml diff --git a/source/server-discovery-and-monitoring/tests/rs/set_version_can_rollback.json b/source/server-discovery-and-monitoring/tests/rs/set_version_can_rollback.json new file mode 100644 index 0000000000..28ecbeefca --- /dev/null +++ b/source/server-discovery-and-monitoring/tests/rs/set_version_can_rollback.json @@ -0,0 +1,149 @@ +{ + "description": "Set version rolls back after new primary with higher election Id", + "uri": "mongodb://a/?replicaSet=rs", + "phases": [ + { + "responses": [ + [ + "a:27017", + { + "ok": 1, + "hello": true, + "isWritablePrimary": true, + "hosts": [ + "a:27017", + "b:27017" + ], + "setName": "rs", + "setVersion": 2, + "electionId": { + "$oid": "000000000000000000000001" + }, + "minWireVersion": 0, + "maxWireVersion": 6 + } + ] + ], + "outcome": { + "servers": { + "a:27017": { + "type": "RSPrimary", + "setName": "rs", + "setVersion": 2, + "electionId": { + "$oid": "000000000000000000000001" + } + }, + "b:27017": { + "type": "Unknown", + "setName": null, + "electionId": null + } + }, + "topologyType": "ReplicaSetWithPrimary", + "logicalSessionTimeoutMinutes": null, + "setName": "rs", + "maxSetVersion": 2, + "maxElectionId": { + "$oid": "000000000000000000000001" + } + } + }, + { + "_comment": "Response from new primary with newer election Id", + "responses": [ + [ + "b:27017", + { + "ok": 1, + "hello": true, + "isWritablePrimary": true, + "hosts": [ + "a:27017", + "b:27017" + ], + "setName": "rs", + "setVersion": 1, + "electionId": { + "$oid": "000000000000000000000002" + }, + "minWireVersion": 0, + "maxWireVersion": 6 + } + ] + ], + "outcome": { + "servers": { + "a:27017": { + "type": "Unknown", + "setName": null, + "electionId": null + }, + "b:27017": { + "type": "RSPrimary", + "setName": "rs", + "setVersion": 1, + "electionId": { + "$oid": "000000000000000000000002" + } + } + }, + "topologyType": "ReplicaSetWithPrimary", + "logicalSessionTimeoutMinutes": null, + "setName": "rs", + "maxSetVersion": 1, + "maxElectionId": { + "$oid": "000000000000000000000002" + } + } + }, + { + "_comment": "Response from stale primary", + "responses": [ + [ + "a:27017", + { + "ok": 1, + "hello": true, + "isWritablePrimary": true, + "hosts": [ + "a:27017", + "b:27017" + ], + "setName": "rs", + "setVersion": 2, + "electionId": { + "$oid": "000000000000000000000001" + }, + "minWireVersion": 0, + "maxWireVersion": 6 + } + ] + ], + "outcome": { + "servers": { + "a:27017": { + "type": "Unknown", + "setName": null, + "electionId": null + }, + "b:27017": { + "type": "RSPrimary", + "setName": "rs", + "setVersion": 1, + "electionId": { + "$oid": "000000000000000000000002" + } + } + }, + "topologyType": "ReplicaSetWithPrimary", + "logicalSessionTimeoutMinutes": null, + "setName": "rs", + "maxSetVersion": 1, + "maxElectionId": { + "$oid": "000000000000000000000002" + } + } + } + ] +} diff --git a/source/server-discovery-and-monitoring/tests/rs/set_version_can_rollback.yml b/source/server-discovery-and-monitoring/tests/rs/set_version_can_rollback.yml new file mode 100644 index 0000000000..ef91a074a6 --- /dev/null +++ b/source/server-discovery-and-monitoring/tests/rs/set_version_can_rollback.yml @@ -0,0 +1,101 @@ +description: Set version rolls back after new primary with higher election Id +uri: mongodb://a/?replicaSet=rs +phases: + - responses: + - - a:27017 + - ok: 1 + hello: true + isWritablePrimary: true + hosts: + - a:27017 + - b:27017 + setName: rs + setVersion: 2 + electionId: + $oid: '000000000000000000000001' + minWireVersion: 0 + maxWireVersion: 6 + outcome: + servers: + a:27017: + type: RSPrimary + setName: rs + setVersion: 2 + electionId: + $oid: '000000000000000000000001' + b:27017: + type: Unknown + setName: null + electionId: null + topologyType: ReplicaSetWithPrimary + logicalSessionTimeoutMinutes: null + setName: rs + maxSetVersion: 2 + maxElectionId: + $oid: '000000000000000000000001' + - _comment: Response from new primary with newer election Id + responses: + - - b:27017 + - ok: 1 + hello: true + isWritablePrimary: true + hosts: + - a:27017 + - b:27017 + setName: rs + setVersion: 1 + electionId: + $oid: '000000000000000000000002' + minWireVersion: 0 + maxWireVersion: 6 + outcome: + servers: + a:27017: + type: Unknown + setName: null + electionId: null + b:27017: + type: RSPrimary + setName: rs + setVersion: 1 + electionId: + $oid: '000000000000000000000002' + topologyType: ReplicaSetWithPrimary + logicalSessionTimeoutMinutes: null + setName: rs + maxSetVersion: 1 + maxElectionId: + $oid: '000000000000000000000002' + - _comment: Response from stale primary + responses: + - - a:27017 + - ok: 1 + hello: true + isWritablePrimary: true + hosts: + - a:27017 + - b:27017 + setName: rs + setVersion: 2 + electionId: + $oid: '000000000000000000000001' + minWireVersion: 0 + maxWireVersion: 6 + outcome: + servers: + a:27017: + type: Unknown + setName: null + electionId: null + b:27017: + type: RSPrimary + setName: rs + setVersion: 1 + electionId: + $oid: '000000000000000000000002' + topologyType: ReplicaSetWithPrimary + logicalSessionTimeoutMinutes: null + setName: rs + maxSetVersion: 1 + maxElectionId: + $oid: '000000000000000000000002' From ca93b16baf7b83eb862acf7541498e9613edcbdd Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Thu, 24 Feb 2022 14:59:20 -0500 Subject: [PATCH 04/14] fix: pseudo code for updating tuple --- .../server-discovery-and-monitoring.rst | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst index 59d033f872..5c9356a5e8 100644 --- a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst +++ b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst @@ -294,9 +294,10 @@ Fields: * type: a `TopologyType`_ enum value. See `initial TopologyType`_. * setName: the replica set name. Default null. * maxElectionId: an ObjectId or null. The largest electionId ever reported by - a primary. Default null. + a primary. Default null. Part of the (``electionId``, ``setVersion``) tuple. * maxSetVersion: an integer or null. The largest setVersion ever reported by - a primary. Default null. + a primary. It may not monotonically increase, as electionId takes precedence in ordering + Default null. Part of the (``electionId``, ``setVersion``) tuple. * servers: a set of ServerDescription instances. Default contains one server: "localhost:27017", ServerType Unknown. * stale: a boolean for single-threaded clients, whether the topology must @@ -1123,16 +1124,17 @@ updateRSFromPrimary checkIfHasPrimary() return - if description.electionId is not null and ( - topologyDescription.maxElectionId is null - or description.electionId > topologyDescription.maxElectionId - ): - topologyDescription.maxElectionId = description.electionId + if topologyDescription.maxElectionId is null: + topologyDescription.maxElectionId = description.electionId - if description.setVersion is not null and ( - topologyDescription.maxSetVersion is null - or description.setVersion > topologyDescription.maxSetVersion + if topologyDescription.maxSetVersion is null: + topologyDescription.maxSetVersion = description.setVersion + + if ( + description.electionId is not null and (description.electionId > topologyDescription.maxElectionId)) + or (description.setVersion is not null and (description.setVersion > topologyDescription.maxSetVersion) ): + topologyDescription.maxElectionId = description.electionId topologyDescription.maxSetVersion = description.setVersion for each server in topologyDescription.servers: @@ -2436,6 +2438,17 @@ Why is auto-discovery the preferred default? Auto-discovery is most resilient and is therefore preferred. +Why is it possible for maxSetVersion to go down? +'''''''''''''''''''''''''''''''''''''''''''''''' + +``maxElectionId`` and ``maxSetVersion`` are actually considered a pair of values +Drivers MAY consider implementing comparison in code as a tuple of the two to ensure their always updated together: + +:: + New tuple old tuple + { electionId: 2, setVersion: 1 } > { electionId: 1, setVersion: 50 } + +In this scenario, the maxSetVersion goes from 50 to 1, but the maxElectionId is raised to 2. Acknowledgments --------------- From aeed121409d02d4692c93196735913d1bb8e5f24 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 1 Mar 2022 10:24:17 -0500 Subject: [PATCH 05/14] update pseudo code --- .../server-discovery-and-monitoring.rst | 39 +++++++------------ 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst index 5c9356a5e8..22a65d1da2 100644 --- a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst +++ b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst @@ -1106,36 +1106,23 @@ updateRSFromPrimary # Election ids are ObjectIds, see # "using setVersion and electionId to detect stale primaries" # for comparison rules. - if ( - topologyDescription.maxElectionId is not null - and topologyDescription.maxSetVersion is not null - and ( - # electionId change is checked first to align tuple ordering with RSM - topologyDescription.maxElectionId > description.electionId - or ( - # If there is no difference in electionId, we check ordering based on setVersion - topologyDescription.maxElectionId == description.electionId - and topologyDescription.maxSetVersion > description.setVersion - ) - ) - ): + + # Null values are always considered "less than" + maxElectionIdIsGreater = topologyDescription.maxElectionId < serverDescription.electionId if topologyDescription.maxElectionId is not null else false + maxElectionIdIsEqual = topologyDescription.maxElectionId == serverDescription.electionId if topologyDescription.maxElectionId is not null else false + maxElectionIdIsLess = topologyDescription.maxElectionId > serverDescription.electionId if topologyDescription.maxElectionId is not null else false + maxSetVersionIsGreater = topologyDescription.maxSetVersion > serverDescription.setVersion if topologyDescription.maxSetVersion is not null else false + maxSetVersionIsLess = topologyDescription.maxSetVersion < serverDescription.setVersion if topologyDescription.maxSetVersion is not null else true + + if maxElectionIdIsGreater or (maxElectionIdIsEqual and maxSetVersionIsGreater): # Stale primary. - replace description with a default ServerDescription of type "Unknown" + # replace description with a default ServerDescription of type "Unknown" checkIfHasPrimary() return - if topologyDescription.maxElectionId is null: - topologyDescription.maxElectionId = description.electionId - - if topologyDescription.maxSetVersion is null: - topologyDescription.maxSetVersion = description.setVersion - - if ( - description.electionId is not null and (description.electionId > topologyDescription.maxElectionId)) - or (description.setVersion is not null and (description.setVersion > topologyDescription.maxSetVersion) - ): - topologyDescription.maxElectionId = description.electionId - topologyDescription.maxSetVersion = description.setVersion + if maxElectionIdIsLess or (maxElectionIdIsEqual and maxSetVersionIsLess): + topologyDescription.maxElectionId = serverDescription.electionId; + topologyDescription.maxSetVersion = serverDescription.setVersion; for each server in topologyDescription.servers: if server.address != description.address: From 0b3170a76e69816a55af356cce920aa55f1001ab Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 1 Mar 2022 15:42:29 -0500 Subject: [PATCH 06/14] correct code block --- .../server-discovery-and-monitoring.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst index 22a65d1da2..2b2619d3d7 100644 --- a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst +++ b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst @@ -2431,8 +2431,9 @@ Why is it possible for maxSetVersion to go down? ``maxElectionId`` and ``maxSetVersion`` are actually considered a pair of values Drivers MAY consider implementing comparison in code as a tuple of the two to ensure their always updated together: -:: - New tuple old tuple +.. code:: typescript + + // New tuple old tuple { electionId: 2, setVersion: 1 } > { electionId: 1, setVersion: 50 } In this scenario, the maxSetVersion goes from 50 to 1, but the maxElectionId is raised to 2. From 36ab34b38602d14295217f6edccd4283204810f1 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 2 Mar 2022 11:08:56 -0500 Subject: [PATCH 07/14] fix one singular character that has the power to make all this code incorrect --- .../server-discovery-and-monitoring.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst index 2b2619d3d7..4911895946 100644 --- a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst +++ b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst @@ -1108,7 +1108,7 @@ updateRSFromPrimary # for comparison rules. # Null values are always considered "less than" - maxElectionIdIsGreater = topologyDescription.maxElectionId < serverDescription.electionId if topologyDescription.maxElectionId is not null else false + maxElectionIdIsGreater = topologyDescription.maxElectionId > serverDescription.electionId if topologyDescription.maxElectionId is not null else false maxElectionIdIsEqual = topologyDescription.maxElectionId == serverDescription.electionId if topologyDescription.maxElectionId is not null else false maxElectionIdIsLess = topologyDescription.maxElectionId > serverDescription.electionId if topologyDescription.maxElectionId is not null else false maxSetVersionIsGreater = topologyDescription.maxSetVersion > serverDescription.setVersion if topologyDescription.maxSetVersion is not null else false From d124a8008f2fe1633683884c19b90ec66810ae5c Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 2 Mar 2022 12:08:11 -0500 Subject: [PATCH 08/14] fix --- .../server-discovery-and-monitoring.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst index 4911895946..57d7125407 100644 --- a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst +++ b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst @@ -1110,7 +1110,7 @@ updateRSFromPrimary # Null values are always considered "less than" maxElectionIdIsGreater = topologyDescription.maxElectionId > serverDescription.electionId if topologyDescription.maxElectionId is not null else false maxElectionIdIsEqual = topologyDescription.maxElectionId == serverDescription.electionId if topologyDescription.maxElectionId is not null else false - maxElectionIdIsLess = topologyDescription.maxElectionId > serverDescription.electionId if topologyDescription.maxElectionId is not null else false + maxElectionIdIsLess = topologyDescription.maxElectionId < serverDescription.electionId if topologyDescription.maxElectionId is not null else false maxSetVersionIsGreater = topologyDescription.maxSetVersion > serverDescription.setVersion if topologyDescription.maxSetVersion is not null else false maxSetVersionIsLess = topologyDescription.maxSetVersion < serverDescription.setVersion if topologyDescription.maxSetVersion is not null else true From 577c497164d92e319cd1b300d99db07418383faf Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 4 Mar 2022 10:41:40 -0500 Subject: [PATCH 09/14] fix tests & code --- .../server-discovery-and-monitoring.rst | 54 ++++++++++--------- .../tests/rs/electionId_setVersion.json | 2 +- .../tests/rs/electionId_setVersion.yml | 2 +- .../tests/rs/null_election_id.json | 30 ++++++----- .../tests/rs/null_election_id.yml | 30 ++++++----- .../tests/rs/secondary_ignore_ok_0.json | 2 +- .../tests/rs/secondary_ignore_ok_0.yml | 2 +- .../rs/setversion_without_electionid.json | 10 ++-- .../rs/setversion_without_electionid.yml | 10 ++-- .../rs/use_setversion_without_electionid.json | 17 +++--- .../rs/use_setversion_without_electionid.yml | 17 +++--- 11 files changed, 95 insertions(+), 81 deletions(-) diff --git a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst index 57d7125407..f7abb6b8a8 100644 --- a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst +++ b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst @@ -1089,54 +1089,56 @@ updateRSWithPrimaryFromMember updateRSFromPrimary This subroutine is executed with a ServerDescription of type RSPrimary:: - if description.address not in topologyDescription.servers: + if serverDescription.address not in topologyDescription.servers: return if topologyDescription.setName is null: - topologyDescription.setName = description.setName + topologyDescription.setName = serverDescription.setName - else if topologyDescription.setName != description.setName: + else if topologyDescription.setName != serverDescription.setName: # We found a primary but it doesn't have the setName # provided by the user or previously discovered. remove this server from topologyDescription and stop monitoring it checkIfHasPrimary() return - if description.electionId is not null and description.setVersion is not null: - # Election ids are ObjectIds, see - # "using setVersion and electionId to detect stale primaries" - # for comparison rules. - - # Null values are always considered "less than" - maxElectionIdIsGreater = topologyDescription.maxElectionId > serverDescription.electionId if topologyDescription.maxElectionId is not null else false - maxElectionIdIsEqual = topologyDescription.maxElectionId == serverDescription.electionId if topologyDescription.maxElectionId is not null else false - maxElectionIdIsLess = topologyDescription.maxElectionId < serverDescription.electionId if topologyDescription.maxElectionId is not null else false - maxSetVersionIsGreater = topologyDescription.maxSetVersion > serverDescription.setVersion if topologyDescription.maxSetVersion is not null else false - maxSetVersionIsLess = topologyDescription.maxSetVersion < serverDescription.setVersion if topologyDescription.maxSetVersion is not null else true - - if maxElectionIdIsGreater or (maxElectionIdIsEqual and maxSetVersionIsGreater): - # Stale primary. - # replace description with a default ServerDescription of type "Unknown" - checkIfHasPrimary() - return + # Election ids are ObjectIds, see + # "using setVersion and electionId to detect stale primaries" + # for comparison rules. + + # TODO!!! handle null is min for serverDescription.electionId and serverDescription.setVersion - if maxElectionIdIsLess or (maxElectionIdIsEqual and maxSetVersionIsLess): - topologyDescription.maxElectionId = serverDescription.electionId; - topologyDescription.maxSetVersion = serverDescription.setVersion; + # Null values are always considered "less than" + electionIdIsNull = serverDescription.electionId == null; + maxElectionIdIsGreater = topologyDescription.maxElectionId > serverDescription.electionId if topologyDescription.maxElectionId is not null else false + maxElectionIdIsEqual = topologyDescription.maxElectionId == serverDescription.electionId if topologyDescription.maxElectionId is not null else false + maxElectionIdIsLess = topologyDescription.maxElectionId < serverDescription.electionId if topologyDescription.maxElectionId is not null else false + maxSetVersionIsGreater = topologyDescription.maxSetVersion > serverDescription.setVersion if topologyDescription.maxSetVersion is not null else false + maxSetVersionIsLess = topologyDescription.maxSetVersion < serverDescription.setVersion if topologyDescription.maxSetVersion is not null else true + + if maxElectionIdIsGreater or ((maxElectionIdIsEqual or electionIdIsNull) and maxSetVersionIsGreater): + # Stale primary. + # replace serverDescription with a default ServerDescription of type "Unknown" + checkIfHasPrimary() + return + + if maxElectionIdIsLess or ((maxElectionIdIsEqual or electionIdIsNull) and maxSetVersionIsLess): + topologyDescription.maxElectionId = serverDescription.electionId; + topologyDescription.maxSetVersion = serverDescription.setVersion; for each server in topologyDescription.servers: - if server.address != description.address: + if server.address != serverDescription.address: if server.type is RSPrimary: # See note below about invalidating an old primary. replace the server with a default ServerDescription of type "Unknown" - for each address in description's "hosts", "passives", and "arbiters": + for each address in serverDescription's "hosts", "passives", and "arbiters": if address is not in topologyDescription.servers: add new default ServerDescription of type "Unknown" begin monitoring the new server for each server in topologyDescription.servers: - if server.address not in description's "hosts", "passives", or "arbiters": + if server.address not in serverDescription's "hosts", "passives", or "arbiters": remove the server and stop monitoring it checkIfHasPrimary() diff --git a/source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.json b/source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.json index 3b3cb014ff..a7b49e2b97 100644 --- a/source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.json +++ b/source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.json @@ -82,7 +82,7 @@ "topologyType": "ReplicaSetWithPrimary", "logicalSessionTimeoutMinutes": null, "setName": "rs", - "maxSetVersion": 2, + "maxSetVersion": 1, "maxElectionId": { "$oid": "000000000000000000000002" } diff --git a/source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.yml b/source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.yml index 18c58ac400..e552e984c2 100644 --- a/source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.yml +++ b/source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.yml @@ -57,6 +57,6 @@ phases: topologyType: ReplicaSetWithPrimary logicalSessionTimeoutMinutes: null setName: rs - maxSetVersion: 2 + maxSetVersion: 1 maxElectionId: $oid: "000000000000000000000002" diff --git a/source/server-discovery-and-monitoring/tests/rs/null_election_id.json b/source/server-discovery-and-monitoring/tests/rs/null_election_id.json index 62120e8448..8eb519595a 100644 --- a/source/server-discovery-and-monitoring/tests/rs/null_election_id.json +++ b/source/server-discovery-and-monitoring/tests/rs/null_election_id.json @@ -123,16 +123,19 @@ "outcome": { "servers": { "a:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 1, - "electionId": null - }, - "b:27017": { "type": "Unknown", "setName": null, + "setVersion": null, "electionId": null }, + "b:27017": { + "type": "RSPrimary", + "setName": "rs", + "setVersion": 1, + "electionId": { + "$oid": "000000000000000000000002" + } + }, "c:27017": { "type": "Unknown", "setName": null, @@ -174,16 +177,19 @@ "outcome": { "servers": { "a:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 1, - "electionId": null - }, - "b:27017": { "type": "Unknown", "setName": null, + "setVersion": null, "electionId": null }, + "b:27017": { + "type": "RSPrimary", + "setName": "rs", + "setVersion": 1, + "electionId": { + "$oid": "000000000000000000000002" + } + }, "c:27017": { "type": "Unknown", "setName": null, diff --git a/source/server-discovery-and-monitoring/tests/rs/null_election_id.yml b/source/server-discovery-and-monitoring/tests/rs/null_election_id.yml index 7de496cd79..9377285e43 100644 --- a/source/server-discovery-and-monitoring/tests/rs/null_election_id.yml +++ b/source/server-discovery-and-monitoring/tests/rs/null_election_id.yml @@ -88,7 +88,7 @@ phases: [ } }, - # A still claims to be primary, no electionId, we have to trust it. + # A still claims to be primary, no electionId, we don't trust it. { responses: [ ["a:27017", { @@ -104,17 +104,19 @@ phases: [ ], outcome: { servers: { + # A ignored for missing electionId "a:27017": { - type: "RSPrimary", - setName: "rs", - setVersion: 1, - electionId: - }, - "b:27017": { type: "Unknown", setName: , + setVersion: , electionId: }, + "b:27017": { + type: "RSPrimary", + setName: "rs", + setVersion: 1, + electionId: { "$oid": "000000000000000000000002" } + }, "c:27017": { type: "Unknown", setName: , @@ -147,18 +149,18 @@ phases: [ ], outcome: { servers: { - # Still primary. "a:27017": { - type: "RSPrimary", - setName: "rs", - setVersion: 1, - electionId: - }, - "b:27017": { type: "Unknown", setName: , + setVersion: , electionId: }, + "b:27017": { + type: "RSPrimary", + setName: "rs", + setVersion: 1, + electionId: { "$oid": "000000000000000000000002" } + }, "c:27017": { type: "Unknown", setName: , diff --git a/source/server-discovery-and-monitoring/tests/rs/secondary_ignore_ok_0.json b/source/server-discovery-and-monitoring/tests/rs/secondary_ignore_ok_0.json index 4c1cb011a5..ee9519930b 100644 --- a/source/server-discovery-and-monitoring/tests/rs/secondary_ignore_ok_0.json +++ b/source/server-discovery-and-monitoring/tests/rs/secondary_ignore_ok_0.json @@ -1,5 +1,5 @@ { - "description": "New primary", + "description": "Secondary ignored when ok is zero", "uri": "mongodb://a,b/?replicaSet=rs", "phases": [ { diff --git a/source/server-discovery-and-monitoring/tests/rs/secondary_ignore_ok_0.yml b/source/server-discovery-and-monitoring/tests/rs/secondary_ignore_ok_0.yml index c7e605d662..d94fafaf99 100644 --- a/source/server-discovery-and-monitoring/tests/rs/secondary_ignore_ok_0.yml +++ b/source/server-discovery-and-monitoring/tests/rs/secondary_ignore_ok_0.yml @@ -1,4 +1,4 @@ -description: "New primary" +description: "Secondary ignored when ok is zero" uri: "mongodb://a,b/?replicaSet=rs" diff --git a/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.json b/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.json index 2f68287f1d..22f712fa75 100644 --- a/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.json +++ b/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.json @@ -63,14 +63,14 @@ "outcome": { "servers": { "a:27017": { - "type": "Unknown", - "setName": null, + "type": "RSPrimary", + "setName": "rs", + "setVersion": 2, "electionId": null }, "b:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 1, + "type": "Unknown", + "setName": null, "electionId": null } }, diff --git a/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.yml b/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.yml index 55c841f24b..0f3c89e18a 100644 --- a/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.yml +++ b/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.yml @@ -59,14 +59,14 @@ phases: [ outcome: { servers: { "a:27017": { - type: "Unknown", - setName: , + type: "RSPrimary", + setName: "rs", + setVersion: 2 , electionId: }, "b:27017": { - type: "RSPrimary", - setName: "rs", - setVersion: 1, + type: "Unknown", + setName: , electionId: } }, diff --git a/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.json b/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.json index 06b0faca11..6dd753d5d8 100644 --- a/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.json +++ b/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.json @@ -71,20 +71,23 @@ "outcome": { "servers": { "a:27017": { + "type": "RSPrimary", + "setName": "rs", + "setVersion": 1, + "electionId": { + "$oid": "000000000000000000000001" + } + }, + "b:27017": { "type": "Unknown", "setName": null, "electionId": null - }, - "b:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 2 } }, "topologyType": "ReplicaSetWithPrimary", "logicalSessionTimeoutMinutes": null, "setName": "rs", - "maxSetVersion": 2, + "maxSetVersion": 1, "maxElectionId": { "$oid": "000000000000000000000001" } @@ -131,7 +134,7 @@ "topologyType": "ReplicaSetWithPrimary", "logicalSessionTimeoutMinutes": null, "setName": "rs", - "maxSetVersion": 2, + "maxSetVersion": 1, "maxElectionId": { "$oid": "000000000000000000000002" } diff --git a/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.yml b/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.yml index 815ee96025..669ec20d43 100644 --- a/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.yml +++ b/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.yml @@ -59,21 +59,22 @@ phases: [ outcome: { servers: { - "a:27017": { + "a:27017": { + type: "RSPrimary", + setName: "rs", + setVersion: 1, + electionId: { "$oid": "000000000000000000000001" } + }, + "b:27017": { type: "Unknown", setName: , electionId: - }, - "b:27017": { - type: "RSPrimary", - setName: "rs", - setVersion: 2 } }, topologyType: "ReplicaSetWithPrimary", logicalSessionTimeoutMinutes: null, setName: "rs", - maxSetVersion: 2, + maxSetVersion: 1, maxElectionId: {"$oid": "000000000000000000000001"}, } }, @@ -111,7 +112,7 @@ phases: [ topologyType: "ReplicaSetWithPrimary", logicalSessionTimeoutMinutes: null, setName: "rs", - maxSetVersion: 2, + maxSetVersion: 1, maxElectionId: {"$oid": "000000000000000000000002"}, } } From 389c7d584ba0c36928071d78def60bce35a9117d Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 4 Mar 2022 10:43:44 -0500 Subject: [PATCH 10/14] rm todo --- .../server-discovery-and-monitoring.rst | 2 -- 1 file changed, 2 deletions(-) diff --git a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst index f7abb6b8a8..5c31eb2eed 100644 --- a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst +++ b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst @@ -1106,8 +1106,6 @@ updateRSFromPrimary # "using setVersion and electionId to detect stale primaries" # for comparison rules. - # TODO!!! handle null is min for serverDescription.electionId and serverDescription.setVersion - # Null values are always considered "less than" electionIdIsNull = serverDescription.electionId == null; maxElectionIdIsGreater = topologyDescription.maxElectionId > serverDescription.electionId if topologyDescription.maxElectionId is not null else false From 171b630fafc8bb30751cb4ce5bc8a8d30a17d397 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 4 Mar 2022 12:45:28 -0500 Subject: [PATCH 11/14] simplify ternaries --- .../server-discovery-and-monitoring.rst | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst index 5c31eb2eed..24b749823b 100644 --- a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst +++ b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst @@ -1106,13 +1106,13 @@ updateRSFromPrimary # "using setVersion and electionId to detect stale primaries" # for comparison rules. - # Null values are always considered "less than" + # Null values for both electionId and setVersion are always considered less than electionIdIsNull = serverDescription.electionId == null; - maxElectionIdIsGreater = topologyDescription.maxElectionId > serverDescription.electionId if topologyDescription.maxElectionId is not null else false - maxElectionIdIsEqual = topologyDescription.maxElectionId == serverDescription.electionId if topologyDescription.maxElectionId is not null else false - maxElectionIdIsLess = topologyDescription.maxElectionId < serverDescription.electionId if topologyDescription.maxElectionId is not null else false - maxSetVersionIsGreater = topologyDescription.maxSetVersion > serverDescription.setVersion if topologyDescription.maxSetVersion is not null else false - maxSetVersionIsLess = topologyDescription.maxSetVersion < serverDescription.setVersion if topologyDescription.maxSetVersion is not null else true + maxElectionIdIsGreater = topologyDescription.maxElectionId > serverDescription.electionId + maxElectionIdIsEqual = topologyDescription.maxElectionId == serverDescription.electionId + maxElectionIdIsLess = topologyDescription.maxElectionId < serverDescription.electionId + maxSetVersionIsGreater = topologyDescription.maxSetVersion > serverDescription.setVersion + maxSetVersionIsLess = topologyDescription.maxSetVersion < serverDescription.setVersion if maxElectionIdIsGreater or ((maxElectionIdIsEqual or electionIdIsNull) and maxSetVersionIsGreater): # Stale primary. From 62587e21c2c43c11ddd9da3718a866691b72b9b3 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 4 Mar 2022 13:24:33 -0500 Subject: [PATCH 12/14] rm tmp variables, update test comment --- .../server-discovery-and-monitoring.rst | 27 +++++++++++-------- .../rs/setversion_without_electionid.yml | 3 +-- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst index 24b749823b..2e015eedc7 100644 --- a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst +++ b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst @@ -1107,22 +1107,27 @@ updateRSFromPrimary # for comparison rules. # Null values for both electionId and setVersion are always considered less than - electionIdIsNull = serverDescription.electionId == null; - maxElectionIdIsGreater = topologyDescription.maxElectionId > serverDescription.electionId - maxElectionIdIsEqual = topologyDescription.maxElectionId == serverDescription.electionId - maxElectionIdIsLess = topologyDescription.maxElectionId < serverDescription.electionId - maxSetVersionIsGreater = topologyDescription.maxSetVersion > serverDescription.setVersion - maxSetVersionIsLess = topologyDescription.maxSetVersion < serverDescription.setVersion - - if maxElectionIdIsGreater or ((maxElectionIdIsEqual or electionIdIsNull) and maxSetVersionIsGreater): + if topologyDescription.maxElectionId > serverDescription.electionId or ( + ( + topologyDescription.maxElectionId == serverDescription.electionId + or serverDescription.electionId == null + ) + and topologyDescription.maxSetVersion > serverDescription.setVersion + ): # Stale primary. # replace serverDescription with a default ServerDescription of type "Unknown" checkIfHasPrimary() return - if maxElectionIdIsLess or ((maxElectionIdIsEqual or electionIdIsNull) and maxSetVersionIsLess): - topologyDescription.maxElectionId = serverDescription.electionId; - topologyDescription.maxSetVersion = serverDescription.setVersion; + if topologyDescription.maxElectionId < serverDescription.electionId or ( + ( + topologyDescription.maxElectionId == serverDescription.electionId + or serverDescription.electionId == null + ) + and topologyDescription.maxSetVersion < serverDescription.setVersion + ): + topologyDescription.maxElectionId = serverDescription.electionId + topologyDescription.maxSetVersion = serverDescription.setVersion for each server in topologyDescription.servers: if server.address != serverDescription.address: diff --git a/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.yml b/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.yml index 0f3c89e18a..235d5aaa0f 100644 --- a/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.yml +++ b/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.yml @@ -40,8 +40,7 @@ phases: [ } }, - # B is elected, its setVersion is older but we believe it anyway, because - # setVersion is only used in conjunction with electionId. + # B is elected, its setVersion is older so it is stale { responses: [ ["b:27017", { From 7431b3b8fe58e1da5055397565e1318578d5cd0f Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 4 Mar 2022 14:23:06 -0500 Subject: [PATCH 13/14] Add less/greater/equal setVersion tests --- ... => electionId_precedence_setVersion.json} | 0 ...l => electionId_precedence_setVersion.yml} | 0 ...tversion_equal_max_without_electionid.json | 84 +++++++++++++++++++ ...etversion_equal_max_without_electionid.yml | 78 +++++++++++++++++ ...on_greaterthan_max_without_electionid.json | 84 +++++++++++++++++++ ...ion_greaterthan_max_without_electionid.yml | 79 +++++++++++++++++ .../rs/setversion_without_electionid.json | 2 +- .../rs/setversion_without_electionid.yml | 2 +- 8 files changed, 327 insertions(+), 2 deletions(-) rename source/server-discovery-and-monitoring/tests/rs/{electionId_setVersion.json => electionId_precedence_setVersion.json} (100%) rename source/server-discovery-and-monitoring/tests/rs/{electionId_setVersion.yml => electionId_precedence_setVersion.yml} (100%) create mode 100644 source/server-discovery-and-monitoring/tests/rs/setversion_equal_max_without_electionid.json create mode 100644 source/server-discovery-and-monitoring/tests/rs/setversion_equal_max_without_electionid.yml create mode 100644 source/server-discovery-and-monitoring/tests/rs/setversion_greaterthan_max_without_electionid.json create mode 100644 source/server-discovery-and-monitoring/tests/rs/setversion_greaterthan_max_without_electionid.yml diff --git a/source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.json b/source/server-discovery-and-monitoring/tests/rs/electionId_precedence_setVersion.json similarity index 100% rename from source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.json rename to source/server-discovery-and-monitoring/tests/rs/electionId_precedence_setVersion.json diff --git a/source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.yml b/source/server-discovery-and-monitoring/tests/rs/electionId_precedence_setVersion.yml similarity index 100% rename from source/server-discovery-and-monitoring/tests/rs/electionId_setVersion.yml rename to source/server-discovery-and-monitoring/tests/rs/electionId_precedence_setVersion.yml diff --git a/source/server-discovery-and-monitoring/tests/rs/setversion_equal_max_without_electionid.json b/source/server-discovery-and-monitoring/tests/rs/setversion_equal_max_without_electionid.json new file mode 100644 index 0000000000..91e84d4fa0 --- /dev/null +++ b/source/server-discovery-and-monitoring/tests/rs/setversion_equal_max_without_electionid.json @@ -0,0 +1,84 @@ +{ + "description": "setVersion version that is equal is treated the same as greater than if there is no electionId", + "uri": "mongodb://a/?replicaSet=rs", + "phases": [ + { + "responses": [ + [ + "a:27017", + { + "ok": 1, + "helloOk": true, + "isWritablePrimary": true, + "hosts": [ + "a:27017", + "b:27017" + ], + "setName": "rs", + "setVersion": 1, + "minWireVersion": 0, + "maxWireVersion": 6 + } + ] + ], + "outcome": { + "servers": { + "a:27017": { + "type": "RSPrimary", + "setName": "rs", + "setVersion": 1, + "electionId": null + }, + "b:27017": { + "type": "Unknown", + "setName": null, + "electionId": null + } + }, + "topologyType": "ReplicaSetWithPrimary", + "logicalSessionTimeoutMinutes": null, + "setName": "rs", + "maxSetVersion": 1 + } + }, + { + "responses": [ + [ + "b:27017", + { + "ok": 1, + "helloOk": true, + "isWritablePrimary": true, + "hosts": [ + "a:27017", + "b:27017" + ], + "setName": "rs", + "setVersion": 1, + "minWireVersion": 0, + "maxWireVersion": 6 + } + ] + ], + "outcome": { + "servers": { + "a:27017": { + "type": "Unknown", + "setName": null, + "electionId": null + }, + "b:27017": { + "type": "RSPrimary", + "setName": "rs", + "setVersion": 1, + "electionId": null + } + }, + "topologyType": "ReplicaSetWithPrimary", + "logicalSessionTimeoutMinutes": null, + "setName": "rs", + "maxSetVersion": 1 + } + } + ] +} diff --git a/source/server-discovery-and-monitoring/tests/rs/setversion_equal_max_without_electionid.yml b/source/server-discovery-and-monitoring/tests/rs/setversion_equal_max_without_electionid.yml new file mode 100644 index 0000000000..cdce70f030 --- /dev/null +++ b/source/server-discovery-and-monitoring/tests/rs/setversion_equal_max_without_electionid.yml @@ -0,0 +1,78 @@ +description: "setVersion version that is equal is treated the same as greater than if there is no electionId" + +uri: "mongodb://a/?replicaSet=rs" + +phases: [ + + # Primary A is discovered and tells us about B. + { + responses: [ + ["a:27017", { + ok: 1, + helloOk: true, + isWritablePrimary: true, + hosts: ["a:27017", "b:27017"], + setName: "rs", + setVersion: 1, + minWireVersion: 0, + maxWireVersion: 6 + }] + ], + + outcome: { + servers: { + "a:27017": { + type: "RSPrimary", + setName: "rs", + setVersion: 1, + electionId: + }, + "b:27017": { + type: "Unknown", + setName: , + electionId: + } + }, + topologyType: "ReplicaSetWithPrimary", + logicalSessionTimeoutMinutes: null, + setName: "rs", + maxSetVersion: 1, + } + }, + + # B is elected, its setVersion is older so it is stale + { + responses: [ + ["b:27017", { + ok: 1, + helloOk: true, + isWritablePrimary: true, + hosts: ["a:27017", "b:27017"], + setName: "rs", + setVersion: 1, + minWireVersion: 0, + maxWireVersion: 6 + }] + ], + + outcome: { + servers: { + "a:27017": { + type: "Unknown", + setName: , + electionId: + }, + "b:27017": { + type: "RSPrimary", + setName: "rs", + setVersion: 1, + electionId: + } + }, + topologyType: "ReplicaSetWithPrimary", + logicalSessionTimeoutMinutes: null, + setName: "rs", + maxSetVersion: 1, # Max is still 1, there wasn't an actual larger setVersion seen + } + } +] diff --git a/source/server-discovery-and-monitoring/tests/rs/setversion_greaterthan_max_without_electionid.json b/source/server-discovery-and-monitoring/tests/rs/setversion_greaterthan_max_without_electionid.json new file mode 100644 index 0000000000..b15fd5c1a7 --- /dev/null +++ b/source/server-discovery-and-monitoring/tests/rs/setversion_greaterthan_max_without_electionid.json @@ -0,0 +1,84 @@ +{ + "description": "setVersion that is greater than maxSetVersion is used if there is no electionId", + "uri": "mongodb://a/?replicaSet=rs", + "phases": [ + { + "responses": [ + [ + "a:27017", + { + "ok": 1, + "helloOk": true, + "isWritablePrimary": true, + "hosts": [ + "a:27017", + "b:27017" + ], + "setName": "rs", + "setVersion": 1, + "minWireVersion": 0, + "maxWireVersion": 6 + } + ] + ], + "outcome": { + "servers": { + "a:27017": { + "type": "RSPrimary", + "setName": "rs", + "setVersion": 1, + "electionId": null + }, + "b:27017": { + "type": "Unknown", + "setName": null, + "electionId": null + } + }, + "topologyType": "ReplicaSetWithPrimary", + "logicalSessionTimeoutMinutes": null, + "setName": "rs", + "maxSetVersion": 1 + } + }, + { + "responses": [ + [ + "b:27017", + { + "ok": 1, + "helloOk": true, + "isWritablePrimary": true, + "hosts": [ + "a:27017", + "b:27017" + ], + "setName": "rs", + "setVersion": 2, + "minWireVersion": 0, + "maxWireVersion": 6 + } + ] + ], + "outcome": { + "servers": { + "a:27017": { + "type": "Unknown", + "setName": null, + "electionId": null + }, + "b:27017": { + "type": "RSPrimary", + "setName": "rs", + "setVersion": 2, + "electionId": null + } + }, + "topologyType": "ReplicaSetWithPrimary", + "logicalSessionTimeoutMinutes": null, + "setName": "rs", + "maxSetVersion": 2 + } + } + ] +} diff --git a/source/server-discovery-and-monitoring/tests/rs/setversion_greaterthan_max_without_electionid.yml b/source/server-discovery-and-monitoring/tests/rs/setversion_greaterthan_max_without_electionid.yml new file mode 100644 index 0000000000..34150c4422 --- /dev/null +++ b/source/server-discovery-and-monitoring/tests/rs/setversion_greaterthan_max_without_electionid.yml @@ -0,0 +1,79 @@ +description: "setVersion that is greater than maxSetVersion is used if there is no electionId" + +uri: "mongodb://a/?replicaSet=rs" + +phases: [ + + # Primary A is discovered and tells us about B. + { + responses: [ + ["a:27017", { + ok: 1, + helloOk: true, + isWritablePrimary: true, + hosts: ["a:27017", "b:27017"], + setName: "rs", + setVersion: 1, + minWireVersion: 0, + maxWireVersion: 6 + }] + ], + + outcome: { + servers: { + "a:27017": { + type: "RSPrimary", + setName: "rs", + setVersion: 1, + electionId: + }, + "b:27017": { + type: "Unknown", + setName: , + electionId: + } + }, + topologyType: "ReplicaSetWithPrimary", + logicalSessionTimeoutMinutes: null, + setName: "rs", + maxSetVersion: 1, + } + }, + + # B is elected, its setVersion is greater than our current maxSetVersion + # B is primary, A is marked Unknown + { + responses: [ + ["b:27017", { + ok: 1, + helloOk: true, + isWritablePrimary: true, + hosts: ["a:27017", "b:27017"], + setName: "rs", + setVersion: 2, + minWireVersion: 0, + maxWireVersion: 6 + }] + ], + + outcome: { + servers: { + "a:27017": { + type: "Unknown", + setName: , + electionId: + }, + "b:27017": { + type: "RSPrimary", + setName: "rs", + setVersion: 2, + electionId: + }, + }, + topologyType: "ReplicaSetWithPrimary", + logicalSessionTimeoutMinutes: null, + setName: "rs", + maxSetVersion: 2, + } + } +] diff --git a/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.json b/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.json index 22f712fa75..f59c162ae1 100644 --- a/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.json +++ b/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.json @@ -1,5 +1,5 @@ { - "description": "setVersion is ignored if there is no electionId", + "description": "setVersion that is less than maxSetVersion is ignored if there is no electionId", "uri": "mongodb://a/?replicaSet=rs", "phases": [ { diff --git a/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.yml b/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.yml index 235d5aaa0f..ea8f28c238 100644 --- a/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.yml +++ b/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.yml @@ -1,4 +1,4 @@ -description: "setVersion is ignored if there is no electionId" +description: "setVersion that is less than maxSetVersion is ignored if there is no electionId" uri: "mongodb://a/?replicaSet=rs" From 5e179aad209ba2a30851a71ffa197b0bb94742f0 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 7 Mar 2022 14:47:42 -0500 Subject: [PATCH 14/14] simplify the if stmt --- .../server-discovery-and-monitoring.rst | 22 +++++-------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst index 2e015eedc7..18a306c073 100644 --- a/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst +++ b/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst @@ -1107,28 +1107,18 @@ updateRSFromPrimary # for comparison rules. # Null values for both electionId and setVersion are always considered less than - if topologyDescription.maxElectionId > serverDescription.electionId or ( - ( - topologyDescription.maxElectionId == serverDescription.electionId - or serverDescription.electionId == null - ) - and topologyDescription.maxSetVersion > serverDescription.setVersion + if serverDescription.electionId > serverDescription.maxElectionId or ( + serverDescription.electionId == topologyDescription.maxElectionId + and serverDescription.setVersion >= topologyDescription.maxSetVersion ): + topologyDescription.maxElectionId = serverDescription.electionId + topologyDescription.maxSetVersion = serverDescription.setVersion + else: # Stale primary. # replace serverDescription with a default ServerDescription of type "Unknown" checkIfHasPrimary() return - if topologyDescription.maxElectionId < serverDescription.electionId or ( - ( - topologyDescription.maxElectionId == serverDescription.electionId - or serverDescription.electionId == null - ) - and topologyDescription.maxSetVersion < serverDescription.setVersion - ): - topologyDescription.maxElectionId = serverDescription.electionId - topologyDescription.maxSetVersion = serverDescription.setVersion - for each server in topologyDescription.servers: if server.address != serverDescription.address: if server.type is RSPrimary: