SERVER-46686 Explain does not respect maxTimeMS

(cherry picked from commit 4ba1495c898b5644d7be9efe21cfb2eecb3f1863)

5 files changed, 110 insertions, 62 deletions

diff --git a/jstests/noPassthrough/explain_max_time_ms.js b/jstests/noPassthrough/explain_max_time_ms.js
new file mode 100644
index 00000000000..a5b3ece1d76
--- /dev/null
+++ b/jstests/noPassthrough/explain_max_time_ms.js
@@ -0,0 +1,71 @@
+/**
+ * Tests the explain command with the maxTimeMS option.
+ */
+(function() {
+    "use strict";
+
+    const standalone = MongoRunner.runMongod();
+    assert.neq(null, standalone, "mongod was unable to start up");
+
+    const dbName = "test";
+    const db = standalone.getDB(dbName);
+    const collName = "explain_max_time_ms";
+    const coll = db.getCollection(collName);
+
+    const destCollName = "explain_max_time_ms_dest";
+    const mapFn = function() {
+        emit(this.i, this.j);
+    };
+    const reduceFn = function(key, values) {
+        return Array.sum(values);
+    };
+
+    coll.drop();
+    assert.commandWorked(db.createCollection(collName));
+
+    assert.commandWorked(coll.insert({i: 1, j: 1}));
+    assert.commandWorked(coll.insert({i: 2, j: 1}));
+    assert.commandWorked(coll.insert({i: 2, j: 2}));
+
+    // Set fail point to make sure operations with "maxTimeMS" set will time out.
+    assert.commandWorked(
+        db.adminCommand({configureFailPoint: "maxTimeAlwaysTimeOut", mode: "alwaysOn"}));
+
+    for (let verbosity of["executionStats", "allPlansExecution"]) {
+        // Expect explain to time out if "maxTimeMS" is set on the aggregate command.
+        assert.commandFailedWithCode(assert.throws(function() {
+            coll.explain(verbosity).aggregate([{$match: {i: 1}}], {maxTimeMS: 1});
+        }),
+                                     ErrorCodes.MaxTimeMSExpired);
+        // Expect explain to time out if "maxTimeMS" is set on the count command.
+        assert.commandFailedWithCode(assert.throws(function() {
+            coll.explain(verbosity).count({i: 1}, {maxTimeMS: 1});
+        }),
+                                     ErrorCodes.MaxTimeMSExpired);
+        // Expect explain to time out if "maxTimeMS" is set on the distinct command.
+        assert.commandFailedWithCode(assert.throws(function() {
+            coll.explain(verbosity).distinct("i", {}, {maxTimeMS: 1});
+        }),
+                                     ErrorCodes.MaxTimeMSExpired);
+        // Expect explain to time out if "maxTimeMS" is set on the find command.
+        assert.commandFailedWithCode(assert.throws(function() {
+            coll.find().maxTimeMS(1).explain(verbosity);
+        }),
+                                     ErrorCodes.MaxTimeMSExpired);
+        assert.commandFailedWithCode(assert.throws(function() {
+            coll.explain(verbosity).find().maxTimeMS(1).finish();
+        }),
+                                     ErrorCodes.MaxTimeMSExpired);
+        // Expect explain to time out if "maxTimeMS" is set on the findAndModify command.
+        assert.commandFailedWithCode(assert.throws(function() {
+            coll.explain(verbosity).findAndModify({update: {$inc: {j: 1}}, maxTimeMS: 1});
+        }),
+                                     ErrorCodes.MaxTimeMSExpired);
+    }
+
+    // Disable fail point.
+    assert.commandWorked(
+        db.adminCommand({configureFailPoint: "maxTimeAlwaysTimeOut", mode: "off"}));
+
+    MongoRunner.stopMongod(standalone);
+})();
diff --git a/jstests/sharding/shard_collection_cache_upgrade_downgrade.js b/jstests/sharding/shard_collection_cache_upgrade_downgrade.js
index 1cc534ce22c..0baa6bd7954 100644
--- a/jstests/sharding/shard_collection_cache_upgrade_downgrade.js
+++ b/jstests/sharding/shard_collection_cache_upgrade_downgrade.js
@@ -41,66 +41,45 @@
 
     const db1Name = "db1";
     const db2Name = "db2";
-    const db3Name = "db3";
     const collName = "foo";
     const ns1 = db1Name + "." + collName;
     const ns2 = db2Name + "." + collName;
-    const ns3 = db3Name + "." + collName;
 
     // Create both collections in the sharding catalog and ensure they are on different shards.
     assert.commandWorked(st.s.adminCommand({enableSharding: db1Name}));
     assert.commandWorked(st.s.adminCommand({enableSharding: db2Name}));
-    assert.commandWorked(st.s.adminCommand({enableSharding: db3Name}));
     st.ensurePrimaryShard(db1Name, st.shard0.shardName);
     st.ensurePrimaryShard(db2Name, st.shard1.shardName);
-    st.ensurePrimaryShard(db3Name, st.shard0.shardName);
     assert.commandWorked(st.s.adminCommand({shardCollection: ns1, key: {_id: 1}}));
     assert.commandWorked(st.s.adminCommand({shardCollection: ns2, key: {_id: 1}}));
-    assert.commandWorked(st.s.adminCommand({shardCollection: ns3, key: {_id: 1}}));
-
-    // Ensure ns3 has chunks in both shards
-    assert.commandWorked(st.s.adminCommand({split: ns3, middle: {_id: 0}}));
-    assert.commandWorked(
-        st.s.adminCommand({moveChunk: ns3, find: {_id: 0}, to: st.shard1.shardName}));
 
     // Ensure the collection entries have UUIDs.
     const ns1EntryOriginal = st.s.getDB("config").getCollection("collections").findOne({_id: ns1});
     const ns2EntryOriginal = st.s.getDB("config").getCollection("collections").findOne({_id: ns2});
-    const ns3EntryOriginal = st.s.getDB("config").getCollection("collections").findOne({_id: ns3});
     assert.neq(null, ns1EntryOriginal.uuid);
     assert.neq(null, ns2EntryOriginal.uuid);
-    assert.neq(null, ns3EntryOriginal.uuid);
 
     const ns1ChunkEntryOriginal = st.s.getDB("config").getCollection("chunks").findOne({ns: ns1});
     const ns2ChunkEntryOriginal = st.s.getDB("config").getCollection("chunks").findOne({ns: ns2});
-    const ns3ChunkEntryOriginal = st.s.getDB("config").getCollection("chunks").findOne({ns: ns3});
     assert.neq(null, ns1ChunkEntryOriginal);
     assert(!ns1ChunkEntryOriginal.hasOwnProperty("history"));
     assert.neq(null, ns2ChunkEntryOriginal);
     assert(!ns2ChunkEntryOriginal.hasOwnProperty("history"));
-    assert.neq(null, ns3ChunkEntryOriginal);
-    assert(!ns3ChunkEntryOriginal.hasOwnProperty("history"));
 
     // Force each shard to refresh for the collection it owns to ensure it writes a cache entry.
 
     assert.commandWorked(st.shard0.adminCommand({_flushRoutingTableCacheUpdates: ns1}));
     assert.commandWorked(st.shard1.adminCommand({_flushRoutingTableCacheUpdates: ns2}));
-    assert.commandWorked(st.shard0.adminCommand({_flushRoutingTableCacheUpdates: ns3}));
-    assert.commandWorked(st.shard1.adminCommand({_flushRoutingTableCacheUpdates: ns3}));
 
     checkCachedCollectionEntry(st.shard0, ns1, ns1EntryOriginal);
     checkCachedCollectionEntry(st.shard0, ns2, undefined);
     checkCachedCollectionEntry(st.shard1, ns1, undefined);
     checkCachedCollectionEntry(st.shard1, ns2, ns2EntryOriginal);
-    checkCachedCollectionEntry(st.shard0, ns3, ns3EntryOriginal);
-    checkCachedCollectionEntry(st.shard1, ns3, ns3EntryOriginal);
 
     checkCachedChunksEntry(st.shard0, ns1, ns1ChunkEntryOriginal);
     checkCachedChunksEntry(st.shard0, ns2, undefined);
     checkCachedChunksEntry(st.shard1, ns1, undefined);
     checkCachedChunksEntry(st.shard1, ns2, ns2ChunkEntryOriginal);
-    checkCachedChunksEntry(st.shard0, ns3, ns3ChunkEntryOriginal);
-    checkCachedChunksEntry(st.shard1, ns3, ns3ChunkEntryOriginal);
 
     // Simulate that the cache entry was written without a UUID (SERVER-33356).
     assert.writeOK(st.shard0.getDB("config")
@@ -119,24 +98,18 @@
     // The UUID in the authoritative collection entries should not have changed.
     const ns1EntryFCV40 = st.s.getDB("config").getCollection("collections").findOne({_id: ns1});
     const ns2EntryFCV40 = st.s.getDB("config").getCollection("collections").findOne({_id: ns2});
-    const ns3EntryFCV40 = st.s.getDB("config").getCollection("collections").findOne({_id: ns3});
     assert.docEq(ns1EntryFCV40, ns1EntryOriginal);
     assert.docEq(ns2EntryFCV40, ns2EntryOriginal);
-    assert.docEq(ns3EntryFCV40, ns3EntryOriginal);
 
     const ns1ChunkEntryFCV40 = st.s.getDB("config").getCollection("chunks").findOne({ns: ns1});
     const ns2ChunkEntryFCV40 = st.s.getDB("config").getCollection("chunks").findOne({ns: ns2});
-    const ns3ChunkEntryFCV40 = st.s.getDB("config").getCollection("chunks").findOne({ns: ns3});
     assert.neq(null, ns1ChunkEntryFCV40);
     assert(ns1ChunkEntryFCV40.hasOwnProperty("history"));
     assert.neq(null, ns2ChunkEntryFCV40);
     assert(ns2ChunkEntryFCV40.hasOwnProperty("history"));
-    assert.neq(null, ns3ChunkEntryFCV40);
-    assert(ns3ChunkEntryFCV40.hasOwnProperty("history"));
 
     st.s.getDB(db1Name).getCollection(collName).findOne();
     st.s.getDB(db2Name).getCollection(collName).findOne();
-    st.s.getDB(db3Name).getCollection(collName).findOne();
 
     // We wait for the refresh triggered by the finds to persist the new cache entry to disk,
     // because it's done asynchronously.
@@ -144,26 +117,18 @@
         st.shard0.adminCommand({_flushRoutingTableCacheUpdates: ns1, syncFromConfig: false}));
     assert.commandWorked(
         st.shard1.adminCommand({_flushRoutingTableCacheUpdates: ns2, syncFromConfig: false}));
-    assert.commandWorked(
-        st.shard0.adminCommand({_flushRoutingTableCacheUpdates: ns3, syncFromConfig: false}));
-    assert.commandWorked(
-        st.shard1.adminCommand({_flushRoutingTableCacheUpdates: ns3, syncFromConfig: false}));
 
     // The shards' collection caches should have been updated with UUIDs.
     checkCachedCollectionEntry(st.shard0, ns1, ns1EntryOriginal);
     checkCachedCollectionEntry(st.shard0, ns2, undefined);
     checkCachedCollectionEntry(st.shard1, ns1, undefined);
     checkCachedCollectionEntry(st.shard1, ns2, ns2EntryOriginal);
-    checkCachedCollectionEntry(st.shard0, ns3, ns3EntryOriginal);
-    checkCachedCollectionEntry(st.shard1, ns3, ns3EntryOriginal);
 
     // The shards' chunk caches should have been updated with histories.
     checkCachedChunksEntry(st.shard0, ns1, ns1ChunkEntryFCV40);
     checkCachedChunksEntry(st.shard0, ns2, undefined);
     checkCachedChunksEntry(st.shard1, ns1, undefined);
     checkCachedChunksEntry(st.shard1, ns2, ns2ChunkEntryFCV40);
-    checkCachedChunksEntry(st.shard0, ns3, ns3ChunkEntryFCV40);
-    checkCachedChunksEntry(st.shard1, ns3, ns3ChunkEntryFCV40);
 
     //
     // setFCV 3.6 (downgrade)
@@ -174,33 +139,23 @@
     // The UUID in the authoritative collection entries should still not have changed.
     const ns1EntryFCV36 = st.s.getDB("config").getCollection("collections").findOne({_id: ns1});
     const ns2EntryFCV36 = st.s.getDB("config").getCollection("collections").findOne({_id: ns2});
-    const ns3EntryFCV36 = st.s.getDB("config").getCollection("collections").findOne({_id: ns3});
     assert.docEq(ns1EntryFCV36, ns1EntryOriginal);
     assert.docEq(ns2EntryFCV36, ns2EntryOriginal);
-    assert.docEq(ns3EntryFCV36, ns3EntryOriginal);
 
     const ns1ChunkEntryFCV36 = st.s.getDB("config").getCollection("chunks").findOne({ns: ns1});
     const ns2ChunkEntryFCV36 = st.s.getDB("config").getCollection("chunks").findOne({ns: ns2});
-    const ns3ChunkEntryFCV36 = st.s.getDB("config").getCollection("chunks").findOne({ns: ns3});
     assert.neq(null, ns1ChunkEntryFCV36);
     assert(!ns1ChunkEntryFCV36.hasOwnProperty("history"));
     assert.neq(null, ns2ChunkEntryFCV36);
     assert(!ns2ChunkEntryFCV36.hasOwnProperty("history"));
-    assert.neq(null, ns3ChunkEntryFCV36);
-    assert(!ns3ChunkEntryFCV36.hasOwnProperty("history"));
 
     st.s.getDB(db1Name).getCollection(collName).findOne();
     st.s.getDB(db2Name).getCollection(collName).findOne();
-    st.s.getDB(db3Name).getCollection(collName).findOne();
 
     assert.commandWorked(
         st.shard0.adminCommand({_flushRoutingTableCacheUpdates: ns1, syncFromConfig: false}));
     assert.commandWorked(
         st.shard1.adminCommand({_flushRoutingTableCacheUpdates: ns2, syncFromConfig: false}));
-    assert.commandWorked(
-        st.shard0.adminCommand({_flushRoutingTableCacheUpdates: ns3, syncFromConfig: false}));
-    assert.commandWorked(
-        st.shard1.adminCommand({_flushRoutingTableCacheUpdates: ns3, syncFromConfig: false}));
 
     // Also refresh the sessions collection so that the UUID consistency check at the end of
     // ShardingTest, which will check for its UUID on the shards, passes.
@@ -214,16 +169,12 @@
     checkCachedCollectionEntry(st.shard0, ns2, undefined);
     checkCachedCollectionEntry(st.shard1, ns1, undefined);
     checkCachedCollectionEntry(st.shard1, ns2, ns2EntryOriginal);
-    checkCachedCollectionEntry(st.shard0, ns3, ns3EntryOriginal);
-    checkCachedCollectionEntry(st.shard1, ns3, ns3EntryOriginal);
 
     // The shards' chunk caches should have been updated with histories removed.
     checkCachedChunksEntry(st.shard0, ns1, ns1ChunkEntryFCV40);
     checkCachedChunksEntry(st.shard0, ns2, undefined);
     checkCachedChunksEntry(st.shard1, ns1, undefined);
     checkCachedChunksEntry(st.shard1, ns2, ns2ChunkEntryFCV40);
-    checkCachedChunksEntry(st.shard0, ns3, ns3ChunkEntryFCV40);
-    checkCachedChunksEntry(st.shard1, ns3, ns3ChunkEntryFCV40);
 
     st.stop();
 })();
diff --git a/src/mongo/db/s/config/sharding_catalog_manager_chunk_operations.cpp b/src/mongo/db/s/config/sharding_catalog_manager_chunk_operations.cpp
index 564b3dcf4e7..033a96c6a25 100644
--- a/src/mongo/db/s/config/sharding_catalog_manager_chunk_operations.cpp
+++ b/src/mongo/db/s/config/sharding_catalog_manager_chunk_operations.cpp
@@ -51,7 +51,6 @@
 #include "mongo/s/client/shard_registry.h"
 #include "mongo/s/grid.h"
 #include "mongo/s/shard_key_pattern.h"
-#include "mongo/stdx/unordered_set.h"
 #include "mongo/util/fail_point_service.h"
 #include "mongo/util/log.h"
 #include "mongo/util/mongoutils/str.h"
@@ -941,7 +940,6 @@ Status ShardingCatalogManager::upgradeChunksHistory(OperationContext* opCtx,
                                              0,
                                              currentCollectionVersion.getValue().epoch());
 
-    stdx::unordered_set<ShardId, ShardId::Hasher> bumpedShards;
     for (const auto& chunk : chunksVector) {
         auto swChunk = ChunkType::fromConfigBSON(chunk);
         if (!swChunk.isOK()) {
@@ -950,14 +948,10 @@ Status ShardingCatalogManager::upgradeChunksHistory(OperationContext* opCtx,
         auto& upgradeChunk = swChunk.getValue();
 
         if (upgradeChunk.getHistory().empty()) {
-            // Bump the version for only one chunk per shard to satisfy the requirement imposed by
-            // SERVER-33356
-            const auto& shardId = upgradeChunk.getShard();
-            if (!bumpedShards.count(shardId)) {
-                upgradeChunk.setVersion(newCollectionVersion);
-                newCollectionVersion.incMajor();
-                bumpedShards.emplace(shardId);
-            }
+
+            // Bump the version.
+            upgradeChunk.setVersion(newCollectionVersion);
+            newCollectionVersion.incMajor();
 
             // Construct the fresh history.
             upgradeChunk.setHistory({ChunkHistory{validAfter, upgradeChunk.getShard()}});
@@ -1010,6 +1004,16 @@ Status ShardingCatalogManager::downgradeChunksHistory(OperationContext* opCtx,
                               << ", but found no chunks"};
     }
 
+    const auto currentCollectionVersion = _findCollectionVersion(opCtx, nss, collectionEpoch);
+    if (!currentCollectionVersion.isOK()) {
+        return currentCollectionVersion.getStatus();
+    }
+
+    // Bump the version.
+    auto newCollectionVersion = ChunkVersion(currentCollectionVersion.getValue().majorVersion() + 1,
+                                             0,
+                                             currentCollectionVersion.getValue().epoch());
+
     for (const auto& chunk : chunksVector) {
         auto swChunk = ChunkType::fromConfigBSON(chunk);
         if (!swChunk.isOK()) {
@@ -1017,6 +1021,10 @@ Status ShardingCatalogManager::downgradeChunksHistory(OperationContext* opCtx,
         }
         auto& downgradeChunk = swChunk.getValue();
 
+        // Bump the version.
+        downgradeChunk.setVersion(newCollectionVersion);
+        newCollectionVersion.incMajor();
+
         // Clear the history.
         downgradeChunk.setHistory({});
 
diff --git a/src/mongo/shell/explain_query.js b/src/mongo/shell/explain_query.js
index 78e57c86e69..80d79ae18fa 100644
--- a/src/mongo/shell/explain_query.js
+++ b/src/mongo/shell/explain_query.js
@@ -151,6 +151,11 @@ var DBExplainQuery = (function() {
 
                 var explainCmd = {explain: innerCmd};
                 explainCmd["verbosity"] = this._verbosity;
+                // If "maxTimeMS" is set on innerCmd, it needs to be propagated to the top-level
+                // of explainCmd so that it has the intended effect.
+                if (innerCmd.hasOwnProperty("maxTimeMS")) {
+                    explainCmd.maxTimeMS = innerCmd.maxTimeMS;
+                }
 
                 var explainDb = this._query._db;
 
diff --git a/src/mongo/shell/explainable.js b/src/mongo/shell/explainable.js
index ae4f918f264..930d690290b 100644
--- a/src/mongo/shell/explainable.js
+++ b/src/mongo/shell/explainable.js
@@ -28,6 +28,16 @@ var Explainable = (function() {
         return explainResult;
     };
 
+    var buildExplainCmd = function(innerCmd, verbosity) {
+        var explainCmd = {"explain": innerCmd, "verbosity": verbosity};
+        // If "maxTimeMS" is set on innerCmd, it needs to be propagated to the top-level
+        // of explainCmd so that it has the intended effect.
+        if (innerCmd.hasOwnProperty("maxTimeMS")) {
+            explainCmd.maxTimeMS = innerCmd.maxTimeMS;
+        }
+        return explainCmd;
+    };
+
     function constructor(collection, verbosity) {
         //
         // Private vars.
@@ -110,7 +120,7 @@ var Explainable = (function() {
 
                 let aggCmd = Object.extend(
                     {"aggregate": this._collection.getName(), "pipeline": pipeline}, extraOptsCopy);
-                let explainCmd = {"explain": aggCmd, "verbosity": this._verbosity};
+                let explainCmd = buildExplainCmd(aggCmd, this._verbosity);
                 let explainResult = this._collection.runReadCommand(explainCmd);
                 return throwOrReturn(explainResult);
             }
@@ -133,7 +143,7 @@ var Explainable = (function() {
 
         this.findAndModify = function(params) {
             var famCmd = Object.extend({"findAndModify": this._collection.getName()}, params);
-            var explainCmd = {"explain": famCmd, "verbosity": this._verbosity};
+            var explainCmd = buildExplainCmd(famCmd, this._verbosity);
             var explainResult = this._collection.runReadCommand(explainCmd);
             return throwOrReturn(explainResult);
         };
@@ -156,8 +166,11 @@ var Explainable = (function() {
             if (options && options.hasOwnProperty("collation")) {
                 distinctCmd.collation = options.collation;
             }
+            if (options && options.hasOwnProperty("maxTimeMS")) {
+                distinctCmd.maxTimeMS = options.maxTimeMS;
+            }
 
-            var explainCmd = {explain: distinctCmd, verbosity: this._verbosity};
+            var explainCmd = buildExplainCmd(distinctCmd, this._verbosity);
             var explainResult = this._collection.runReadCommand(explainCmd);
             return throwOrReturn(explainResult);
         };