SERVER-54204 Load external validation keys with local read concern

11 files changed, 35 insertions, 61 deletions

diff --git a/src/mongo/db/keys_collection_cache.cpp b/src/mongo/db/keys_collection_cache.cpp
index 87f9e798a33..5e3533ea5f9 100644
--- a/src/mongo/db/keys_collection_cache.cpp
+++ b/src/mongo/db/keys_collection_cache.cpp
@@ -113,7 +113,7 @@ Status KeysCollectionCache::_refreshExternalKeys(OperationContext* opCtx) {
         originalSize = _externalKeysCache.size();
     }
 
-    auto refreshStatus = _client->getNewExternalKeys(opCtx, _purpose, LogicalTime(), true);
+    auto refreshStatus = _client->getAllExternalKeys(opCtx, _purpose);
 
     if (!refreshStatus.isOK()) {
         return refreshStatus.getStatus();
diff --git a/src/mongo/db/keys_collection_client.h b/src/mongo/db/keys_collection_client.h
index 7bb4f16341d..4b5189ecaa9 100644
--- a/src/mongo/db/keys_collection_client.h
+++ b/src/mongo/db/keys_collection_client.h
@@ -57,15 +57,11 @@ public:
         bool useMajority) = 0;
 
     /**
-     * Returns external keys (validation-only keys copied from other clusters) that match the given
-     * purpose and have an expiresAt value greater than newerThanThis. Uses readConcern level
-     * majority if possible.
+     * Returns all external keys (validation-only keys copied from other clusters) that match the
+     * given purpose.
      */
-    virtual StatusWith<std::vector<ExternalKeysCollectionDocument>> getNewExternalKeys(
-        OperationContext* opCtx,
-        StringData purpose,
-        const LogicalTime& newerThanThis,
-        bool useMajority) = 0;
+    virtual StatusWith<std::vector<ExternalKeysCollectionDocument>> getAllExternalKeys(
+        OperationContext* opCtx, StringData purpose) = 0;
 
     /**
      * Directly inserts a key document to the storage
diff --git a/src/mongo/db/keys_collection_client_direct.cpp b/src/mongo/db/keys_collection_client_direct.cpp
index af1e90d57a5..69f6fc471c0 100644
--- a/src/mongo/db/keys_collection_client_direct.cpp
+++ b/src/mongo/db/keys_collection_client_direct.cpp
@@ -82,16 +82,16 @@ StatusWith<std::vector<KeysCollectionDocument>> KeysCollectionClientDirect::getN
 }
 
 StatusWith<std::vector<ExternalKeysCollectionDocument>>
-KeysCollectionClientDirect::getNewExternalKeys(OperationContext* opCtx,
-                                               StringData purpose,
-                                               const LogicalTime& newerThanThis,
-                                               bool useMajority) {
+KeysCollectionClientDirect::getAllExternalKeys(OperationContext* opCtx, StringData purpose) {
     return _getNewKeys<ExternalKeysCollectionDocument>(
         opCtx,
         NamespaceString::kExternalKeysCollectionNamespace,
         purpose,
-        newerThanThis,
-        useMajority);
+        LogicalTime(),
+        // It is safe to read external keys with local read concern because they are only used to
+        // validate incoming signatures, not to sign them. If a cached key is rolled back, it will
+        // eventually be reaped from the cache.
+        false /* useMajority */);
 }
 
 template <typename KeyDocumentType>
diff --git a/src/mongo/db/keys_collection_client_direct.h b/src/mongo/db/keys_collection_client_direct.h
index 9cd71aad750..c166a964760 100644
--- a/src/mongo/db/keys_collection_client_direct.h
+++ b/src/mongo/db/keys_collection_client_direct.h
@@ -55,14 +55,10 @@ public:
         bool useMajority) override;
 
     /**
-     * Returns keys in admin.system.external_validation_keys that match the given purpose and have
-     * an expiresAt value greater than newerThanThis. Uses readConcern level majority if possible.
+     * Returns all keys in admin.system.external_validation_keys that match the given purpose.
      */
-    StatusWith<std::vector<ExternalKeysCollectionDocument>> getNewExternalKeys(
-        OperationContext* opCtx,
-        StringData purpose,
-        const LogicalTime& newerThanThis,
-        bool useMajority) override;
+    StatusWith<std::vector<ExternalKeysCollectionDocument>> getAllExternalKeys(
+        OperationContext* opCtx, StringData purpose) override;
 
     /**
      * Directly inserts a key document to the storage
diff --git a/src/mongo/db/keys_collection_client_sharded.cpp b/src/mongo/db/keys_collection_client_sharded.cpp
index 7db511bcaca..8f745cd0f87 100644
--- a/src/mongo/db/keys_collection_client_sharded.cpp
+++ b/src/mongo/db/keys_collection_client_sharded.cpp
@@ -50,10 +50,7 @@ StatusWith<std::vector<KeysCollectionDocument>> KeysCollectionClientSharded::get
 }
 
 StatusWith<std::vector<ExternalKeysCollectionDocument>>
-KeysCollectionClientSharded::getNewExternalKeys(OperationContext* opCtx,
-                                                StringData purpose,
-                                                const LogicalTime& newerThanThis,
-                                                bool useMajority) {
+KeysCollectionClientSharded::getAllExternalKeys(OperationContext* opCtx, StringData purpose) {
     return std::vector<ExternalKeysCollectionDocument>{};
 }
 
diff --git a/src/mongo/db/keys_collection_client_sharded.h b/src/mongo/db/keys_collection_client_sharded.h
index 5ab20a626cd..aee2ec01962 100644
--- a/src/mongo/db/keys_collection_client_sharded.h
+++ b/src/mongo/db/keys_collection_client_sharded.h
@@ -50,15 +50,11 @@ public:
         bool useMajority) override;
 
     /**
-     * Returns validation-only keys copied from other clusters that match the given purpose
-     * and have an expiresAt value greater than newerThanThis. Uses readConcern level majority if
-     * possible. Currently, a sharded cluster never copies cluster time keys from other clusters.
+     * Returns validation-only keys copied from other clusters that match the given purpose.
+     * Currently, a sharded cluster never copies cluster time keys from other clusters.
      */
-    StatusWith<std::vector<ExternalKeysCollectionDocument>> getNewExternalKeys(
-        OperationContext* opCtx,
-        StringData purpose,
-        const LogicalTime& newerThanThis,
-        bool useMajority) override;
+    StatusWith<std::vector<ExternalKeysCollectionDocument>> getAllExternalKeys(
+        OperationContext* opCtx, StringData purpose) override;
 
     /**
      * Directly inserts a key document to the storage
diff --git a/src/mongo/db/repl/tenant_migration_donor_service.cpp b/src/mongo/db/repl/tenant_migration_donor_service.cpp
index 6f534bfc29c..fc58789db3a 100644
--- a/src/mongo/db/repl/tenant_migration_donor_service.cpp
+++ b/src/mongo/db/repl/tenant_migration_donor_service.cpp
@@ -399,7 +399,7 @@ TenantMigrationDonorService::Instance::_fetchAndStoreRecipientClusterTimeKeyDocs
         .then([this, self = shared_from_this(), executor, token](auto keyDocs) {
             checkIfReceivedDonorAbortMigration(token, _instanceCancelationSource.token());
 
-            return tenant_migration_util::storeExternalClusterTimeKeyDocsAndRefreshCache(
+            tenant_migration_util::storeExternalClusterTimeKeyDocsAndRefreshCache(
                 executor, std::move(keyDocs), _instanceCancelationSource.token());
         });
 }
diff --git a/src/mongo/db/repl/tenant_migration_recipient_service.cpp b/src/mongo/db/repl/tenant_migration_recipient_service.cpp
index e7d9d632b45..4d299b8c459 100644
--- a/src/mongo/db/repl/tenant_migration_recipient_service.cpp
+++ b/src/mongo/db/repl/tenant_migration_recipient_service.cpp
@@ -1230,8 +1230,7 @@ SharedSemiFuture<void> TenantMigrationRecipientService::Instance::_updateStateDo
         .waitUntilMajority(repl::ReplClientInfo::forClient(cc()).getLastOp());
 }
 
-ExecutorFuture<void>
-TenantMigrationRecipientService::Instance::_fetchAndStoreDonorClusterTimeKeyDocs(
+void TenantMigrationRecipientService::Instance::_fetchAndStoreDonorClusterTimeKeyDocs(
     const CancelationToken& token) {
     std::vector<ExternalKeysCollectionDocument> keyDocs;
 
@@ -1244,7 +1243,7 @@ TenantMigrationRecipientService::Instance::_fetchAndStoreDonorClusterTimeKeyDocs
             _serviceContext, _donorUri.getSetName(), doc));
     }
 
-    return tenant_migration_util::storeExternalClusterTimeKeyDocsAndRefreshCache(
+    tenant_migration_util::storeExternalClusterTimeKeyDocsAndRefreshCache(
         _scopedExecutor, std::move(keyDocs), token);
 }
 
@@ -1306,7 +1305,7 @@ SemiFuture<void> TenantMigrationRecipientService::Instance::run(
                 _stateDoc.getStartFetchingDonorOpTime().has_value());
         })
         .then([this, self = shared_from_this(), token] {
-            return _fetchAndStoreDonorClusterTimeKeyDocs(token);
+            _fetchAndStoreDonorClusterTimeKeyDocs(token);
         })
         .then([this, self = shared_from_this()] {
             _stopOrHangOnFailPoint(&fpAfterConnectingTenantMigrationRecipientInstance);
diff --git a/src/mongo/db/repl/tenant_migration_recipient_service.h b/src/mongo/db/repl/tenant_migration_recipient_service.h
index 9de9ebc73c1..0387a3f441e 100644
--- a/src/mongo/db/repl/tenant_migration_recipient_service.h
+++ b/src/mongo/db/repl/tenant_migration_recipient_service.h
@@ -310,7 +310,7 @@ public:
          * Fetches all key documents from the donor's admin.system.keys collection, stores them in
          * admin.system.external_validation_keys, and refreshes the keys cache.
          */
-        ExecutorFuture<void> _fetchAndStoreDonorClusterTimeKeyDocs(const CancelationToken& token);
+        void _fetchAndStoreDonorClusterTimeKeyDocs(const CancelationToken& token);
 
         /**
          * Retrieves the start optimes from the donor and updates the in-memory state accordingly.
diff --git a/src/mongo/db/repl/tenant_migration_util.cpp b/src/mongo/db/repl/tenant_migration_util.cpp
index 66c39f1a897..f92baa8b1e7 100644
--- a/src/mongo/db/repl/tenant_migration_util.cpp
+++ b/src/mongo/db/repl/tenant_migration_util.cpp
@@ -58,7 +58,7 @@ ExternalKeysCollectionDocument makeExternalClusterTimeKeyDoc(ServiceContext* ser
     return externalKeyDoc;
 }
 
-ExecutorFuture<void> storeExternalClusterTimeKeyDocsAndRefreshCache(
+void storeExternalClusterTimeKeyDocsAndRefreshCache(
     std::shared_ptr<executor::ScopedTaskExecutor> executor,
     std::vector<ExternalKeysCollectionDocument> keyDocs,
     const CancelationToken& token) {
@@ -88,23 +88,13 @@ ExecutorFuture<void> storeExternalClusterTimeKeyDocsAndRefreshCache(
         });
     }
 
-    const auto opTime = repl::ReplClientInfo::forClient(opCtx->getClient()).getLastOp();
-
-    return WaitForMajorityService::get(opCtx->getServiceContext())
-        .waitUntilMajority(opTime)
-        .thenRunOn(**executor)
-        .then([] {
-            auto opCtxHolder = cc().makeOperationContext();
-            auto opCtx = opCtxHolder.get();
-
-            auto validator = LogicalTimeValidator::get(opCtx);
-            if (validator) {
-                // Refresh the keys cache to avoid validation errors for external cluster times with
-                // a keyId that matches the keyId of an internal key since the LogicalTimeValidator
-                // only refreshes the cache when it cannot find a matching internal key.
-                validator->refreshKeyManagerCache(opCtx);
-            }
-        });
+    auto validator = LogicalTimeValidator::get(opCtx);
+    if (validator) {
+        // Refresh the keys cache to avoid validation errors for external cluster times with
+        // a keyId that matches the keyId of an internal key since the LogicalTimeValidator
+        // only refreshes the cache when it cannot find a matching internal key.
+        validator->refreshKeyManagerCache(opCtx);
+    }
 }
 
 void createRetryableWritesView(OperationContext* opCtx, Database* db) {
diff --git a/src/mongo/db/repl/tenant_migration_util.h b/src/mongo/db/repl/tenant_migration_util.h
index ddc26e5f7bf..e76e8464ad0 100644
--- a/src/mongo/db/repl/tenant_migration_util.h
+++ b/src/mongo/db/repl/tenant_migration_util.h
@@ -139,10 +139,10 @@ ExternalKeysCollectionDocument makeExternalClusterTimeKeyDoc(ServiceContext* ser
 /*
  * For each given ExternalKeysCollectionDocument, inserts it if there is not an existing document in
  * admin.system.external_validation_keys for it with the same keyId and replicaSetName. Otherwise,
- * updates the ttlExpiresAt of the existing document if it is less than the new ttlExpiresAt. Waits
- * for the writes to be majority-committed, and refreshes the logical validator's cache.
+ * updates the ttlExpiresAt of the existing document if it is less than the new ttlExpiresAt.
+ * Refreshes the logical validator's cache before returning.
  */
-ExecutorFuture<void> storeExternalClusterTimeKeyDocsAndRefreshCache(
+void storeExternalClusterTimeKeyDocsAndRefreshCache(
     std::shared_ptr<executor::ScopedTaskExecutor> executor,
     std::vector<ExternalKeysCollectionDocument> keyDocs,
     const CancelationToken& token);