SERVER-45950 TopologyManager should not use idl generated comparison operator to compare TopologyVersion

4 files changed, 35 insertions, 26 deletions

diff --git a/src/mongo/client/sdam/server_description.cpp b/src/mongo/client/sdam/server_description.cpp
index 06c395e232c..8d919251054 100644
--- a/src/mongo/client/sdam/server_description.cpp
+++ b/src/mongo/client/sdam/server_description.cpp
@@ -26,9 +26,9 @@
  *    exception statement from all source files in the program, then also delete
  *    it in the license file.
  */
+#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kNetwork
 
 #include "mongo/client/sdam/server_description.h"
-#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kNetwork
 
 #include <algorithm>
 #include <boost/algorithm/string.hpp>
@@ -293,6 +293,15 @@ bool ServerDescription::isStreamable() const {
 }
 
 bool ServerDescription::isEquivalent(const ServerDescription& other) const {
+    if (_topologyVersion && other._topologyVersion &&
+        ((_topologyVersion->getProcessId() != other._topologyVersion->getProcessId()) ||
+         (_topologyVersion->getCounter() != other._topologyVersion->getCounter()))) {
+        return false;
+    } else if ((!_topologyVersion && other._topologyVersion) ||
+               (_topologyVersion && !other._topologyVersion)) {
+        return false;
+    }
+
     auto otherValues = std::tie(other._type,
                                 other._minWireVersion,
                                 other._maxWireVersion,
@@ -306,7 +315,6 @@ bool ServerDescription::isEquivalent(const ServerDescription& other) const {
                                 other._electionId,
                                 other._primary,
                                 other._logicalSessionTimeoutMinutes,
-                                other._topologyVersion,
                                 other._streamable,
                                 other._poolResetCounter);
     auto thisValues = std::tie(_type,
@@ -322,7 +330,6 @@ bool ServerDescription::isEquivalent(const ServerDescription& other) const {
                                _electionId,
                                _primary,
                                _logicalSessionTimeoutMinutes,
-                               _topologyVersion,
                                _streamable,
                                _poolResetCounter);
     return thisValues == otherValues;
diff --git a/src/mongo/client/sdam/topology_description_test.cpp b/src/mongo/client/sdam/topology_description_test.cpp
index 3f580a9d415..b5aca529bfd 100644
--- a/src/mongo/client/sdam/topology_description_test.cpp
+++ b/src/mongo/client/sdam/topology_description_test.cpp
@@ -304,7 +304,8 @@ TEST_F(TopologyDescriptionTestFixture, ShouldUpdateTopologyVersionOnSuccess) {
     topologyDescription.installServerDescription(newDescription);
     ASSERT_EQUALS(topologyDescription.getServers().size(), 3);
     auto topologyVersion = topologyDescription.getServers()[1]->getTopologyVersion();
-    ASSERT(topologyVersion == TopologyVersion(processId, 1));
+    ASSERT(topologyVersion->getProcessId() == processId);
+    ASSERT(topologyVersion->getCounter() == 1);
 }
 
 TEST_F(TopologyDescriptionTestFixture, ShouldNotUpdateTopologyVersionOnError) {
diff --git a/src/mongo/client/sdam/topology_manager.cpp b/src/mongo/client/sdam/topology_manager.cpp
index 9cad8440f57..656ec9bde4d 100644
--- a/src/mongo/client/sdam/topology_manager.cpp
+++ b/src/mongo/client/sdam/topology_manager.cpp
@@ -38,12 +38,21 @@ namespace mongo::sdam {
 
 namespace {
 
-// Compare topologyVersions. If the isMaster response has topologyVersion < lastServerDescription's
-// toplogyVersion, we will ignore this reply because the lastServerDescription is fresher.
+/* Compare topologyVersions to determine if the isMaster response's topologyVersion is stale
+ * according to the following rules:
+ * 1. If the response's topologyVersion is unset or the lastServerDescription's topologyVersion is
+ * null, the client MUST assume the response is more recent.
+ * 2. If the response’s topologyVersion.processId != to lastServerDescription's, the client MUST
+ * assume the response is more recent.
+ * 3. If the response’s topologyVersion.processId == to lastServerDescription's and response's
+ * topologyVersion.counter < lastServerDescription's topologyVersion.counter, the client MUST ignore
+ * this reply because the lastServerDescription is fresher.
+ */
 bool isStaleTopologyVersion(boost::optional<TopologyVersion> lastTopologyVersion,
                             boost::optional<TopologyVersion> newTopologyVersion) {
     if (lastTopologyVersion && newTopologyVersion &&
-        (lastTopologyVersion.get() > newTopologyVersion.get())) {
+        ((lastTopologyVersion->getProcessId() == newTopologyVersion->getProcessId()) &&
+         (lastTopologyVersion->getCounter() > newTopologyVersion->getCounter()))) {
         return true;
     }
 
@@ -74,27 +83,20 @@ void TopologyManager::onServerDescription(const IsMasterOutcome& isMasterOutcome
     }
 
     boost::optional<TopologyVersion> newTopologyVersion = isMasterOutcome.getTopologyVersion();
-    boost::optional<int> poolResetCounter = boost::none;
-    if (isMasterOutcome.isSuccess()) {
-        if (isStaleTopologyVersion(lastTopologyVersion, newTopologyVersion)) {
-            LOGV2(20218,
-                  "Ignoring this isMaster response because our topologyVersion: "
-                  "{lastTopologyVersion}is fresher than the provided topologyVersion: "
-                  "{newTopologyVersion}",
-                  "lastTopologyVersion"_attr = lastTopologyVersion->toBSON(),
-                  "newTopologyVersion"_attr = newTopologyVersion->toBSON());
-            return;
-        }
-
-        // Maintain the poolResetCounter.
-        poolResetCounter = lastPoolResetCounter;
-    } else {
+    if (isStaleTopologyVersion(lastTopologyVersion, newTopologyVersion)) {
+        log() << "Ignoring this isMaster response because our topologyVersion: "
+              << lastTopologyVersion->toBSON()
+              << "is fresher than the provided topologyVersion: " << newTopologyVersion->toBSON();
+        return;
+    }
+
+    boost::optional<int> poolResetCounter = lastPoolResetCounter;
+    if (!isMasterOutcome.isSuccess() && lastPoolResetCounter) {
         // Bump the poolResetCounter on error if we have one established already.
-        if (lastServerDescription) {
-            poolResetCounter = ++lastPoolResetCounter.get();
-        }
+        poolResetCounter = ++lastPoolResetCounter.get();
     }
 
+    // newTopologyVersion will be null if the isMaster response did not provide one.
     auto newServerDescription = std::make_shared<ServerDescription>(
         _clockSource, isMasterOutcome, lastRTT, newTopologyVersion, poolResetCounter);
 
diff --git a/src/mongo/rpc/topology_version.idl b/src/mongo/rpc/topology_version.idl
index d2b51f5f56c..a9a732c36b3 100644
--- a/src/mongo/rpc/topology_version.idl
+++ b/src/mongo/rpc/topology_version.idl
@@ -37,7 +37,6 @@ structs:
     TopologyVersion:
         description: "Clients and servers exchange topologyVersions to determine the relative
                      freshness of topology information in concurrent messages."
-        generate_comparison_operators: true
         fields:
             processId:
                 type: objectid