SERVER-43081 Validate should report when an index's 'multikeyPaths' are set but the 'multikey' flag is false

This also removes the invariant added by SERVER-43008 that performed the
same check in the catalog layer. It is not safe to invariant based on
durable data if it is known that data could be corrupt in an undefined
way (see SERVER-43074), so the safest option is to report a validation
error, which requires rebuilding the index to fix the problem.

2 files changed, 52 insertions, 16 deletions

diff --git a/src/mongo/db/catalog/collection_validation.cpp b/src/mongo/db/catalog/collection_validation.cpp
index b774da1301c..bfd16f5e1a2 100644
--- a/src/mongo/db/catalog/collection_validation.cpp
+++ b/src/mongo/db/catalog/collection_validation.cpp
@@ -33,6 +33,8 @@
 
 #include "mongo/db/catalog/collection_validation.h"
 
+#include <fmt/format.h>
+
 #include "mongo/db/catalog/collection.h"
 #include "mongo/db/catalog/index_consistency.h"
 #include "mongo/db/catalog/throttle_cursor.h"
@@ -338,6 +340,35 @@ void addErrorIfUnequal(T stored, T cached, StringData name, ValidateResults* res
     }
 }
 
+std::string multikeyPathsToString(MultikeyPaths paths) {
+    str::stream builder;
+    builder << "[";
+    auto pathIt = paths.begin();
+    while (true) {
+        builder << "{";
+
+        auto pathSet = *pathIt;
+        auto setIt = pathSet.begin();
+        while (true) {
+            builder << *setIt++;
+            if (setIt == pathSet.end()) {
+                break;
+            } else {
+                builder << ",";
+            }
+        }
+        builder << "}";
+
+        if (++pathIt == paths.end()) {
+            break;
+        } else {
+            builder << ",";
+        }
+    }
+    builder << "]";
+    return builder;
+}
+
 void _validateCatalogEntry(OperationContext* opCtx,
                            Collection* coll,
                            BSONObj validatorDoc,
@@ -377,8 +408,28 @@ void _validateCatalogEntry(OperationContext* opCtx,
         results->errors.push_back(str::stream() << "collection options are not valid for storage: "
                                                 << options.toBSON());
     }
-}
 
+    std::vector<std::string> indexes;
+    DurableCatalog::get(opCtx)->getReadyIndexes(opCtx, coll->ns(), &indexes);
+    for (auto& index : indexes) {
+        MultikeyPaths multikeyPaths;
+        const bool isMultikey =
+            DurableCatalog::get(opCtx)->isIndexMultikey(opCtx, coll->ns(), index, &multikeyPaths);
+        const bool hasMultiKeyPaths = std::any_of(multikeyPaths.begin(),
+                                                  multikeyPaths.end(),
+                                                  [](auto& pathSet) { return pathSet.size() > 0; });
+        // It is illegal for multikey paths to exist without the multikey flag set on the index, but
+        // it may be possible for multikey to be set on the index while having no multikey paths. If
+        // any of the paths are multikey, then the entire index should also be marked multikey.
+        if (hasMultiKeyPaths && !isMultikey) {
+            results->valid = false;
+            results->errors.push_back(fmt::format(
+                "The 'multikey' field for index {} was false with non-empty 'multikeyPaths': {}",
+                index,
+                multikeyPathsToString(multikeyPaths)));
+        }
+    }
+}
 }  // namespace
 
 Status validate(OperationContext* opCtx,
diff --git a/src/mongo/db/storage/durable_catalog_impl.cpp b/src/mongo/db/storage/durable_catalog_impl.cpp
index 62d7797dc1b..faf49552f34 100644
--- a/src/mongo/db/storage/durable_catalog_impl.cpp
+++ b/src/mongo/db/storage/durable_catalog_impl.cpp
@@ -601,21 +601,6 @@ void DurableCatalogImpl::putMetaData(OperationContext* opCtx,
             const auto index = md.indexes[i];
             string name = index.name();
 
-            // It is illegal for multikey paths to exist without the multikey flag set on the index,
-            // but it may be possible for multikey to be set on the index while having no multikey
-            // paths. If any of the paths are multikey, then the entire index should also be marked
-            // multikey.
-            const bool hasMultiKeyPaths =
-                std::any_of(index.multikeyPaths.begin(),
-                            index.multikeyPaths.end(),
-                            [](auto& pathSet) { return pathSet.size() > 0; });
-            invariant(!hasMultiKeyPaths || index.multikey,
-                      fmt::format("The 'multikey' field for index {} was false with non-empty "
-                                  "'multikeyPaths'. New metadata: {}, catalog document: {}",
-                                  name,
-                                  md.toBSON().toString(),
-                                  obj.toString()));
-
             // fix ident map
             BSONElement e = oldIdentMap[name];
             if (e.type() == String) {