SERVER-26562 Move CommitChunkMigration cmd impl to catalog, prep for unit tests

3 files changed, 74 insertions, 84 deletions

diff --git a/src/mongo/s/request_types/commit_chunk_migration_request_test.cpp b/src/mongo/s/request_types/commit_chunk_migration_request_test.cpp
index f0563fa76e1..d9201693f2a 100644
--- a/src/mongo/s/request_types/commit_chunk_migration_request_test.cpp
+++ b/src/mongo/s/request_types/commit_chunk_migration_request_test.cpp
@@ -55,23 +55,23 @@ const char kConfigSvrCommitChunkMigration[] = "_configsvrCommitChunkMigration";
 TEST(CommitChunkMigrationRequest, WithControlChunk) {
     BSONObjBuilder builder;
 
-    ChunkType migratedChunkType;
-    migratedChunkType.setMin(kKey0);
-    migratedChunkType.setMax(kKey1);
+    ChunkVersion fromShardCollectionVersion(1, 2, OID::gen());
 
-    ChunkType controlChunkTypeTemp;
-    controlChunkTypeTemp.setMin(kKey2);
-    controlChunkTypeTemp.setMax(kKey3);
-    boost::optional<ChunkType> controlChunkType = std::move(controlChunkTypeTemp);
+    ChunkType migratedChunk;
+    migratedChunk.setMin(kKey0);
+    migratedChunk.setMax(kKey1);
 
-    ChunkVersion fromShardCollectionVersion(1, 2, OID::gen());
+    ChunkType controlChunk;
+    controlChunk.setMin(kKey2);
+    controlChunk.setMax(kKey3);
+    boost::optional<ChunkType> controlChunkOpt = controlChunk;
 
     CommitChunkMigrationRequest::appendAsCommand(&builder,
                                                  kNamespaceString,
                                                  kShardId0,
                                                  kShardId1,
-                                                 migratedChunkType,
-                                                 controlChunkType,
+                                                 migratedChunk,
+                                                 controlChunkOpt,
                                                  fromShardCollectionVersion,
                                                  kShardHasDistributedLock);
 
@@ -83,21 +83,21 @@ TEST(CommitChunkMigrationRequest, WithControlChunk) {
     ASSERT_EQ(kNamespaceString, request.getNss());
     ASSERT_EQ(kShardId0, request.getFromShard());
     ASSERT_EQ(kShardId1, request.getToShard());
-    ASSERT_BSONOBJ_EQ(kKey0, request.getMigratedChunkRange().getMin());
-    ASSERT_BSONOBJ_EQ(kKey1, request.getMigratedChunkRange().getMax());
-    ASSERT(request.hasControlChunkRange());
-    ASSERT_BSONOBJ_EQ(kKey2, request.getControlChunkRange().getMin());
-    ASSERT_BSONOBJ_EQ(kKey3, request.getControlChunkRange().getMax());
-    ASSERT_EQ(fromShardCollectionVersion, request.getFromShardCollectionVersion());
+    ASSERT_BSONOBJ_EQ(kKey0, request.getMigratedChunk().getMin());
+    ASSERT_BSONOBJ_EQ(kKey1, request.getMigratedChunk().getMax());
+    ASSERT(request.getControlChunk());
+    ASSERT_BSONOBJ_EQ(kKey2, request.getControlChunk()->getMin());
+    ASSERT_BSONOBJ_EQ(kKey3, request.getControlChunk()->getMax());
+    ASSERT_EQ(fromShardCollectionVersion.epoch(), request.getCollectionEpoch());
     ASSERT_EQ(kShardHasDistributedLock, request.shardHasDistributedLock());
 }
 
 TEST(CommitChunkMigrationRequest, WithoutControlChunk) {
     BSONObjBuilder builder;
 
-    ChunkType migratedChunkType;
-    migratedChunkType.setMin(kKey0);
-    migratedChunkType.setMax(kKey1);
+    ChunkType migratedChunk;
+    migratedChunk.setMin(kKey0);
+    migratedChunk.setMax(kKey1);
 
     ChunkVersion fromShardCollectionVersion(1, 2, OID::gen());
 
@@ -105,7 +105,7 @@ TEST(CommitChunkMigrationRequest, WithoutControlChunk) {
                                                  kNamespaceString,
                                                  kShardId0,
                                                  kShardId1,
-                                                 migratedChunkType,
+                                                 migratedChunk,
                                                  boost::none,
                                                  fromShardCollectionVersion,
                                                  kShardHasDistributedLock);
@@ -118,10 +118,10 @@ TEST(CommitChunkMigrationRequest, WithoutControlChunk) {
     ASSERT_EQ(kNamespaceString, request.getNss());
     ASSERT_EQ(kShardId0, request.getFromShard());
     ASSERT_EQ(kShardId1, request.getToShard());
-    ASSERT_BSONOBJ_EQ(kKey0, request.getMigratedChunkRange().getMin());
-    ASSERT_BSONOBJ_EQ(kKey1, request.getMigratedChunkRange().getMax());
-    ASSERT(!request.hasControlChunkRange());
-    ASSERT_EQ(fromShardCollectionVersion, request.getFromShardCollectionVersion());
+    ASSERT_BSONOBJ_EQ(kKey0, request.getMigratedChunk().getMin());
+    ASSERT_BSONOBJ_EQ(kKey1, request.getMigratedChunk().getMax());
+    ASSERT(!request.getControlChunk());
+    ASSERT_EQ(fromShardCollectionVersion.epoch(), request.getCollectionEpoch());
     ASSERT_EQ(kShardHasDistributedLock, request.shardHasDistributedLock());
 }
 
diff --git a/src/mongo/s/request_types/commit_chunk_migration_request_type.cpp b/src/mongo/s/request_types/commit_chunk_migration_request_type.cpp
index 94cfd070e7c..4b9fbfd149d 100644
--- a/src/mongo/s/request_types/commit_chunk_migration_request_type.cpp
+++ b/src/mongo/s/request_types/commit_chunk_migration_request_type.cpp
@@ -44,15 +44,22 @@ const char kFromShardCollectionVersion[] = "fromShardCollectionVersion";
 const char kShardHasDistributedLock[] = "shardHasDistributedLock";
 
 /**
- * Attempts to parse a ChunkRange from "field" in "source".
+ * Attempts to parse a (range-only!) ChunkType from "field" in "source".
  */
-StatusWith<ChunkRange> extractChunkRange(const BSONObj& source, StringData field) {
+StatusWith<ChunkType> extractChunk(const BSONObj& source, StringData field) {
     BSONElement fieldElement;
     auto status = bsonExtractTypedField(source, field, BSONType::Object, &fieldElement);
     if (!status.isOK())
         return status;
 
-    return ChunkRange::fromBSON(fieldElement.Obj());
+    auto rangeWith = ChunkRange::fromBSON(fieldElement.Obj());
+    if (!rangeWith.isOK())
+        return rangeWith.getStatus();
+
+    ChunkType chunk;
+    chunk.setMin(rangeWith.getValue().getMin());
+    chunk.setMax(rangeWith.getValue().getMax());
+    return chunk;
 }
 
 /**
@@ -79,12 +86,12 @@ StatusWith<ShardId> extractShardId(const BSONObj& source, StringData field) {
 StatusWith<CommitChunkMigrationRequest> CommitChunkMigrationRequest::createFromCommand(
     const NamespaceString& nss, const BSONObj& obj) {
 
-    auto migratedChunkRange = extractChunkRange(obj, kMigratedChunk);
-    if (!migratedChunkRange.isOK()) {
-        return migratedChunkRange.getStatus();
+    auto migratedChunk = extractChunk(obj, kMigratedChunk);
+    if (!migratedChunk.isOK()) {
+        return migratedChunk.getStatus();
     }
 
-    CommitChunkMigrationRequest request(nss, std::move(migratedChunkRange.getValue()));
+    CommitChunkMigrationRequest request(nss, std::move(migratedChunk.getValue()));
 
     {
         auto fromShard = extractShardId(obj, kFromShard);
@@ -107,23 +114,23 @@ StatusWith<CommitChunkMigrationRequest> CommitChunkMigrationRequest::createFromC
     {
         // controlChunk is optional, so parse it if present.
         if (obj.hasField(kControlChunk)) {
-            auto controlChunkRange = extractChunkRange(obj, kControlChunk);
-            if (!controlChunkRange.isOK()) {
-                return controlChunkRange.getStatus();
+            auto controlChunk = extractChunk(obj, kControlChunk);
+            if (!controlChunk.isOK()) {
+                return controlChunk.getStatus();
             }
 
-            request._controlChunkRange = std::move(controlChunkRange.getValue());
+            request._controlChunk = std::move(controlChunk.getValue());
         }
     }
 
     {
         auto statusWithChunkVersion =
             ChunkVersion::parseFromBSONWithFieldForCommands(obj, kFromShardCollectionVersion);
-        if (statusWithChunkVersion.isOK()) {
-            request._fromShardCollectionVersion = std::move(statusWithChunkVersion.getValue());
-        } else if (statusWithChunkVersion != ErrorCodes::NoSuchKey) {
+        if (!statusWithChunkVersion.isOK()) {
             return statusWithChunkVersion.getStatus();
         }
+
+        request._collectionEpoch = statusWithChunkVersion.getValue().epoch();
     }
 
     {
@@ -137,37 +144,27 @@ StatusWith<CommitChunkMigrationRequest> CommitChunkMigrationRequest::createFromC
     return request;
 }
 
-void CommitChunkMigrationRequest::appendAsCommand(
-    BSONObjBuilder* builder,
-    const NamespaceString& nss,
-    const ShardId& fromShard,
-    const ShardId& toShard,
-    const ChunkType& migratedChunkType,
-    const boost::optional<ChunkType>& controlChunkType,
-    const ChunkVersion& fromShardCollectionVersion,
-    const bool& shardHasDistributedLock) {
+void CommitChunkMigrationRequest::appendAsCommand(BSONObjBuilder* builder,
+                                                  const NamespaceString& nss,
+                                                  const ShardId& fromShard,
+                                                  const ShardId& toShard,
+                                                  const ChunkType& migratedChunk,
+                                                  const boost::optional<ChunkType>& controlChunk,
+                                                  const ChunkVersion& fromShardCollectionVersion,
+                                                  const bool& shardHasDistributedLock) {
     invariant(builder->asTempObj().isEmpty());
     invariant(nss.isValid());
 
     builder->append(kConfigSvrCommitChunkMigration, nss.ns());
     builder->append(kFromShard, fromShard.toString());
     builder->append(kToShard, toShard.toString());
-    builder->append(kMigratedChunk, migratedChunkType.toBSON());
+    builder->append(kMigratedChunk, migratedChunk.toBSON());
     fromShardCollectionVersion.appendWithFieldForCommands(builder, kFromShardCollectionVersion);
     builder->append(kShardHasDistributedLock, shardHasDistributedLock);
 
-    if (controlChunkType) {
-        builder->append(kControlChunk, controlChunkType->toBSON());
+    if (controlChunk) {
+        builder->append(kControlChunk, controlChunk->toBSON());
     }
 }
 
-const ChunkRange& CommitChunkMigrationRequest::getControlChunkRange() const {
-    invariant(_controlChunkRange);
-    return _controlChunkRange.get();
-}
-
-CommitChunkMigrationRequest::CommitChunkMigrationRequest(const NamespaceString& nss,
-                                                         const ChunkRange& range)
-    : _nss(nss), _migratedChunkRange(range) {}
-
 }  // namespace mongo
diff --git a/src/mongo/s/request_types/commit_chunk_migration_request_type.h b/src/mongo/s/request_types/commit_chunk_migration_request_type.h
index 461278e429e..c1f97f5c7fc 100644
--- a/src/mongo/s/request_types/commit_chunk_migration_request_type.h
+++ b/src/mongo/s/request_types/commit_chunk_migration_request_type.h
@@ -38,8 +38,11 @@ namespace mongo {
 /**
  * Creates and parses commit chunk migration command BSON objects.
  */
-class CommitChunkMigrationRequest {
-public:
+struct CommitChunkMigrationRequest {
+
+    CommitChunkMigrationRequest(const NamespaceString& nss, const ChunkType& chunk)
+        : _nss(nss), _migratedChunk(chunk), _shardHasDistributedLock() {}
+
     /**
      * Parses the input command and produces a request corresponding to its arguments.
      */
@@ -63,35 +66,26 @@ public:
     const NamespaceString& getNss() const {
         return _nss;
     }
-
     const ShardId& getFromShard() const {
         return _fromShard;
     }
-
     const ShardId& getToShard() const {
         return _toShard;
     }
-
-    const ChunkRange& getMigratedChunkRange() const {
-        return _migratedChunkRange;
-    }
-
-    const ChunkRange& getControlChunkRange() const;
-
-    bool hasControlChunkRange() {
-        return bool(_controlChunkRange);
+    const ChunkType& getMigratedChunk() const {
+        return _migratedChunk;
     }
-
-    const ChunkVersion& getFromShardCollectionVersion() const {
-        return _fromShardCollectionVersion;
+    const boost::optional<ChunkType>& getControlChunk() const {
+        return _controlChunk;
     }
 
     bool shardHasDistributedLock() {
         return _shardHasDistributedLock;
     }
 
-private:
-    CommitChunkMigrationRequest(const NamespaceString& nss, const ChunkRange& range);
+    const OID& getCollectionEpoch() {
+        return _collectionEpoch;
+    }
 
     // The collection for which this request applies.
     NamespaceString _nss;
@@ -102,17 +96,16 @@ private:
     // The recipient shard name.
     ShardId _toShard;
 
-    // Range of migrated chunk being moved.
-    ChunkRange _migratedChunkRange;
-
-    // Range of control chunk being moved, if it exists.
-    boost::optional<ChunkRange> _controlChunkRange;
+    // The chunk being moved.
+    ChunkType _migratedChunk;
 
-    // Collection version of the source shard.
-    ChunkVersion _fromShardCollectionVersion;
+    // A chunk on the shard moved from, if any remain.
+    boost::optional<ChunkType> _controlChunk;
 
     // Flag to indicate whether the shard has the distlock.
     bool _shardHasDistributedLock;
+
+    OID _collectionEpoch;
 };
 
 }  // namespace mongo