SERVER-31979 Include chunk migration statistics in moveChunk.commit in changelog

7 files changed, 59 insertions, 8 deletions

diff --git a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
index c5dc6e8b463..5346a05e3f8 100644
--- a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
+++ b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
@@ -18,6 +18,7 @@ selector:
   # 3.6 mongods have improved covering behavior for queries on shard key index. Un-blacklist when
   # 3.6 becomes 'last-stable'.
   - jstests/sharding/covered_shard_key_indexes.js
+  - jstests/sharding/movechunk_commit_changelog_stats.js
   # Enable when 3.6 becomes last-stable.
   - jstests/sharding/configsvr_metadata_commands_require_majority_write_concern.js
   - jstests/sharding/views.js
diff --git a/jstests/sharding/movechunk_commit_changelog_stats.js b/jstests/sharding/movechunk_commit_changelog_stats.js
new file mode 100644
index 00000000000..cff5ae2a445
--- /dev/null
+++ b/jstests/sharding/movechunk_commit_changelog_stats.js
@@ -0,0 +1,40 @@
+//
+// Tests that the changelog entry for moveChunk.commit contains stats on the migration.
+//
+
+(function() {
+    'use strict';
+
+    var st = new ShardingTest({mongos: 1, shards: 2});
+    var kDbName = 'db';
+
+    var mongos = st.s0;
+    var shard0 = st.shard0.shardName;
+    var shard1 = st.shard1.shardName;
+
+    assert.commandWorked(mongos.adminCommand({enableSharding: kDbName}));
+    st.ensurePrimaryShard(kDbName, shard0);
+
+    function assertCountsInChangelog() {
+        let changeLog = st.s.getDB('config').changelog.find({what: 'moveChunk.commit'}).toArray();
+        assert.gt(changeLog.length, 0);
+        for (let i = 0; i < changeLog.length; i++) {
+            assert(changeLog[i].details.hasOwnProperty('counts'));
+        }
+    }
+
+    var ns = kDbName + '.fooHashed';
+    assert.commandWorked(mongos.adminCommand({shardCollection: ns, key: {_id: 'hashed'}}));
+
+    var aChunk = mongos.getDB('config').chunks.findOne({_id: RegExp(ns), shard: shard0});
+    assert(aChunk);
+
+    // Assert counts field exists in the changelog entry for moveChunk.commit
+    assert.commandWorked(
+        mongos.adminCommand({moveChunk: ns, bounds: [aChunk.min, aChunk.max], to: shard1}));
+    assertCountsInChangelog();
+
+    mongos.getDB(kDbName).fooHashed.drop();
+
+    st.stop();
+})();
\ No newline at end of file
diff --git a/src/mongo/db/s/migration_chunk_cloner_source.h b/src/mongo/db/s/migration_chunk_cloner_source.h
index ff3d31132fa..75dd433c702 100644
--- a/src/mongo/db/s/migration_chunk_cloner_source.h
+++ b/src/mongo/db/s/migration_chunk_cloner_source.h
@@ -93,9 +93,12 @@ public:
      * This must only be called once and no more methods on the cloner must be used afterwards
      * regardless of whether it succeeds or not.
      *
+     * Returns statistics about the move. These are informational only and should not be
+     * interpreted by the caller for any means other than reporting.
+     *
      * NOTE: Must be called without any locks.
      */
-    virtual Status commitClone(OperationContext* opCtx) = 0;
+    virtual StatusWith<BSONObj> commitClone(OperationContext* opCtx) = 0;
 
     /**
      * Tells the recipient to abort the clone and cleanup any unused data. This method's
diff --git a/src/mongo/db/s/migration_chunk_cloner_source_legacy.cpp b/src/mongo/db/s/migration_chunk_cloner_source_legacy.cpp
index 42bb184d83a..18a73342a39 100644
--- a/src/mongo/db/s/migration_chunk_cloner_source_legacy.cpp
+++ b/src/mongo/db/s/migration_chunk_cloner_source_legacy.cpp
@@ -334,7 +334,7 @@ Status MigrationChunkClonerSourceLegacy::awaitUntilCriticalSectionIsAppropriate(
     return {ErrorCodes::ExceededTimeLimit, "Timed out waiting for the cloner to catch up"};
 }
 
-Status MigrationChunkClonerSourceLegacy::commitClone(OperationContext* opCtx) {
+StatusWith<BSONObj> MigrationChunkClonerSourceLegacy::commitClone(OperationContext* opCtx) {
     invariant(_state == kCloning);
     invariant(!opCtx->lockState()->isLocked());
 
@@ -348,7 +348,8 @@ Status MigrationChunkClonerSourceLegacy::commitClone(OperationContext* opCtx) {
                     "destination shard finished committing but there are still some session "
                     "metadata that needs to be transferred"};
         }
-        return Status::OK();
+
+        return responseStatus;
     }
 
     cancelClone(opCtx);
diff --git a/src/mongo/db/s/migration_chunk_cloner_source_legacy.h b/src/mongo/db/s/migration_chunk_cloner_source_legacy.h
index 1bdad9ee4c7..3755a8b37ad 100644
--- a/src/mongo/db/s/migration_chunk_cloner_source_legacy.h
+++ b/src/mongo/db/s/migration_chunk_cloner_source_legacy.h
@@ -68,7 +68,7 @@ public:
     Status awaitUntilCriticalSectionIsAppropriate(OperationContext* opCtx,
                                                   Milliseconds maxTimeToWait) override;
 
-    Status commitClone(OperationContext* opCtx) override;
+    StatusWith<BSONObj> commitClone(OperationContext* opCtx) override;
 
     void cancelClone(OperationContext* opCtx) override;
 
diff --git a/src/mongo/db/s/migration_source_manager.cpp b/src/mongo/db/s/migration_source_manager.cpp
index 5ed5f2e66b6..f4325d74466 100644
--- a/src/mongo/db/s/migration_source_manager.cpp
+++ b/src/mongo/db/s/migration_source_manager.cpp
@@ -360,7 +360,7 @@ Status MigrationSourceManager::commitChunkOnRecipient(OperationContext* opCtx) {
     auto scopedGuard = MakeGuard([&] { cleanupOnError(opCtx); });
 
     // Tell the recipient shard to fetch the latest changes.
-    Status commitCloneStatus = _cloneDriver->commitClone(opCtx);
+    auto commitCloneStatus = _cloneDriver->commitClone(opCtx);
 
     if (MONGO_FAIL_POINT(failMigrationCommit) && commitCloneStatus.isOK()) {
         commitCloneStatus = {ErrorCodes::InternalError,
@@ -368,10 +368,11 @@ Status MigrationSourceManager::commitChunkOnRecipient(OperationContext* opCtx) {
     }
 
     if (!commitCloneStatus.isOK()) {
-        return {commitCloneStatus.code(),
-                str::stream() << "commit clone failed due to " << commitCloneStatus.toString()};
+        return commitCloneStatus.getStatus().withContext("commit clone failed");
     }
 
+    _recipientCloneCounts = commitCloneStatus.getValue()["counts"].Obj().getOwned();
+
     _state = kCloneCompleted;
     scopedGuard.Dismiss();
     return Status::OK();
@@ -575,7 +576,9 @@ Status MigrationSourceManager::commitChunkMetadataOnConfig(OperationContext* opC
                     BSON("min" << _args.getMinKey() << "max" << _args.getMaxKey() << "from"
                                << _args.getFromShardId()
                                << "to"
-                               << _args.getToShardId()),
+                               << _args.getToShardId()
+                               << "counts"
+                               << _recipientCloneCounts),
                     ShardingCatalogClient::kMajorityWriteConcern)
         .ignore();
 
diff --git a/src/mongo/db/s/migration_source_manager.h b/src/mongo/db/s/migration_source_manager.h
index d6a6df981c0..e052e6b95b3 100644
--- a/src/mongo/db/s/migration_source_manager.h
+++ b/src/mongo/db/s/migration_source_manager.h
@@ -242,6 +242,9 @@ private:
     // The transition from false to true is protected by the collection X-lock, which happens just
     // before the config server metadata commit is scheduled.
     bool _readsShouldWaitOnCritSec{false};
+
+    // The statistics about a chunk migration to be included in moveChunk.commit
+    BSONObj _recipientCloneCounts;
 };
 
 }  // namespace mongo