SERVER-52587 Making collection and database instances comparable through cluster time (no more epochs)

Part 1: setting up the environment to start comparing DatabaseVersions
through timestamps.

Changelog:

- Moving the ComparableDatabaseVersion class to the DatabaseVersion
  header.
- Using ComparableDatabaseVersions in two places where we were doing
 DatabaseVersion comparsions.

5 files changed, 162 insertions, 138 deletions

diff --git a/src/mongo/db/s/shard_filtering_metadata_refresh.cpp b/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
index eb79b9d7ab0..f6a88cd8502 100644
--- a/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
+++ b/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
@@ -47,6 +47,7 @@
 #include "mongo/db/s/sharding_statistics.h"
 #include "mongo/logv2/log.h"
 #include "mongo/s/catalog_cache.h"
+#include "mongo/s/database_version.h"
 #include "mongo/s/grid.h"
 #include "mongo/util/fail_point.h"
 
@@ -66,10 +67,19 @@ void onDbVersionMismatch(OperationContext* opCtx,
 
     invariant(ShardingState::get(opCtx)->canAcceptShardedCommands());
 
-    if (serverDbVersion && serverDbVersion->getUuid() == clientDbVersion.getUuid() &&
-        serverDbVersion->getLastMod() >= clientDbVersion.getLastMod()) {
-        // The client was stale; do not trigger server-side refresh.
-        return;
+    if (serverDbVersion) {
+        // Do not reorder these two statements! if the comparison is done through epochs, the
+        // construction order matters: we are pessimistically assuming that the client version
+        // is newer when they have different uuids
+        const ComparableDatabaseVersion comparableServerDbVersion =
+            ComparableDatabaseVersion::makeComparableDatabaseVersion(*serverDbVersion);
+        const ComparableDatabaseVersion comparableClientDbVersion =
+            ComparableDatabaseVersion::makeComparableDatabaseVersion(clientDbVersion);
+
+        if (comparableClientDbVersion <= comparableServerDbVersion) {
+            // The client was stale; do not trigger server-side refresh.
+            return;
+        }
     }
 
     // Ensure any ongoing movePrimary's have completed before trying to do the refresh. This wait is
@@ -501,19 +511,29 @@ void forceDatabaseRefresh(OperationContext* opCtx, const StringData dbName) {
         auto dssLock = DatabaseShardingState::DSSLock::lockShared(opCtx, dss);
 
         const auto cachedDbVersion = dss->getDbVersion(opCtx, dssLock);
-        if (cachedDbVersion && cachedDbVersion->getUuid() == refreshedDbVersion.getUuid() &&
-            cachedDbVersion->getLastMod() >= refreshedDbVersion.getLastMod()) {
-            LOGV2_DEBUG(22066,
-                        2,
-                        "Skipping setting cached databaseVersion for {db} to refreshed version "
-                        "{refreshedDbVersion} because current cached databaseVersion is already "
-                        "{cachedDbVersion}",
-                        "Skipping setting cached databaseVersion to refreshed version "
-                        "because current cached databaseVersion is more recent",
-                        "db"_attr = dbName,
-                        "refreshedDbVersion"_attr = refreshedDbVersion.toBSON(),
-                        "cachedDbVersion"_attr = cachedDbVersion->toBSON());
-            return;
+        if (cachedDbVersion) {
+            // Do not reorder these two statements! if the comparison is done through epochs, the
+            // construction order matters: we are pessimistically assuming that the client version
+            // is newer when they have different uuids
+            const ComparableDatabaseVersion comparableCachedDbVersion =
+                ComparableDatabaseVersion::makeComparableDatabaseVersion(*cachedDbVersion);
+            const ComparableDatabaseVersion comparableRefreshedDbVersion =
+                ComparableDatabaseVersion::makeComparableDatabaseVersion(refreshedDbVersion);
+
+            if (comparableRefreshedDbVersion <= comparableCachedDbVersion) {
+                LOGV2_DEBUG(
+                    22066,
+                    2,
+                    "Skipping setting cached databaseVersion for {db} to refreshed version "
+                    "{refreshedDbVersion} because current cached databaseVersion is already "
+                    "{cachedDbVersion}",
+                    "Skipping setting cached databaseVersion to refreshed version "
+                    "because current cached databaseVersion is more recent",
+                    "db"_attr = dbName,
+                    "refreshedDbVersion"_attr = refreshedDbVersion.toBSON(),
+                    "cachedDbVersion"_attr = cachedDbVersion->toBSON());
+                return;
+            }
         }
     }
 
diff --git a/src/mongo/s/catalog_cache.cpp b/src/mongo/s/catalog_cache.cpp
index 1bd781bb674..1c93b581c9b 100644
--- a/src/mongo/s/catalog_cache.cpp
+++ b/src/mongo/s/catalog_cache.cpp
@@ -639,47 +639,6 @@ CatalogCache::CollectionCache::LookupResult CatalogCache::CollectionCache::_look
     }
 }
 
-AtomicWord<uint64_t> ComparableDatabaseVersion::_uuidDisambiguatingSequenceNumSource{1ULL};
-
-ComparableDatabaseVersion ComparableDatabaseVersion::makeComparableDatabaseVersion(
-    const DatabaseVersion& version) {
-    return ComparableDatabaseVersion(version, _uuidDisambiguatingSequenceNumSource.fetchAndAdd(1));
-}
-
-BSONObj ComparableDatabaseVersion::toBSONForLogging() const {
-    BSONObjBuilder builder;
-    if (_dbVersion)
-        builder.append("dbVersion"_sd, _dbVersion->toBSON());
-    else
-        builder.append("dbVersion"_sd, "None");
-
-    builder.append("uuidDisambiguatingSequenceNum"_sd,
-                   static_cast<int64_t>(_uuidDisambiguatingSequenceNum));
-
-    return builder.obj();
-}
-
-
-bool ComparableDatabaseVersion::operator==(const ComparableDatabaseVersion& other) const {
-    if (!_dbVersion && !other._dbVersion)
-        return true;  // Default constructed value
-    if (_dbVersion.is_initialized() != other._dbVersion.is_initialized())
-        return false;  // One side is default constructed value
-
-    return sameUuid(other) && (_dbVersion->getLastMod() == other._dbVersion->getLastMod());
-}
-
-bool ComparableDatabaseVersion::operator<(const ComparableDatabaseVersion& other) const {
-    if (!_dbVersion && !other._dbVersion)
-        return false;  // Default constructed value
-
-    if (_dbVersion && other._dbVersion && sameUuid(other)) {
-        return _dbVersion->getLastMod() < other._dbVersion->getLastMod();
-    } else {
-        return _uuidDisambiguatingSequenceNum < other._uuidDisambiguatingSequenceNum;
-    }
-}
-
 CachedDatabaseInfo::CachedDatabaseInfo(DatabaseType dbt) : _dbt(std::move(dbt)) {}
 
 const ShardId& CachedDatabaseInfo::primaryId() const {
diff --git a/src/mongo/s/catalog_cache.h b/src/mongo/s/catalog_cache.h
index 656d163f26c..bf61fcb1f54 100644
--- a/src/mongo/s/catalog_cache.h
+++ b/src/mongo/s/catalog_cache.h
@@ -45,86 +45,6 @@ class BSONObjBuilder;
 static constexpr int kMaxNumStaleVersionRetries = 10;
 
 /**
- * Constructed exclusively by the CatalogCache to be used as vector clock (Time) to drive
- * DatabaseCache's refreshes.
- *
- * The DatabaseVersion class contains a UUID that is not comparable,
- * in fact is impossible to compare two different DatabaseVersion that have different UUIDs.
- *
- * This class wrap a DatabaseVersion object to make it always comparable by timestamping it with a
- * node-local sequence number (_uuidDisambiguatingSequenceNum).
- *
- * This class class should go away once a cluster-wide comparable DatabaseVersion will be
- * implemented.
- */
-class ComparableDatabaseVersion {
-public:
-    /**
-     * Creates a ComparableDatabaseVersion that wraps the given DatabaseVersion.
-     * Each object created through this method will have a local sequence number greater than the
-     * previously created ones.
-     */
-    static ComparableDatabaseVersion makeComparableDatabaseVersion(const DatabaseVersion& version);
-
-    /**
-     * Empty constructor needed by the ReadThroughCache.
-     *
-     * Instances created through this constructor will be always less then the ones created through
-     * the static constructor.
-     */
-    ComparableDatabaseVersion() = default;
-
-    const DatabaseVersion& getVersion() const {
-        return *_dbVersion;
-    }
-
-    BSONObj toBSONForLogging() const;
-
-    bool sameUuid(const ComparableDatabaseVersion& other) const {
-        return _dbVersion->getUuid() == other._dbVersion->getUuid();
-    }
-
-    bool operator==(const ComparableDatabaseVersion& other) const;
-
-    bool operator!=(const ComparableDatabaseVersion& other) const {
-        return !(*this == other);
-    }
-
-    /**
-     * In case the two compared instances have different UUIDs, the most recently created one will
-     * be greater, otherwise the comparison will be driven by the lastMod field of the underlying
-     * DatabaseVersion.
-     */
-    bool operator<(const ComparableDatabaseVersion& other) const;
-
-    bool operator>(const ComparableDatabaseVersion& other) const {
-        return other < *this;
-    }
-
-    bool operator<=(const ComparableDatabaseVersion& other) const {
-        return !(*this > other);
-    }
-
-    bool operator>=(const ComparableDatabaseVersion& other) const {
-        return !(*this < other);
-    }
-
-private:
-    static AtomicWord<uint64_t> _uuidDisambiguatingSequenceNumSource;
-
-    ComparableDatabaseVersion(const DatabaseVersion& version,
-                              uint64_t uuidDisambiguatingSequenceNum)
-        : _dbVersion(version), _uuidDisambiguatingSequenceNum(uuidDisambiguatingSequenceNum) {}
-
-    boost::optional<DatabaseVersion> _dbVersion;
-
-    // Locally incremented sequence number that allows to compare two database versions with
-    // different UUIDs. Each new comparableDatabaseVersion will have a greater sequence number then
-    // the ones created before.
-    uint64_t _uuidDisambiguatingSequenceNum{0};
-};
-
-/**
  * Constructed exclusively by the CatalogCache, contains a reference to the cached information for
  * the specified database.
  */
diff --git a/src/mongo/s/database_version.cpp b/src/mongo/s/database_version.cpp
index 53da1268f97..e2043e39299 100644
--- a/src/mongo/s/database_version.cpp
+++ b/src/mongo/s/database_version.cpp
@@ -33,6 +33,8 @@
 
 namespace mongo {
 
+AtomicWord<uint64_t> ComparableDatabaseVersion::_uuidDisambiguatingSequenceNumSource{1ULL};
+
 DatabaseVersion DatabaseVersion::makeFixed() {
     DatabaseVersion dbVersion;
     dbVersion.setLastMod(0);
@@ -45,4 +47,42 @@ DatabaseVersion DatabaseVersion::makeUpdated() const {
     return newVersion;
 }
 
+ComparableDatabaseVersion ComparableDatabaseVersion::makeComparableDatabaseVersion(
+    const DatabaseVersion& version) {
+    return ComparableDatabaseVersion(version, _uuidDisambiguatingSequenceNumSource.fetchAndAdd(1));
+}
+
+BSONObj ComparableDatabaseVersion::toBSONForLogging() const {
+    BSONObjBuilder builder;
+    if (_dbVersion)
+        builder.append("dbVersion"_sd, _dbVersion->toBSON());
+    else
+        builder.append("dbVersion"_sd, "None");
+
+    builder.append("uuidDisambiguatingSequenceNum"_sd,
+                   static_cast<int64_t>(_uuidDisambiguatingSequenceNum));
+
+    return builder.obj();
+}
+
+bool ComparableDatabaseVersion::operator==(const ComparableDatabaseVersion& other) const {
+    if (!_dbVersion && !other._dbVersion)
+        return true;  // Default constructed value
+    if (_dbVersion.is_initialized() != other._dbVersion.is_initialized())
+        return false;  // One side is default constructed value
+
+    return *_dbVersion == *other._dbVersion;
+}
+
+bool ComparableDatabaseVersion::operator<(const ComparableDatabaseVersion& other) const {
+    if (!_dbVersion && !other._dbVersion)
+        return false;  // Default constructed value
+
+    if (_dbVersion && other._dbVersion && _dbVersion->getUuid() == other._dbVersion->getUuid()) {
+        return _dbVersion->getLastMod() < other._dbVersion->getLastMod();
+    } else {
+        return _uuidDisambiguatingSequenceNum < other._uuidDisambiguatingSequenceNum;
+    }
+}
+
 }  // namespace mongo
diff --git a/src/mongo/s/database_version.h b/src/mongo/s/database_version.h
index 3daac2d53b8..adaa1ff3963 100644
--- a/src/mongo/s/database_version.h
+++ b/src/mongo/s/database_version.h
@@ -33,6 +33,16 @@
 
 namespace mongo {
 
+/**
+ * This class is used to represent a specific version of a Database.
+ *
+ * Currently it is implemented as a (uuid, [timestamp,] lastMod) triplet, where the
+ * timestamp is optional in versions prior 4.9. The uuid is going to be removed soon,
+ * since they are not comparable (that's the reason why there is a ComparableDatabaseVersion class).
+ *
+ * Once uuids are gone, relational operators should be implemented in this class.
+ *
+ */
 class DatabaseVersion : private DatabaseVersionBase {
 public:
     using DatabaseVersionBase::getLastMod;
@@ -83,4 +93,79 @@ public:
     }
 };
 
+
+/**
+ * The DatabaseVersion class contains a UUID that is not comparable,
+ * in fact is impossible to compare two different DatabaseVersion that have different UUIDs.
+ *
+ * This class wrap a DatabaseVersion object to make it always comparable by timestamping it with a
+ * node-local sequence number (_uuidDisambiguatingSequenceNum).
+ *
+ * This class class should go away once a cluster-wide comparable DatabaseVersion will be
+ * implemented.
+ */
+class ComparableDatabaseVersion {
+public:
+    /**
+     * Creates a ComparableDatabaseVersion that wraps the given DatabaseVersion.
+     * Each object created through this method will have a local sequence number greater than the
+     * previously created ones.
+     */
+    static ComparableDatabaseVersion makeComparableDatabaseVersion(const DatabaseVersion& version);
+
+    /**
+     * Empty constructor needed by the ReadThroughCache.
+     *
+     * Instances created through this constructor will be always less then the ones created through
+     * the static constructor.
+     */
+    ComparableDatabaseVersion() = default;
+
+    const DatabaseVersion& getVersion() const {
+        return *_dbVersion;
+    }
+
+    BSONObj toBSONForLogging() const;
+
+    bool operator==(const ComparableDatabaseVersion& other) const;
+
+    bool operator!=(const ComparableDatabaseVersion& other) const {
+        return !(*this == other);
+    }
+
+    /**
+     * In case the two compared instances have different UUIDs, the most recently created one will
+     * be greater, otherwise the comparison will be driven by the lastMod field of the underlying
+     * DatabaseVersion.
+     */
+    bool operator<(const ComparableDatabaseVersion& other) const;
+
+    bool operator>(const ComparableDatabaseVersion& other) const {
+        return other < *this;
+    }
+
+    bool operator<=(const ComparableDatabaseVersion& other) const {
+        return !(*this > other);
+    }
+
+    bool operator>=(const ComparableDatabaseVersion& other) const {
+        return !(*this < other);
+    }
+
+private:
+    static AtomicWord<uint64_t> _uuidDisambiguatingSequenceNumSource;
+
+    ComparableDatabaseVersion(const DatabaseVersion& version,
+                              uint64_t uuidDisambiguatingSequenceNum)
+        : _dbVersion(version), _uuidDisambiguatingSequenceNum(uuidDisambiguatingSequenceNum) {}
+
+    boost::optional<DatabaseVersion> _dbVersion;
+
+    // Locally incremented sequence number that allows to compare two database versions with
+    // different UUIDs. Each new comparableDatabaseVersion will have a greater sequence number then
+    // the ones created before.
+    uint64_t _uuidDisambiguatingSequenceNum{0};
+};
+
+
 }  // namespace mongo