Skip to content

Commit 225d131

Browse files
ShaneHarveyShaneHarvey
authored
PYTHON-2970 Prioritize electionId over setVersion for stale primary check (#845)
1 parent f081297 commit 225d131

11 files changed

+481
-50
lines changed

‎doc/changelog.rst

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,21 @@ PyMongo 4.1 brings a number of improvements including:
2626
- :meth:`gridfs.GridOut.seek` now returns the new position in the file, to
2727
conform to the behavior of :meth:`io.IOBase.seek`.
2828

29+
Bug fixes
30+
.........
31+
32+
- Fixed a bug where the client could be unable to discover the new primary
33+
after a simultaneous replica set election and reconfig (`PYTHON-2970`_).
34+
35+
.. _PYTHON-2970: https://jira.mongodb.org/browse/PYTHON-2970
36+
37+
Issues Resolved
38+
...............
39+
40+
See the `PyMongo 4.1 release notes in JIRA`_ for the list of resolved issues
41+
in this release.
42+
43+
.. _PyMongo 4.1 release notes in JIRA: https://jira.mongodb.org/secure/ReleaseNote.jspa?projectId=10004&version=30619
2944

3045
Changes in Version 4.0
3146
----------------------

‎pymongo/topology_description.py

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from random import sample
1818
from typing import Any, Callable, Dict, List, NamedTuple, Optional, Tuple
1919

20+
from bson.min_key import MinKey
2021
from bson.objectid import ObjectId
2122
from pymongo import common
2223
from pymongo.errors import ConfigurationError
@@ -531,24 +532,16 @@ def _update_rs_from_primary(
531532
sds.pop(server_description.address)
532533
return (_check_has_primary(sds), replica_set_name, max_set_version, max_election_id)
533534

534-
max_election_tuple = max_set_version, max_election_id
535-
if None not in server_description.election_tuple:
536-
if (
537-
None not in max_election_tuple
538-
and max_election_tuple > server_description.election_tuple
539-
):
540-
541-
# Stale primary, set to type Unknown.
542-
sds[server_description.address] = server_description.to_unknown()
543-
return (_check_has_primary(sds), replica_set_name, max_set_version, max_election_id)
544-
545-
max_election_id = server_description.election_id
546-
547-
if server_description.set_version is not None and (
548-
max_set_version is None or server_description.set_version > max_set_version
549-
):
550-
551-
max_set_version = server_description.set_version
535+
new_election_tuple = server_description.election_id, server_description.set_version
536+
max_election_tuple = max_election_id, max_set_version
537+
new_election_safe = tuple(MinKey() if i is None else i for i in new_election_tuple)
538+
max_election_safe = tuple(MinKey() if i is None else i for i in max_election_tuple)
539+
if new_election_safe >= max_election_safe:
540+
max_election_id, max_set_version = new_election_tuple
541+
else:
542+
# Stale primary, set to type Unknown.
543+
sds[server_description.address] = server_description.to_unknown()
544+
return _check_has_primary(sds), replica_set_name, max_set_version, max_election_id
552545

553546
# We've heard from the primary. Is it the same primary as before?
554547
for server in sds.values():

‎test/discovery_and_monitoring/rs/electionId_precedence_setVersion.json

Lines changed: 92 additions & 0 deletions
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": 6
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": 6
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": 6
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/discovery_and_monitoring/rs/null_election_id.json

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -123,16 +123,19 @@
123123
"outcome": {
124124
"servers": {
125125
"a:27017": {
126-
"type": "RSPrimary",
127-
"setName": "rs",
128-
"setVersion": 1,
129-
"electionId": null
130-
},
131-
"b:27017": {
132126
"type": "Unknown",
133127
"setName": null,
128+
"setVersion": null,
134129
"electionId": null
135130
},
131+
"b:27017": {
132+
"type": "RSPrimary",
133+
"setName": "rs",
134+
"setVersion": 1,
135+
"electionId": {
136+
"$oid": "000000000000000000000002"
137+
}
138+
},
136139
"c:27017": {
137140
"type": "Unknown",
138141
"setName": null,
@@ -174,16 +177,19 @@
174177
"outcome": {
175178
"servers": {
176179
"a:27017": {
177-
"type": "RSPrimary",
178-
"setName": "rs",
179-
"setVersion": 1,
180-
"electionId": null
181-
},
182-
"b:27017": {
183180
"type": "Unknown",
184181
"setName": null,
182+
"setVersion": null,
185183
"electionId": null
186184
},
185+
"b:27017": {
186+
"type": "RSPrimary",
187+
"setName": "rs",
188+
"setVersion": 1,
189+
"electionId": {
190+
"$oid": "000000000000000000000002"
191+
}
192+
},
187193
"c:27017": {
188194
"type": "Unknown",
189195
"setName": null,

‎test/discovery_and_monitoring/rs/secondary_ignore_ok_0.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"description": "New primary",
2+
"description": "Secondary ignored when ok is zero",
33
"uri": "mongodb://a,b/?replicaSet=rs",
44
"phases": [
55
{

‎test/discovery_and_monitoring/rs/set_version_can_rollback.json

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
{
2+
"description": "Set version rolls back after new primary with higher election Id",
3+
"uri": "mongodb://a/?replicaSet=rs",
4+
"phases": [
5+
{
6+
"responses": [
7+
[
8+
"a:27017",
9+
{
10+
"ok": 1,
11+
"hello": true,
12+
"isWritablePrimary": true,
13+
"hosts": [
14+
"a:27017",
15+
"b:27017"
16+
],
17+
"setName": "rs",
18+
"setVersion": 2,
19+
"electionId": {
20+
"$oid": "000000000000000000000001"
21+
},
22+
"minWireVersion": 0,
23+
"maxWireVersion": 6
24+
}
25+
]
26+
],
27+
"outcome": {
28+
"servers": {
29+
"a:27017": {
30+
"type": "RSPrimary",
31+
"setName": "rs",
32+
"setVersion": 2,
33+
"electionId": {
34+
"$oid": "000000000000000000000001"
35+
}
36+
},
37+
"b:27017": {
38+
"type": "Unknown",
39+
"setName": null,
40+
"electionId": null
41+
}
42+
},
43+
"topologyType": "ReplicaSetWithPrimary",
44+
"logicalSessionTimeoutMinutes": null,
45+
"setName": "rs",
46+
"maxSetVersion": 2,
47+
"maxElectionId": {
48+
"$oid": "000000000000000000000001"
49+
}
50+
}
51+
},
52+
{
53+
"_comment": "Response from new primary with newer election Id",
54+
"responses": [
55+
[
56+
"b:27017",
57+
{
58+
"ok": 1,
59+
"hello": true,
60+
"isWritablePrimary": true,
61+
"hosts": [
62+
"a:27017",
63+
"b:27017"
64+
],
65+
"setName": "rs",
66+
"setVersion": 1,
67+
"electionId": {
68+
"$oid": "000000000000000000000002"
69+
},
70+
"minWireVersion": 0,
71+
"maxWireVersion": 6
72+
}
73+
]
74+
],
75+
"outcome": {
76+
"servers": {
77+
"a:27017": {
78+
"type": "Unknown",
79+
"setName": null,
80+
"electionId": null
81+
},
82+
"b:27017": {
83+
"type": "RSPrimary",
84+
"setName": "rs",
85+
"setVersion": 1,
86+
"electionId": {
87+
"$oid": "000000000000000000000002"
88+
}
89+
}
90+
},
91+
"topologyType": "ReplicaSetWithPrimary",
92+
"logicalSessionTimeoutMinutes": null,
93+
"setName": "rs",
94+
"maxSetVersion": 1,
95+
"maxElectionId": {
96+
"$oid": "000000000000000000000002"
97+
}
98+
}
99+
},
100+
{
101+
"_comment": "Response from stale primary",
102+
"responses": [
103+
[
104+
"a:27017",
105+
{
106+
"ok": 1,
107+
"hello": true,
108+
"isWritablePrimary": true,
109+
"hosts": [
110+
"a:27017",
111+
"b:27017"
112+
],
113+
"setName": "rs",
114+
"setVersion": 2,
115+
"electionId": {
116+
"$oid": "000000000000000000000001"
117+
},
118+
"minWireVersion": 0,
119+
"maxWireVersion": 6
120+
}
121+
]
122+
],
123+
"outcome": {
124+
"servers": {
125+
"a:27017": {
126+
"type": "Unknown",
127+
"setName": null,
128+
"electionId": null
129+
},
130+
"b:27017": {
131+
"type": "RSPrimary",
132+
"setName": "rs",
133+
"setVersion": 1,
134+
"electionId": {
135+
"$oid": "000000000000000000000002"
136+
}
137+
}
138+
},
139+
"topologyType": "ReplicaSetWithPrimary",
140+
"logicalSessionTimeoutMinutes": null,
141+
"setName": "rs",
142+
"maxSetVersion": 1,
143+
"maxElectionId": {
144+
"$oid": "000000000000000000000002"
145+
}
146+
}
147+
}
148+
]
149+
}

0 commit comments

Comments
 (0)