SERVER-20221 Remove unsetSharding call from dropCollection

3 files changed, 8 insertions, 180 deletions

diff --git a/src/mongo/db/s/config/sharding_catalog_manager.h b/src/mongo/db/s/config/sharding_catalog_manager.h
index 750c86f76c5..ba9ddccd945 100644
--- a/src/mongo/db/s/config/sharding_catalog_manager.h
+++ b/src/mongo/db/s/config/sharding_catalog_manager.h
@@ -295,9 +295,8 @@ public:
     /**
      * Ensures that a namespace that has received a dropCollection, but no longer has an entry in
      * config.collections, has cleared all relevant metadata entries for the corresponding
-     * collection. As part of this, sends dropCollection, setShardVersion, and unsetSharding to all
-     * shards -- in case shards didn't receive these commands as part of the original
-     * dropCollection.
+     * collection. As part of this, sends dropCollection and setShardVersion to all shards -- in
+     * case shards didn't receive these commands as part of the original dropCollection.
      *
      * This function does not guarantee that all shards will eventually receive setShardVersion,
      * unless the client infinitely retries until hearing back success. This function does, however,
diff --git a/src/mongo/db/s/config/sharding_catalog_manager_collection_operations.cpp b/src/mongo/db/s/config/sharding_catalog_manager_collection_operations.cpp
index 3bdf40442f0..4979e519ed8 100644
--- a/src/mongo/db/s/config/sharding_catalog_manager_collection_operations.cpp
+++ b/src/mongo/db/s/config/sharding_catalog_manager_collection_operations.cpp
@@ -369,7 +369,7 @@ void sendDropCollectionToAllShards(OperationContext* opCtx, const NamespaceStrin
     }
 }
 
-void sendSSVAndUnsetShardingToAllShards(OperationContext* opCtx, const NamespaceString& nss) {
+void sendSSVToAllShards(OperationContext* opCtx, const NamespaceString& nss) {
     const auto catalogClient = Grid::get(opCtx)->catalogClient();
 
     const auto shardsStatus =
@@ -401,16 +401,6 @@ void sendSSVAndUnsetShardingToAllShards(OperationContext* opCtx, const Namespace
 
         uassertStatusOK(ssvResult.getStatus());
         uassertStatusOK(ssvResult.getValue().commandStatus);
-
-        auto unsetShardingStatus = shard->runCommandWithFixedRetryAttempts(
-            opCtx,
-            ReadPreferenceSetting{ReadPreference::PrimaryOnly},
-            "admin",
-            BSON("unsetSharding" << 1),
-            Shard::RetryPolicy::kIdempotent);
-
-        uassertStatusOK(unsetShardingStatus);
-        uassertStatusOK(unsetShardingStatus.getValue().commandStatus);
     }
 }
 
@@ -463,7 +453,7 @@ void ShardingCatalogManager::dropCollection(OperationContext* opCtx, const Names
 
     LOG(1) << "dropCollection " << nss.ns() << " collection marked as dropped";
 
-    sendSSVAndUnsetShardingToAllShards(opCtx, nss);
+    sendSSVToAllShards(opCtx, nss);
 
     LOG(1) << "dropCollection " << nss.ns() << " completed";
 
@@ -478,7 +468,7 @@ void ShardingCatalogManager::ensureDropCollectionCompleted(OperationContext* opC
            << " from previous dropCollection are cleared";
     sendDropCollectionToAllShards(opCtx, nss);
     removeChunksAndTagsForDroppedCollection(opCtx, nss);
-    sendSSVAndUnsetShardingToAllShards(opCtx, nss);
+    sendSSVToAllShards(opCtx, nss);
 }
 
 void ShardingCatalogManager::renameCollection(OperationContext* opCtx,
diff --git a/src/mongo/db/s/config/sharding_catalog_manager_drop_coll_test.cpp b/src/mongo/db/s/config/sharding_catalog_manager_drop_coll_test.cpp
index 03e169ded2c..155ca6cf924 100644
--- a/src/mongo/db/s/config/sharding_catalog_manager_drop_coll_test.cpp
+++ b/src/mongo/db/s/config/sharding_catalog_manager_drop_coll_test.cpp
@@ -125,19 +125,6 @@ public:
             HostAndPort(shard.getHost()), shard, dropNS(), ChunkVersion::DROPPED());
     }
 
-    void expectUnsetSharding(const ShardType& shard) {
-        onCommand([shard](const RemoteCommandRequest& request) {
-            ASSERT_EQ(HostAndPort(shard.getHost()), request.target);
-            ASSERT_EQ("admin", request.dbname);
-            ASSERT_BSONOBJ_EQ(BSON("unsetSharding" << 1), request.cmdObj);
-
-            ASSERT_BSONOBJ_EQ(rpc::makeEmptyMetadata(),
-                              rpc::TrackingMetadata::removeTrackingData(request.metadata));
-
-            return BSON("n" << 1 << "ok" << 1);
-        });
-    }
-
     void expectCollectionDocMarkedAsDropped() {
         auto findStatus =
             findOneOnConfigCollection(operationContext(), CollectionType::ConfigNS, BSONObj());
@@ -196,10 +183,7 @@ TEST_F(DropColl2ShardTest, Basic) {
     expectDrop(shard2());
 
     expectSetShardVersionZero(shard1());
-    expectUnsetSharding(shard1());
-
     expectSetShardVersionZero(shard2());
-    expectUnsetSharding(shard2());
 
     future.default_timed_get();
 
@@ -234,10 +218,7 @@ TEST_F(DropColl2ShardTest, NSNotFound) {
     });
 
     expectSetShardVersionZero(shard1());
-    expectUnsetSharding(shard1());
-
     expectSetShardVersionZero(shard2());
-    expectUnsetSharding(shard2());
 
     future.default_timed_get();
 
@@ -368,48 +349,6 @@ TEST_F(DropColl2ShardTest, SSVErrorOnShard1) {
     expectNoTagDocs();
 }
 
-TEST_F(DropColl2ShardTest, UnsetCmdErrorOnShard1) {
-    auto future = launchAsync(
-        [this] { ASSERT_THROWS_CODE(doDrop(), AssertionException, ErrorCodes::Unauthorized); });
-
-    expectDrop(shard1());
-    expectDrop(shard2());
-
-    expectSetShardVersionZero(shard1());
-
-    onCommand([](const RemoteCommandRequest& request) {
-        return BSON("ok" << 0 << "code" << ErrorCodes::Unauthorized << "errmsg"
-                         << "bad");
-    });
-
-    future.default_timed_get();
-
-    expectCollectionDocMarkedAsDropped();
-    expectNoChunkDocs();
-    expectNoTagDocs();
-}
-
-TEST_F(DropColl2ShardTest, UnsetErrorOnShard1) {
-    auto future = launchAsync(
-        [this] { ASSERT_THROWS_CODE(doDrop(), AssertionException, ErrorCodes::CallbackCanceled); });
-
-    expectDrop(shard1());
-    expectDrop(shard2());
-
-    expectSetShardVersionZero(shard1());
-
-    onCommand([this](const RemoteCommandRequest& request) {
-        shutdownExecutor();  // shutdown executor so unsetSharding command will fail.
-        return BSON("ok" << 1);
-    });
-
-    future.default_timed_get();
-
-    expectCollectionDocMarkedAsDropped();
-    expectNoChunkDocs();
-    expectNoTagDocs();
-}
-
 TEST_F(DropColl2ShardTest, SSVCmdErrorOnShard2) {
     auto future = launchAsync(
         [this] { ASSERT_THROWS_CODE(doDrop(), AssertionException, ErrorCodes::Unauthorized); });
@@ -418,7 +357,6 @@ TEST_F(DropColl2ShardTest, SSVCmdErrorOnShard2) {
     expectDrop(shard2());
 
     expectSetShardVersionZero(shard1());
-    expectUnsetSharding(shard1());
 
     onCommand([](const RemoteCommandRequest& request) {
         return BSON("ok" << 0 << "code" << ErrorCodes::Unauthorized << "errmsg"
@@ -440,7 +378,6 @@ TEST_F(DropColl2ShardTest, SSVErrorOnShard2) {
     expectDrop(shard2());
 
     expectSetShardVersionZero(shard1());
-    expectUnsetSharding(shard1());
 
     onCommand([this](const RemoteCommandRequest& request) {
         shutdownExecutor();  // shutdown executor so ssv command will fail.
@@ -454,69 +391,18 @@ TEST_F(DropColl2ShardTest, SSVErrorOnShard2) {
     expectNoTagDocs();
 }
 
-TEST_F(DropColl2ShardTest, UnsetCmdErrorOnShard2) {
-    auto future = launchAsync(
-        [this] { ASSERT_THROWS_CODE(doDrop(), AssertionException, ErrorCodes::Unauthorized); });
-
-    expectDrop(shard1());
-    expectDrop(shard2());
-
-    expectSetShardVersionZero(shard1());
-    expectUnsetSharding(shard1());
-
-    expectSetShardVersionZero(shard2());
-
-    onCommand([](const RemoteCommandRequest& request) {
-        return BSON("ok" << 0 << "code" << ErrorCodes::Unauthorized << "errmsg"
-                         << "bad");
-    });
-
-    future.default_timed_get();
-
-    expectCollectionDocMarkedAsDropped();
-    expectNoChunkDocs();
-    expectNoTagDocs();
-}
-
-TEST_F(DropColl2ShardTest, UnsetErrorOnShard2) {
-    auto future = launchAsync(
-        [this] { ASSERT_THROWS_CODE(doDrop(), AssertionException, ErrorCodes::CallbackCanceled); });
-
-    expectDrop(shard1());
-    expectDrop(shard2());
-
-    expectSetShardVersionZero(shard1());
-    expectUnsetSharding(shard1());
-
-    expectSetShardVersionZero(shard2());
-
-    onCommand([this](const RemoteCommandRequest& request) {
-        shutdownExecutor();  // shutdown executor so unset command will fail.
-        return BSON("ok" << 1);
-    });
-
-    future.default_timed_get();
-
-    expectCollectionDocMarkedAsDropped();
-    expectNoChunkDocs();
-    expectNoTagDocs();
-}
-
 /**
  * Tests of dropCollection retry behavior.
  */
 
-TEST_F(DropColl2ShardTest, AfterSuccessRetryWillStillSendDropSSVandUnsetSharding) {
+TEST_F(DropColl2ShardTest, AfterSuccessRetryWillStillSendDropSSV) {
     auto firstDropFuture = launchAsync([this] { doDrop(); });
 
     expectDrop(shard1());
     expectDrop(shard2());
 
     expectSetShardVersionZero(shard1());
-    expectUnsetSharding(shard1());
-
     expectSetShardVersionZero(shard2());
-    expectUnsetSharding(shard2());
 
     firstDropFuture.default_timed_get();
 
@@ -530,10 +416,7 @@ TEST_F(DropColl2ShardTest, AfterSuccessRetryWillStillSendDropSSVandUnsetSharding
     expectDrop(shard2());
 
     expectSetShardVersionZero(shard1());
-    expectUnsetSharding(shard1());
-
     expectSetShardVersionZero(shard2());
-    expectUnsetSharding(shard2());
 
     secondDropFuture.default_timed_get();
 
@@ -542,7 +425,7 @@ TEST_F(DropColl2ShardTest, AfterSuccessRetryWillStillSendDropSSVandUnsetSharding
     expectNoTagDocs();
 }
 
-TEST_F(DropColl2ShardTest, AfterFailedDropRetryWillStillSendDropSSVandUnsetSharding) {
+TEST_F(DropColl2ShardTest, AfterFailedDropRetryWillStillSendDropSSV) {
     auto firstDropFuture = launchAsync(
         [this] { ASSERT_THROWS_CODE(doDrop(), AssertionException, ErrorCodes::Unauthorized); });
 
@@ -558,10 +441,7 @@ TEST_F(DropColl2ShardTest, AfterFailedDropRetryWillStillSendDropSSVandUnsetShard
     expectDrop(shard2());
 
     expectSetShardVersionZero(shard1());
-    expectUnsetSharding(shard1());
-
     expectSetShardVersionZero(shard2());
-    expectUnsetSharding(shard2());
 
     secondDropFuture.default_timed_get();
 
@@ -571,7 +451,7 @@ TEST_F(DropColl2ShardTest, AfterFailedDropRetryWillStillSendDropSSVandUnsetShard
     expectNoTagDocs();
 }
 
-TEST_F(DropColl2ShardTest, AfterFailedSSVRetryWillStillSendDropSSVandUnsetSharding) {
+TEST_F(DropColl2ShardTest, AfterFailedSSVRetryWillStillSendDropSSV) {
     auto firstDropFuture = launchAsync(
         [this] { ASSERT_THROWS_CODE(doDrop(), AssertionException, ErrorCodes::Unauthorized); });
 
@@ -590,10 +470,7 @@ TEST_F(DropColl2ShardTest, AfterFailedSSVRetryWillStillSendDropSSVandUnsetShardi
     expectDrop(shard2());
 
     expectSetShardVersionZero(shard1());
-    expectUnsetSharding(shard1());
-
     expectSetShardVersionZero(shard2());
-    expectUnsetSharding(shard2());
 
     secondDropFuture.default_timed_get();
 
@@ -602,43 +479,5 @@ TEST_F(DropColl2ShardTest, AfterFailedSSVRetryWillStillSendDropSSVandUnsetShardi
     expectNoTagDocs();
     expectNoTagDocs();
 }
-
-TEST_F(DropColl2ShardTest, AfterFailedUnsetShardingRetryWillStillSendDropSSVandUnsetSharding) {
-    auto firstDropFuture = launchAsync(
-        [this] { ASSERT_THROWS_CODE(doDrop(), AssertionException, ErrorCodes::Unauthorized); });
-
-    expectDrop(shard1());
-    expectDrop(shard2());
-
-    expectSetShardVersionZero(shard1());
-    expectUnsetSharding(shard1());
-
-    expectSetShardVersionZero(shard2());
-
-    onCommand([this](const RemoteCommandRequest& request) {
-        return BSON("ok" << 0 << "code" << ErrorCodes::Unauthorized);
-    });
-
-    firstDropFuture.default_timed_get();
-
-    auto secondDropFuture = launchAsync([this] { doDrop(); });
-
-    expectDrop(shard1());
-    expectDrop(shard2());
-
-    expectSetShardVersionZero(shard1());
-    expectUnsetSharding(shard1());
-
-    expectSetShardVersionZero(shard2());
-    expectUnsetSharding(shard2());
-
-    secondDropFuture.default_timed_get();
-
-    expectCollectionDocMarkedAsDropped();
-    expectNoChunkDocs();
-    expectNoTagDocs();
-    expectNoTagDocs();
-}
-
 }  // unnamed namespace
 }  // namespace mongo