Revert "SERVER-30147 Remove some unused calls around ShardServerCatalogCacheLoader"

This reverts commit 0f5a95b31d59a15d66be97ed1ac2a8cd89b1b6cb.

3 files changed, 26 insertions, 25 deletions

diff --git a/src/mongo/db/s/shard_metadata_util.cpp b/src/mongo/db/s/shard_metadata_util.cpp
index e39983ffd42..1d73bbed89f 100644
--- a/src/mongo/db/s/shard_metadata_util.cpp
+++ b/src/mongo/db/s/shard_metadata_util.cpp
@@ -26,8 +26,6 @@
  *    it in the license file.
  */
 
-#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kSharding
-
 #include "mongo/platform/basic.h"
 
 #include "mongo/db/s/shard_metadata_util.h"
@@ -43,7 +41,6 @@
 #include "mongo/s/write_ops/batched_command_request.h"
 #include "mongo/s/write_ops/batched_command_response.h"
 #include "mongo/stdx/memory.h"
-#include "mongo/util/log.h"
 
 namespace mongo {
 namespace shardmetadatautil {
@@ -404,7 +401,6 @@ Status dropChunksAndDeleteCollectionsEntry(OperationContext* opCtx, const Namesp
             }
         }
 
-        LOG(1) << "Successfully cleared persisted chunk metadata for collection '" << nss << "'.";
         return Status::OK();
     } catch (const DBException& ex) {
         return ex.toStatus();
diff --git a/src/mongo/db/s/shard_server_catalog_cache_loader.cpp b/src/mongo/db/s/shard_server_catalog_cache_loader.cpp
index 1655e48e410..efd4ac61500 100644
--- a/src/mongo/db/s/shard_server_catalog_cache_loader.cpp
+++ b/src/mongo/db/s/shard_server_catalog_cache_loader.cpp
@@ -43,6 +43,7 @@
 #include "mongo/s/config_server_catalog_cache_loader.h"
 #include "mongo/s/grid.h"
 #include "mongo/stdx/memory.h"
+#include "mongo/util/concurrency/thread_pool.h"
 #include "mongo/util/log.h"
 
 namespace mongo {
@@ -639,14 +640,13 @@ Status ShardServerCatalogCacheLoader::_scheduleTask(const NamespaceString& nss,
 void ShardServerCatalogCacheLoader::_runTasks(const NamespaceString& nss) {
     auto context = _contexts.makeOperationContext(*Client::getCurrent());
 
+    // Run task
     bool taskFinished = false;
     try {
-        _updatePersistedMetadata(context.opCtx(), nss);
-        taskFinished = true;
+        taskFinished = _updatePersistedMetadata(context.opCtx(), nss);
     } catch (const DBException& ex) {
+        // This thread must stop if we are shutting down.
         Status exceptionStatus = ex.toStatus();
-
-        // This thread must stop if we are shutting down
         if (ErrorCodes::isShutdownError(exceptionStatus.code())) {
             log() << "Failed to persist chunk metadata update for collection '" << nss
                   << "' due to shutdown.";
@@ -660,6 +660,7 @@ void ShardServerCatalogCacheLoader::_runTasks(const NamespaceString& nss) {
 
     // If task completed successfully, remove it from work queue
     if (taskFinished) {
+        invariant(!_taskLists[nss].empty());
         _taskLists[nss].removeActiveTask();
     }
 
@@ -678,21 +679,24 @@ void ShardServerCatalogCacheLoader::_runTasks(const NamespaceString& nss) {
     }
 }
 
-void ShardServerCatalogCacheLoader::_updatePersistedMetadata(OperationContext* opCtx,
+bool ShardServerCatalogCacheLoader::_updatePersistedMetadata(OperationContext* opCtx,
                                                              const NamespaceString& nss) {
     stdx::unique_lock<stdx::mutex> lock(_mutex);
 
-    const Task& task = _taskLists[nss].getActiveTask();
+    invariant(!_taskLists[nss].empty());
+    const Task task = _taskLists[nss].getActiveTask();
     invariant(task.dropped || !task.collectionAndChangedChunks->changedChunks.empty());
 
     // If this task is from an old term and no longer valid, do not execute and return true so that
     // the task gets removed from the task list
     if (task.termCreated != _term) {
-        return;
+        return true;
     }
 
     lock.unlock();
 
+    // Check if this is a drop task.
+
     if (task.dropped) {
         // The namespace was dropped. The persisted metadata for the collection must be cleared.
         Status status = dropChunksAndDeleteCollectionsEntry(opCtx, nss);
@@ -703,9 +707,13 @@ void ShardServerCatalogCacheLoader::_updatePersistedMetadata(OperationContext* o
                               << status.reason()
                               << "'. Will be retried.",
                 status.isOK());
-        return;
+
+        LOG(1) << "Successfully cleared persisted chunk metadata for collection '" << nss << "'.";
+        return true;
     }
 
+    // This is an update task.
+
     ChunkVersion persistedMaxVersion = getPersistedMaxVersion(opCtx, nss);
 
     // If the epoch of the update task does not match the persisted metadata, the persisted metadata
@@ -725,14 +733,10 @@ void ShardServerCatalogCacheLoader::_updatePersistedMetadata(OperationContext* o
     Status status =
         persistCollectionAndChangedChunks(opCtx, nss, task.collectionAndChangedChunks.get());
     if (status == ErrorCodes::ConflictingOperationInProgress) {
-        // A new epoch was discovered while updating the persisted metadata. The getChunksSince
-        // which enqueued this task would have discovered that independently and also returned
-        // ConflictingOperationInProgress to the catalog cache, which means that the next enqueued
-        // task should have the new epoch, which in turn means that on the next invocation, the
-        // old collection entry will be dropped and recreated.
-        return;
+        // A new epoch was discovered in the new chunks. The CatalogCache will retry refreshing the
+        // chunk metadata: clearing the persisted metadata will be handled then.
+        return true;
     }
-
     uassert(status.code(),
             str::stream() << "Failed to update the persisted chunk metadata for collection '"
                           << nss.ns()
@@ -747,6 +751,7 @@ void ShardServerCatalogCacheLoader::_updatePersistedMetadata(OperationContext* o
 
     LOG(1) << "Successfully updated persisted chunk metadata for collection '" << nss << "' from '"
            << task.minQueryVersion << "' to collection version '" << task.maxQueryVersion << "'.";
+    return true;
 }
 
 StatusWith<CollectionAndChangedChunks>
@@ -806,8 +811,11 @@ ShardServerCatalogCacheLoader::_getCompletePersistedMetadataForSecondarySinceVer
 ShardServerCatalogCacheLoader::Task::Task(
     StatusWith<CollectionAndChangedChunks> statusWithCollectionAndChangedChunks,
     ChunkVersion minimumQueryVersion,
-    long long currentTerm)
-    : minQueryVersion(minimumQueryVersion), termCreated(currentTerm) {
+    const long long currentTerm) {
+
+    minQueryVersion = minimumQueryVersion;
+    termCreated = currentTerm;
+
     if (statusWithCollectionAndChangedChunks.isOK()) {
         collectionAndChangedChunks = statusWithCollectionAndChangedChunks.getValue();
         invariant(!collectionAndChangedChunks->changedChunks.empty());
diff --git a/src/mongo/db/s/shard_server_catalog_cache_loader.h b/src/mongo/db/s/shard_server_catalog_cache_loader.h
index cc502319f06..7357491ac8a 100644
--- a/src/mongo/db/s/shard_server_catalog_cache_loader.h
+++ b/src/mongo/db/s/shard_server_catalog_cache_loader.h
@@ -113,9 +113,6 @@ private:
      * metadata for a specific collection.
      */
     struct Task {
-        MONGO_DISALLOW_COPYING(Task);
-        Task(Task&&) = default;
-
         /**
          * Initializes a task for either dropping or updating the persisted metadata for the
          * associated collection. Which type of task is determined by the Status of
@@ -311,7 +308,7 @@ private:
      *
      * Only run on the shard primary.
      */
-    void _updatePersistedMetadata(OperationContext* opCtx, const NamespaceString& nss);
+    bool _updatePersistedMetadata(OperationContext* opCtx, const NamespaceString& nss);
 
     /**
      * Attempt to read the collection and chunk metadata since version 'sinceVersion' from the shard