SERVER-26777 Improve logging around chunk refresh

Unifies the logging messages between mongos and mongod and adds timing
information.

2 files changed, 28 insertions, 16 deletions

diff --git a/src/mongo/s/catalog/type_collection.cpp b/src/mongo/s/catalog/type_collection.cpp
index 5c94eb3fc69..aed93093462 100644
--- a/src/mongo/s/catalog/type_collection.cpp
+++ b/src/mongo/s/catalog/type_collection.cpp
@@ -105,8 +105,15 @@ StatusWith<CollectionType> CollectionType::fromBSON(const BSONObj& source) {
             }
 
             coll._keyPattern = KeyPattern(obj.getOwned());
-        } else if ((status == ErrorCodes::NoSuchKey) && coll.getDropped()) {
-            // Sharding key can be missing if the collection is dropped
+        } else if (status == ErrorCodes::NoSuchKey) {
+            // Sharding key can only be missing if the collection is dropped
+            if (!coll.getDropped()) {
+                return {status.code(),
+                        str::stream() << "Shard key for collection " << coll._fullNs->ns()
+                                      << " is missing, but the collection is not marked as "
+                                         "dropped. This is an indication of corrupted sharding "
+                                         "metadata."};
+            }
         } else {
             return status;
         }
diff --git a/src/mongo/s/chunk_manager.cpp b/src/mongo/s/chunk_manager.cpp
index f5dca5e24f8..6979b583880 100644
--- a/src/mongo/s/chunk_manager.cpp
+++ b/src/mongo/s/chunk_manager.cpp
@@ -77,8 +77,7 @@ using std::vector;
 namespace {
 
 /**
- * This is an adapter so we can use config diffs - mongos and mongod do them slightly
- * differently
+ * This is an adapter so we can use config diffs - mongos and mongod do them slightly differently.
  *
  * The mongos adapter here tracks all shards, and stores ranges by (max, Chunk) in the map.
  */
@@ -116,7 +115,6 @@ private:
     ChunkManager* const _manager;
 };
 
-
 bool allOfType(BSONType type, const BSONObj& o) {
     BSONObjIterator it(o);
     while (it.more()) {
@@ -207,6 +205,8 @@ ChunkManager::ChunkManager(OperationContext* txn, const CollectionType& coll)
 }
 
 void ChunkManager::loadExistingRanges(OperationContext* txn, const ChunkManager* oldManager) {
+    invariant(!_version.isSet());
+
     int tries = 3;
 
     while (tries--) {
@@ -217,24 +217,29 @@ void ChunkManager::loadExistingRanges(OperationContext* txn, const ChunkManager*
 
         Timer t;
 
-        bool success = _load(txn, chunkMap, shardIds, &shardVersions, oldManager);
-        if (success) {
-            log() << "ChunkManager: time to load chunks for " << _ns << ": " << t.millis() << "ms"
-                  << " sequenceNumber: " << _sequenceNumber << " version: " << _version.toString()
-                  << " based on: "
-                  << (oldManager ? oldManager->getVersion().toString() : "(empty)");
+        log() << "ChunkManager loading chunks for " << _ns << " sequenceNumber: " << _sequenceNumber
+              << " based on: " << (oldManager ? oldManager->getVersion().toString() : "(empty)");
 
+        if (_load(txn, chunkMap, shardIds, &shardVersions, oldManager)) {
             // TODO: Merge into diff code above, so we validate in one place
             if (isChunkMapValid(chunkMap)) {
-                _chunkMap.swap(chunkMap);
-                _shardIds.swap(shardIds);
-                _shardVersions.swap(shardVersions);
+                _chunkMap = std::move(chunkMap);
+                _shardIds = std::move(shardIds);
+                _shardVersions = std::move(shardVersions);
                 _chunkRangeMap = _constructRanges(_chunkMap);
-                return;
+
+                log() << "ChunkManager load took " << t.millis() << " ms and found version "
+                      << _version;
+            } else {
+                warning() << "ChunkManager load took " << t.millis()
+                          << " ms and found invalid chunk ranges at version " << _version;
             }
+
+            return;
         }
 
-        warning() << "ChunkManager loaded an invalid config for " << _ns << ", trying again";
+        warning() << "ChunkManager load failed after " << t.millis()
+                  << " ms and will be retried up to " << tries << " more times";
 
         sleepmillis(10 * (3 - tries));
     }