Skip to content

Commit a17c52e

Browse files
jyeminjyemin
authored
Prioritize electionId over setVersion when updating topology (#886)
JAVA-4375
1 parent 70ad9c9 commit a17c52e

10 files changed

+575
-76
lines changed

‎driver-core/src/main/com/mongodb/internal/connection/AbstractMultiServerCluster.java

Lines changed: 30 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -228,9 +228,7 @@ private boolean handleReplicaSetMemberChanged(final ServerDescription newDescrip
228228
}
229229

230230
if (newDescription.getType() == REPLICA_SET_GHOST) {
231-
if (LOGGER.isInfoEnabled()) {
232-
LOGGER.info(format("Server %s does not appear to be a member of an initiated replica set.", newDescription.getAddress()));
233-
}
231+
LOGGER.info(format("Server %s does not appear to be a member of an initiated replica set.", newDescription.getAddress()));
234232
return true;
235233
}
236234

@@ -251,46 +249,25 @@ private boolean handleReplicaSetMemberChanged(final ServerDescription newDescrip
251249
if (newDescription.getCanonicalAddress() != null
252250
&& !newDescription.getAddress().equals(new ServerAddress(newDescription.getCanonicalAddress()))
253251
&& !newDescription.isPrimary()) {
254-
if (LOGGER.isInfoEnabled()) {
255-
LOGGER.info(format("Canonical address %s does not match server address. Removing %s from client view of cluster",
256-
newDescription.getCanonicalAddress(), newDescription.getAddress()));
257-
}
252+
LOGGER.info(format("Canonical address %s does not match server address. Removing %s from client view of cluster",
253+
newDescription.getCanonicalAddress(), newDescription.getAddress()));
258254
removeServer(newDescription.getAddress());
259255
return true;
260256
}
261257

262258
if (newDescription.isPrimary()) {
263-
ObjectId electionId = newDescription.getElectionId();
264-
Integer setVersion = newDescription.getSetVersion();
265-
if (setVersion != null && electionId != null) {
266-
if (isStalePrimary(newDescription)) {
267-
if (LOGGER.isInfoEnabled()) {
268-
LOGGER.info(format("Invalidating potential primary %s whose (set version, election id) tuple of (%d, %s) "
259+
if (isNonStalePrimary(newDescription.getElectionId(), newDescription.getSetVersion())) {
260+
LOGGER.info(format("Setting max election id to %s and max set version to %d from replica set primary %s",
261+
newDescription.getElectionId(), newDescription.getSetVersion(), newDescription.getAddress()));
262+
maxElectionId = newDescription.getElectionId();
263+
maxSetVersion = newDescription.getSetVersion();
264+
} else {
265+
LOGGER.info(format("Invalidating potential primary %s whose (set version, election id) tuple of (%d, %s) "
269266
+ "is less than one already seen of (%d, %s)",
270-
newDescription.getAddress(),
271-
setVersion, electionId,
272-
maxSetVersion, maxElectionId));
273-
}
274-
addressToServerTupleMap.get(newDescription.getAddress()).server.resetToConnecting();
275-
return false;
276-
}
277-
278-
if (!electionId.equals(maxElectionId)) {
279-
if (LOGGER.isInfoEnabled()) {
280-
LOGGER.info(format("Setting max election id to %s from replica set primary %s", electionId,
281-
newDescription.getAddress()));
282-
}
283-
maxElectionId = electionId;
284-
}
285-
}
286-
287-
if (setVersion != null
288-
&& (maxSetVersion == null || setVersion.compareTo(maxSetVersion) > 0)) {
289-
if (LOGGER.isInfoEnabled()) {
290-
LOGGER.info(format("Setting max set version to %d from replica set primary %s", setVersion,
291-
newDescription.getAddress()));
292-
}
293-
maxSetVersion = setVersion;
267+
newDescription.getAddress(), newDescription.getSetVersion(), newDescription.getElectionId(),
268+
maxSetVersion, maxElectionId));
269+
addressToServerTupleMap.get(newDescription.getAddress()).server.resetToConnecting();
270+
return false;
294271
}
295272

296273
if (isNotAlreadyPrimary(newDescription.getAddress())) {
@@ -301,14 +278,23 @@ private boolean handleReplicaSetMemberChanged(final ServerDescription newDescrip
301278
return true;
302279
}
303280

304-
private boolean isStalePrimary(final ServerDescription newDescription) {
305-
if (maxSetVersion == null || maxElectionId == null) {
306-
return false;
307-
}
281+
private boolean isNonStalePrimary(final ObjectId electionId, final Integer setVersion) {
282+
return nullSafeCompareTo(electionId, maxElectionId) > 0
283+
|| (nullSafeCompareTo(electionId, maxElectionId) == 0 && nullSafeCompareTo(setVersion, maxSetVersion) >= 0);
284+
}
308285

309-
Integer setVersion = newDescription.getSetVersion();
310-
return (setVersion == null || maxSetVersion.compareTo(setVersion) > 0
311-
|| (maxSetVersion.equals(setVersion) && maxElectionId.compareTo(newDescription.getElectionId()) > 0));
286+
/**
287+
* Implements the same contract as {@link Comparable#compareTo(Object)}, except that a null value is always considers less-than any
288+
* other value (except null, which it considers as equal-to).
289+
*/
290+
private static <T extends Comparable<T>> int nullSafeCompareTo(final T first, final T second) {
291+
if (first == null) {
292+
return second == null ? 0 : -1;
293+
}
294+
if (second == null) {
295+
return 1;
296+
}
297+
return first.compareTo(second);
312298
}
313299

314300
private boolean isNotAlreadyPrimary(final ServerAddress address) {

‎driver-core/src/test/resources/server-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+
}

‎driver-core/src/test/resources/server-discovery-and-monitoring/rs/electionId_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+
}

‎driver-core/src/test/resources/server-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,

‎driver-core/src/test/resources/server-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
{

0 commit comments

Comments
 (0)