SERVER-60268 Reintroduce disambiguationg sequence number

4 files changed, 65 insertions, 12 deletions

diff --git a/src/mongo/s/catalog_cache.cpp b/src/mongo/s/catalog_cache.cpp
index d786da676f9..ed6283dacf7 100644
--- a/src/mongo/s/catalog_cache.cpp
+++ b/src/mongo/s/catalog_cache.cpp
@@ -73,6 +73,7 @@ const OperationContext::Decoration<bool> operationBlockedBehindCatalogCacheRefre
 
 }  // namespace
 
+AtomicWord<uint64_t> ComparableDatabaseVersion::_disambiguatingSequenceNumSource{1ULL};
 AtomicWord<uint64_t> ComparableDatabaseVersion::_forcedRefreshSequenceNumSource{1ULL};
 
 CachedDatabaseInfo::CachedDatabaseInfo(DatabaseTypeValueHandle&& dbt) : _dbt(std::move(dbt)){};
@@ -95,12 +96,15 @@ DatabaseVersion CachedDatabaseInfo::databaseVersion() const {
 
 ComparableDatabaseVersion ComparableDatabaseVersion::makeComparableDatabaseVersion(
     const boost::optional<DatabaseVersion>& version) {
-    return ComparableDatabaseVersion(version, _forcedRefreshSequenceNumSource.load());
+    return ComparableDatabaseVersion(version,
+                                     _disambiguatingSequenceNumSource.fetchAndAdd(1),
+                                     _forcedRefreshSequenceNumSource.load());
 }
 
 ComparableDatabaseVersion
 ComparableDatabaseVersion::makeComparableDatabaseVersionForForcedRefresh() {
     return ComparableDatabaseVersion(boost::none /* version */,
+                                     _disambiguatingSequenceNumSource.fetchAndAdd(1),
                                      _forcedRefreshSequenceNumSource.addAndFetch(2) - 1);
 }
 
@@ -115,6 +119,9 @@ BSONObj ComparableDatabaseVersion::toBSONForLogging() const {
     else
         builder.append("dbVersion"_sd, "None");
 
+    builder.append("disambiguatingSequenceNum"_sd,
+                   static_cast<int64_t>(_disambiguatingSequenceNum));
+
     builder.append("forcedRefreshSequenceNum"_sd, static_cast<int64_t>(_forcedRefreshSequenceNum));
 
     return builder.obj();
@@ -142,7 +149,13 @@ bool ComparableDatabaseVersion::operator<(const ComparableDatabaseVersion& other
     if (_forcedRefreshSequenceNum == 0)
         return false;  // Only default constructed values have _forcedRefreshSequenceNum == 0 and
                        // they are always equal
-    return _dbVersion < other._dbVersion;
+
+    // If both versions are valid we rely on the underlying DatabaseVersion comparison
+    if (_dbVersion && other._dbVersion)
+        return _dbVersion < other._dbVersion;
+
+    // Finally, we do a disambiguating sequence number comparison
+    return _disambiguatingSequenceNum < other._disambiguatingSequenceNum;
 }
 
 CatalogCache::CatalogCache(ServiceContext* const service, CatalogCacheLoader& cacheLoader)
diff --git a/src/mongo/s/catalog_cache.h b/src/mongo/s/catalog_cache.h
index 9dd31c65fa7..a50f010984e 100644
--- a/src/mongo/s/catalog_cache.h
+++ b/src/mongo/s/catalog_cache.h
@@ -71,8 +71,9 @@ private:
 };
 
 /**
- * This class wrap a DatabaseVersion object augmenting it with a sequence number to allow for forced
- * catalog cache refreshes.
+ * This class wrap a DatabaseVersion object augmenting it with:
+ *  - a sequence number to allow for forced catalog cache refreshes
+ *  - a sequence number to disambiguate scenarios in which the DatabaseVersion isn't valid
  */
 class ComparableDatabaseVersion {
 public:
@@ -80,8 +81,8 @@ public:
      * Creates a ComparableDatabaseVersion that wraps the given DatabaseVersion.
      *
      * If version is boost::none it creates a ComparableDatabaseVersion that doesn't have a valid
-     * DatabaseVersion. This is useful in some scenarios in which the DatabaseVersion is provided
-     * later through ComparableDatabaseVersion::setVersion.
+     * version. This is useful in some scenarios in which the DatabaseVersion is provided later
+     * through ComparableDatabaseVersion::setVersion or to represent that a Database doesn't exist
      */
     static ComparableDatabaseVersion makeComparableDatabaseVersion(
         const boost::optional<DatabaseVersion>& version);
@@ -128,16 +129,21 @@ public:
 private:
     friend class CatalogCache;
 
+    static AtomicWord<uint64_t> _disambiguatingSequenceNumSource;
     static AtomicWord<uint64_t> _forcedRefreshSequenceNumSource;
 
     ComparableDatabaseVersion(boost::optional<DatabaseVersion> version,
+                              uint64_t disambiguatingSequenceNum,
                               uint64_t forcedRefreshSequenceNum)
-        : _dbVersion(std::move(version)), _forcedRefreshSequenceNum(forcedRefreshSequenceNum) {}
+        : _dbVersion(std::move(version)),
+          _disambiguatingSequenceNum(disambiguatingSequenceNum),
+          _forcedRefreshSequenceNum(forcedRefreshSequenceNum) {}
 
     void setDatabaseVersion(const DatabaseVersion& version);
 
     boost::optional<DatabaseVersion> _dbVersion;
 
+    uint64_t _disambiguatingSequenceNum{0};
     uint64_t _forcedRefreshSequenceNum{0};
 };
 
diff --git a/src/mongo/s/catalog_cache_test.cpp b/src/mongo/s/catalog_cache_test.cpp
index 0106cf4b393..818cf2d38b0 100644
--- a/src/mongo/s/catalog_cache_test.cpp
+++ b/src/mongo/s/catalog_cache_test.cpp
@@ -211,6 +211,34 @@ TEST_F(CatalogCacheTest, GetCachedDatabase) {
     ASSERT_EQ(cachedDb.databaseVersion().getLastMod(), dbVersion.getLastMod());
 }
 
+TEST_F(CatalogCacheTest, GetDatabaseDrop) {
+    const auto dbName = "testDB";
+    const auto dbVersion = DatabaseVersion(UUID::gen(), Timestamp());
+
+    _catalogCacheLoader->setDatabaseRefreshReturnValue(
+        DatabaseType(dbName, kShards[0], true, dbVersion));
+
+    // The CatalogCache doesn't have any valid info about this DB and finds a new DatabaseType
+    auto swDatabase = _catalogCache->getDatabase(operationContext(), dbName);
+    ASSERT_OK(swDatabase.getStatus());
+    const auto cachedDb = swDatabase.getValue();
+    ASSERT_TRUE(cachedDb.shardingEnabled());
+    ASSERT_EQ(cachedDb.databaseVersion().getUuid(), dbVersion.getUuid());
+    ASSERT_EQ(cachedDb.databaseVersion().getLastMod(), dbVersion.getLastMod());
+
+    // Advancing the timeInStore, e.g. because of a movePrimary
+    _catalogCache->onStaleDatabaseVersion(dbName, dbVersion.makeUpdated());
+
+    // However, when this CatalogCache asks to the loader for the new info associated to dbName it
+    // didn't find any (i.e. the database was dropped)
+    _catalogCacheLoader->setDatabaseRefreshReturnValue(
+        Status(ErrorCodes::NamespaceNotFound, "dummy errmsg"));
+
+    // Finally, the CatalogCache shouldn't find the Database
+    swDatabase = _catalogCache->getDatabase(operationContext(), dbName);
+    ASSERT_EQUALS(ErrorCodes::NamespaceNotFound, swDatabase.getStatus());
+}
+
 TEST_F(CatalogCacheTest, InvalidateSingleDbOnShardRemoval) {
     const auto dbName = "testDB";
     const auto dbVersion = DatabaseVersion(UUID::gen(), Timestamp());
diff --git a/src/mongo/s/comparable_database_version_test.cpp b/src/mongo/s/comparable_database_version_test.cpp
index d1a499b26f6..ff073db5042 100644
--- a/src/mongo/s/comparable_database_version_test.cpp
+++ b/src/mongo/s/comparable_database_version_test.cpp
@@ -131,11 +131,17 @@ TEST(ComparableDatabaseVersionTest, CompareTwoForcedRefreshVersions) {
     ASSERT_FALSE(forcedRefreshVersion1 > forcedRefreshVersion2);
 }
 
-TEST(ComparableDatabaseVersionTest, CompareTwoComparableChunkVersionsWithBoostNone) {
-    const auto version1 = ComparableDatabaseVersion::makeComparableDatabaseVersion(boost::none);
-    const auto version2 = ComparableDatabaseVersion::makeComparableDatabaseVersion(boost::none);
-
-    ASSERT_TRUE(version1 == version2);
+TEST(ComparableDatabaseVersionTest, CompareVersionsAgainstBoostNone) {
+    auto checkGreatherThan = [](const boost::optional<DatabaseVersion>& v1,
+                                const boost::optional<DatabaseVersion>& v2) {
+        const auto version1 = ComparableDatabaseVersion::makeComparableDatabaseVersion(v1);
+        const auto version2 = ComparableDatabaseVersion::makeComparableDatabaseVersion(v2);
+        ASSERT_TRUE(version1 < version2);
+    };
+    const DatabaseVersion v(UUID::gen(), Timestamp(42));
+    checkGreatherThan(boost::none, v);
+    checkGreatherThan(v, boost::none);
+    checkGreatherThan(boost::none, boost::none);
 }
 
 }  // namespace