Revert "Prioritize electionId over setVersion when updating topology (#886)"

This reverts commit a17c52e.

JAVA-4375

Author: jyemin

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

@@ -224,7 +224,9 @@ private boolean handleReplicaSetMemberChanged(final ServerDescription newDescrip
         }
 
         if (newDescription.getType() == REPLICA_SET_GHOST) {
-            LOGGER.info(format("Server %s does not appear to be a member of an initiated replica set.", newDescription.getAddress()));
+            if (LOGGER.isInfoEnabled()) {
+                LOGGER.info(format("Server %s does not appear to be a member of an initiated replica set.", newDescription.getAddress()));
+            }
             return true;
         }
 
@@ -245,25 +247,46 @@ private boolean handleReplicaSetMemberChanged(final ServerDescription newDescrip
         if (newDescription.getCanonicalAddress() != null
                 && !newDescription.getAddress().equals(new ServerAddress(newDescription.getCanonicalAddress()))
                 && !newDescription.isPrimary()) {
-            LOGGER.info(format("Canonical address %s does not match server address.  Removing %s from client view of cluster",
-                    newDescription.getCanonicalAddress(), newDescription.getAddress()));
+            if (LOGGER.isInfoEnabled()) {
+                LOGGER.info(format("Canonical address %s does not match server address.  Removing %s from client view of cluster",
+                                   newDescription.getCanonicalAddress(), newDescription.getAddress()));
+            }
             removeServer(newDescription.getAddress());
             return true;
         }
 
         if (newDescription.isPrimary()) {
-           if (isNonStalePrimary(newDescription.getElectionId(), newDescription.getSetVersion())) {
-                LOGGER.info(format("Setting max election id to %s and max set version to %d from replica set primary %s",
-                        newDescription.getElectionId(), newDescription.getSetVersion(), newDescription.getAddress()));
-                maxElectionId = newDescription.getElectionId();
-                maxSetVersion = newDescription.getSetVersion();
-            } else {
-                LOGGER.info(format("Invalidating potential primary %s whose (set version, election id) tuple of (%d, %s) "
+            ObjectId electionId = newDescription.getElectionId();
+            Integer setVersion = newDescription.getSetVersion();
+            if (setVersion != null && electionId != null) {
+                if (isStalePrimary(newDescription)) {
+                    if (LOGGER.isInfoEnabled()) {
+                        LOGGER.info(format("Invalidating potential primary %s whose (set version, election id) tuple of (%d, %s) "
                                 + "is less than one already seen of (%d, %s)",
-                        newDescription.getAddress(), newDescription.getSetVersion(), newDescription.getElectionId(),
-                        maxSetVersion, maxElectionId));
-                addressToServerTupleMap.get(newDescription.getAddress()).server.resetToConnecting();
-                return false;
+                                newDescription.getAddress(),
+                                setVersion, electionId,
+                                maxSetVersion, maxElectionId));
+                    }
+                    addressToServerTupleMap.get(newDescription.getAddress()).server.resetToConnecting();
+                    return false;
+                }
+
+                if (!electionId.equals(maxElectionId)) {
+                    if (LOGGER.isInfoEnabled()) {
+                        LOGGER.info(format("Setting max election id to %s from replica set primary %s", electionId,
+                                newDescription.getAddress()));
+                    }
+                    maxElectionId = electionId;
+                }
+            }
+
+            if (setVersion != null
+                    && (maxSetVersion == null || setVersion.compareTo(maxSetVersion) > 0)) {
+                if (LOGGER.isInfoEnabled()) {
+                    LOGGER.info(format("Setting max set version to %d from replica set primary %s", setVersion,
+                            newDescription.getAddress()));
+                }
+                maxSetVersion = setVersion;
             }
 
             if (isNotAlreadyPrimary(newDescription.getAddress())) {
@@ -274,23 +297,14 @@ private boolean handleReplicaSetMemberChanged(final ServerDescription newDescrip
         return true;
     }
 
-    private boolean isNonStalePrimary(final ObjectId electionId, final Integer setVersion) {
-        return nullSafeCompareTo(electionId, maxElectionId) > 0
-                || (nullSafeCompareTo(electionId, maxElectionId) == 0 && nullSafeCompareTo(setVersion, maxSetVersion) >= 0);
-    }
-
-    /**
-     * Implements the same contract as {@link Comparable#compareTo(Object)}, except that a null value is always considers less-than any
-     * other value (except null, which it considers as equal-to).
-     */
-    private static <T extends Comparable<T>> int nullSafeCompareTo(final T first, final T second) {
-        if (first == null) {
-            return second == null ? 0 : -1;
+    private boolean isStalePrimary(final ServerDescription newDescription) {
+        if (maxSetVersion == null || maxElectionId == null) {
+            return false;
         }
-        if (second == null) {
-            return 1;
-        }
-        return first.compareTo(second);
+
+        Integer setVersion = newDescription.getSetVersion();
+        return (setVersion == null || maxSetVersion.compareTo(setVersion) > 0
+                || (maxSetVersion.equals(setVersion) && maxElectionId.compareTo(newDescription.getElectionId()) > 0));
     }
 
     private boolean isNotAlreadyPrimary(final ServerAddress address) {

driver-core/src/test/resources/server-discovery-and-monitoring/rs/electionId_precedence_setVersion.json

@@ -123,18 +123,15 @@
       "outcome": {
         "servers": {
           "a:27017": {
-            "type": "Unknown",
-            "setName": null,
-            "setVersion": null,
-            "electionId": null
-          },
-          "b:27017": {
             "type": "RSPrimary",
             "setName": "rs",
             "setVersion": 1,
-            "electionId": {
-              "$oid": "000000000000000000000002"
-            }
+            "electionId": null
+          },
+          "b:27017": {
+            "type": "Unknown",
+            "setName": null,
+            "electionId": null
           },
           "c:27017": {
             "type": "Unknown",
@@ -177,18 +174,15 @@
       "outcome": {
         "servers": {
           "a:27017": {
-            "type": "Unknown",
-            "setName": null,
-            "setVersion": null,
-            "electionId": null
-          },
-          "b:27017": {
             "type": "RSPrimary",
             "setName": "rs",
             "setVersion": 1,
-            "electionId": {
-              "$oid": "000000000000000000000002"
-            }
+            "electionId": null
+          },
+          "b:27017": {
+            "type": "Unknown",
+            "setName": null,
+            "electionId": null
           },
           "c:27017": {
             "type": "Unknown",

driver-core/src/test/resources/server-discovery-and-monitoring/rs/electionId_setVersion.json

@@ -1,5 +1,5 @@
 {
-  "description": "Secondary ignored when ok is zero",
+  "description": "New primary",
   "uri": "mongodb://a,b/?replicaSet=rs",
   "phases": [
     {