SERVER-26531 Don't reset the control chunk's 'jumbo' flag on migration commit

(cherry picked from commit 8fd79fe845e3e4717df26abb78c3fb7859f3823e)

3 files changed, 59 insertions, 55 deletions

diff --git a/src/mongo/db/s/config/sharding_catalog_manager.h b/src/mongo/db/s/config/sharding_catalog_manager.h
index 6e4bef00edd..ba895fb1864 100644
--- a/src/mongo/db/s/config/sharding_catalog_manager.h
+++ b/src/mongo/db/s/config/sharding_catalog_manager.h
@@ -189,6 +189,12 @@ public:
      * Updates metadata in config.chunks collection to show the given chunk in its new shard.
      * If 'validAfter' is not set, this means the commit request came from an older server version,
      * which is not history-aware.
+     *
+     * Returns a BSON object with the newly produced chunk versions after the migration:
+     *  - migratedChunkVersion - the version of the chunk, which was migrated
+     *  - controlChunkVersion (optional) - the version of the "control" chunk, which was changed in
+     *      order to reflect the change on the donor. This value will be missing if the last chunk
+     *      on the donor shard was migrated out.
      */
     StatusWith<BSONObj> commitChunkMigration(OperationContext* opCtx,
                                              const NamespaceString& nss,
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 36d046f32f9..8b2c4b333d2 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
@@ -224,6 +224,7 @@ BSONObj makeCommitChunkTransactionCommand(const NamespaceString& nss,
         n.append(ChunkType::min(), controlChunk->getMin());
         n.append(ChunkType::max(), controlChunk->getMax());
         n.append(ChunkType::shard(), fromShard);
+        n.append(ChunkType::jumbo(), controlChunk->getJumbo());
         controlChunk->addHistoryToBSON(n);
         n.done();
 
diff --git a/src/mongo/db/s/config/sharding_catalog_manager_commit_chunk_migration_test.cpp b/src/mongo/db/s/config/sharding_catalog_manager_commit_chunk_migration_test.cpp
index 448e56a3faf..08ded5de55a 100644
--- a/src/mongo/db/s/config/sharding_catalog_manager_commit_chunk_migration_test.cpp
+++ b/src/mongo/db/s/config/sharding_catalog_manager_commit_chunk_migration_test.cpp
@@ -43,12 +43,13 @@
 namespace mongo {
 namespace {
 
+using unittest::assertGet;
+
 using CommitChunkMigrate = ConfigServerTestFixture;
 
 const NamespaceString kNamespace("TestDB.TestColl");
 
-TEST_F(CommitChunkMigrate, CheckCorrectOpsCommandWithCtl) {
-
+TEST_F(CommitChunkMigrate, ChunksUpdatedCorrectlyWithControlChunk) {
     ShardType shard0;
     shard0.setName("shard0");
     shard0.setHost("shard0:12");
@@ -59,80 +60,76 @@ TEST_F(CommitChunkMigrate, CheckCorrectOpsCommandWithCtl) {
 
     setupShards({shard0, shard1}).transitional_ignore();
 
-    int origMajorVersion = 12;
-    auto const origVersion = ChunkVersion(origMajorVersion, 7, OID::gen());
-
-    ChunkType chunk0;
-    chunk0.setNS(kNamespace);
-    chunk0.setVersion(origVersion);
-    chunk0.setShard(shard0.getName());
-    chunk0.setHistory({ChunkHistory(Timestamp(100, 0), shard0.getName())});
+    ChunkType migratedChunk, controlChunk;
+    {
+        ChunkVersion origVersion(12, 7, OID::gen());
 
-    // apportion
-    auto chunkMin = BSON("a" << 1);
-    chunk0.setMin(chunkMin);
-    auto chunkMax = BSON("a" << 10);
-    chunk0.setMax(chunkMax);
+        migratedChunk.setNS(kNamespace);
+        migratedChunk.setVersion(origVersion);
+        migratedChunk.setShard(shard0.getName());
+        migratedChunk.setHistory({ChunkHistory(Timestamp(100, 0), shard0.getName())});
+        migratedChunk.setMin(BSON("a" << 1));
+        migratedChunk.setMax(BSON("a" << 10));
 
-    ChunkType chunk1;
-    chunk1.setNS(kNamespace);
-    chunk1.setVersion(origVersion);
-    chunk1.setShard(shard0.getName());
+        origVersion.incMinor();
 
-    Timestamp ctrlChunkValidAfter = Timestamp(50, 0);
-    chunk1.setHistory({ChunkHistory(ctrlChunkValidAfter, shard0.getName())});
+        controlChunk.setNS(kNamespace);
+        controlChunk.setVersion(origVersion);
+        controlChunk.setShard(shard0.getName());
+        controlChunk.setHistory({ChunkHistory(Timestamp(50, 0), shard0.getName())});
+        controlChunk.setMin(BSON("a" << 10));
+        controlChunk.setMax(BSON("a" << 20));
+        controlChunk.setJumbo(true);
+    }
 
-    chunk1.setMin(chunkMax);
-    auto chunkMaxax = BSON("a" << 20);
-    chunk1.setMax(chunkMaxax);
-
-    setupChunks({chunk0, chunk1}).transitional_ignore();
-
-    // use crefs to verify it will take consts:
-    ChunkType const& chunk0cref = chunk0;
+    setupChunks({migratedChunk, controlChunk}).transitional_ignore();
 
     Timestamp validAfter{101, 0};
-
-    StatusWith<BSONObj> resultBSON = ShardingCatalogManager::get(operationContext())
-                                         ->commitChunkMigration(operationContext(),
-                                                                chunk0.getNS(),
-                                                                chunk0cref,
-                                                                origVersion.epoch(),
-                                                                ShardId(shard0.getName()),
-                                                                ShardId(shard1.getName()),
-                                                                validAfter);
-
-    ASSERT_OK(resultBSON.getStatus());
+    BSONObj versions = assertGet(ShardingCatalogManager::get(operationContext())
+                                     ->commitChunkMigration(operationContext(),
+                                                            kNamespace,
+                                                            migratedChunk,
+                                                            migratedChunk.getVersion().epoch(),
+                                                            ShardId(shard0.getName()),
+                                                            ShardId(shard1.getName()),
+                                                            validAfter));
 
     // Verify the versions returned match expected values.
-    BSONObj versions = resultBSON.getValue();
-    auto mver = ChunkVersion::parseWithField(versions, "migratedChunkVersion");
-    ASSERT_OK(mver.getStatus());
-    ASSERT_EQ(ChunkVersion(origMajorVersion + 1, 0, origVersion.epoch()), mver.getValue());
-
-    auto cver = ChunkVersion::parseWithField(versions, "controlChunkVersion");
-    ASSERT_OK(cver.getStatus());
-    ASSERT_EQ(ChunkVersion(origMajorVersion + 1, 1, origVersion.epoch()), cver.getValue());
+    auto mver = assertGet(ChunkVersion::parseWithField(versions, "migratedChunkVersion"));
+    ASSERT_EQ(ChunkVersion(migratedChunk.getVersion().majorVersion() + 1,
+                           0,
+                           migratedChunk.getVersion().epoch()),
+              mver);
+
+    auto cver = assertGet(ChunkVersion::parseWithField(versions, "controlChunkVersion"));
+    ASSERT_EQ(ChunkVersion(migratedChunk.getVersion().majorVersion() + 1,
+                           1,
+                           migratedChunk.getVersion().epoch()),
+              cver);
 
     // Verify the chunks ended up in the right shards, and versions match the values returned.
-    auto chunkDoc0 = uassertStatusOK(getChunkDoc(operationContext(), chunkMin));
+    auto chunkDoc0 = uassertStatusOK(getChunkDoc(operationContext(), migratedChunk.getMin()));
     ASSERT_EQ("shard1", chunkDoc0.getShard().toString());
-    ASSERT_EQ(mver.getValue(), chunkDoc0.getVersion());
+    ASSERT_EQ(mver, chunkDoc0.getVersion());
 
     // The migrated chunk's history should be updated.
     ASSERT_EQ(2UL, chunkDoc0.getHistory().size());
     ASSERT_EQ(validAfter, chunkDoc0.getHistory().front().getValidAfter());
 
-    auto chunkDoc1 = uassertStatusOK(getChunkDoc(operationContext(), chunkMax));
+    auto chunkDoc1 = uassertStatusOK(getChunkDoc(operationContext(), controlChunk.getMin()));
     ASSERT_EQ("shard0", chunkDoc1.getShard().toString());
-    ASSERT_EQ(cver.getValue(), chunkDoc1.getVersion());
+    ASSERT_EQ(cver, chunkDoc1.getVersion());
 
-    // The control chunk's history should be unchanged.
+    // The control chunk's history and jumbo status should be unchanged.
     ASSERT_EQ(1UL, chunkDoc1.getHistory().size());
-    ASSERT_EQ(ctrlChunkValidAfter, chunkDoc1.getHistory().front().getValidAfter());
+    ASSERT_EQ(controlChunk.getHistory().front().getValidAfter(),
+              chunkDoc1.getHistory().front().getValidAfter());
+    ASSERT_EQ(controlChunk.getHistory().front().getShard(),
+              chunkDoc1.getHistory().front().getShard());
+    ASSERT(chunkDoc1.getJumbo());
 }
 
-TEST_F(CommitChunkMigrate, CheckCorrectOpsCommandNoCtl) {
+TEST_F(CommitChunkMigrate, ChunksUpdatedCorrectlyWithoutControlChunk) {
 
     ShardType shard0;
     shard0.setName("shard0");