SERVER-13001: fixup mongos side with mixed version shards

(cherry picked from commit 971bbb28a0c25ae26abe9da38e2b02b4cac36d9b)

6 files changed, 160 insertions, 8 deletions

diff --git a/jstests/multiVersion/batch_write_commands_update_sharded.js b/jstests/multiVersion/batch_write_commands_update_sharded.js
new file mode 100644
index 00000000000..310da28d562
--- /dev/null
+++ b/jstests/multiVersion/batch_write_commands_update_sharded.js
@@ -0,0 +1,131 @@
+/**
+ * This test checks update write command corner cases:
+ *   -- nModified behavior
+ *   -- other?
+ */
+
+// Options for a cluster with two one-node replica set shards, shard0 is v2.4, shard1 is v2.6
+var options = { separateConfig : true,
+                rs : true,
+                configOptions : { binVersion : "2.6" },
+                rsOptions : { nodes : 1,
+                              nojournal : "" },
+                // Options for each replica set shard
+                rs0 : { binVersion : "2.4" },
+                rs1 : { binVersion : "2.6" },
+                mongosOptions : { binVersion : "2.6" } };
+
+var st = new ShardingTest({ shards : 2, mongos : 1, other : options });
+st.stopBalancer();
+
+var mongos = st.s0;
+var admin = mongos.getDB("admin");
+var shards = mongos.getCollection("config.shards").find().toArray();
+var coll24 = mongos.getCollection("test24.coll24");
+var collMixed = mongos.getCollection("testMixed.collMixed");
+var coll26 = mongos.getCollection("test26.coll26");
+
+// Move the primary of the collections to the appropriate shards
+coll24.drop();
+collMixed.drop();
+coll26.drop();
+printjson(admin.runCommand({ movePrimary : coll24.getDB().toString(),
+                             to : shards[0]._id }));
+printjson(admin.runCommand({ movePrimary : collMixed.getDB().toString(),
+                             to : shards[0]._id }));
+printjson(admin.runCommand({ movePrimary : coll26.getDB().toString(),
+                             to : shards[1]._id }));
+
+// The mixed collection spans shards
+assert.commandWorked(admin.runCommand({ enableSharding : collMixed.getDB().toString() }));
+assert.commandWorked(admin.runCommand({ shardCollection : collMixed.toString(),
+                                        key : { _id : 1 } }));
+assert.commandWorked(admin.runCommand({ split : collMixed.toString(),
+                                        middle : { _id : 0 } }));
+assert.commandWorked(admin.runCommand({ moveChunk : collMixed.toString(),
+                                        find : { _id : 0 },
+                                        to : shards[1]._id }));
+
+st.printShardingStatus();
+
+
+// Add test data, 20 docs per collection with _id:[-5, 5]
+var colls = [coll24, collMixed, coll26]
+colls.forEach(function(coll) {
+    for(x = -5; x <= 5; x++) {
+        coll.insert({_id:x}, {writeConcern:{w:1}});
+    }
+    assert.eq(11, coll.count());
+})
+
+// Test nModified behavior
+
+// 2.6 mongod shard
+var req = {update:coll26.getName(),
+           updates:[
+                {q:{}, u:{$inc:{a:1}}, multi:true},
+                {q:{}, u:{$set:{a:1}}, multi:true}
+           ]
+       };
+var res = assert.commandWorked(coll26.getDB().runCommand(req));
+assert.eq(11, res.nModified, "coll26: " + tojson(res))
+assert.eq(22, res.n,  "coll26: " + tojson(res))
+
+// 2.4 mongod shard
+var req = {update:coll24.getName(),
+           updates:[
+                {q:{}, u:{$set:{a:1}}, multi:true},
+                {q:{}, u:{$set:{a:1}}, multi:true}
+           ]
+       };
+var res = assert.commandWorked(coll24.getDB().runCommand(req));
+assert.eq(undefined, res.nModified, tojson(res))
+assert.eq(22, res.n, tojson(res))
+
+// mixed version mongod shards
+var req = {update:collMixed.getName(),
+           updates:[
+                {q:{}, u:{$set:{a:1}}, multi:true},
+                {q:{}, u:{$set:{a:1}}, multi:true}
+           ]
+       };
+var res = assert.commandWorked(collMixed.getDB().runCommand(req));
+assert.eq(undefined, res.nModified, tojson(res))
+assert.eq(22, res.n, tojson(res))
+
+// mixed version mongod shards, only hitting 2.6
+var req = {update:collMixed.getName(),
+           updates:[
+                {q:{_id:1}, u:{$set:{a:1}}},
+           ]
+       };
+var res = assert.commandWorked(collMixed.getDB().runCommand(req));
+assert.eq(0, res.nModified, tojson(res))
+assert.eq(1, res.n, tojson(res))
+
+
+// mixed version mongod shards, only hitting 2.4
+var req = {update:collMixed.getName(),
+           updates:[
+                {q:{_id:-1}, u:{$set:{a:1}}},
+           ]
+       };
+var res = assert.commandWorked(collMixed.getDB().runCommand(req));
+assert.eq(undefined, res.nModified, tojson(res))
+assert.eq(1, res.n, tojson(res))
+
+// mixed version mongod shards, only hitting both
+var req = {update:collMixed.getName(),
+           updates:[
+                {q:{_id:-1}, u:{$set:{a:1}}},
+                {q:{_id:1}, u:{$set:{a:1}}},
+           ]
+       };
+var res = assert.commandWorked(collMixed.getDB().runCommand(req));
+assert.eq(undefined, res.nModified, tojson(res))
+assert.eq(2, res.n, tojson(res))
+
+st.stop();
+
+jsTest.log("DONE!");
+
diff --git a/src/mongo/s/write_ops/batch_downconvert.cpp b/src/mongo/s/write_ops/batch_downconvert.cpp
index 068ab1599aa..cdfa9264cac 100644
--- a/src/mongo/s/write_ops/batch_downconvert.cpp
+++ b/src/mongo/s/write_ops/batch_downconvert.cpp
@@ -180,6 +180,9 @@ namespace mongo {
         // N starts at zero, and we add to it for each item
         response->setN( 0 );
 
+        // GLE path always sets nModified to -1 (sentinel) to indicate we should omit it later.
+        response->setNModified(-1);
+
         for ( size_t i = 0; i < request.sizeWriteOps(); ++i ) {
 
             // Break on first error if we're ordered
diff --git a/src/mongo/s/write_ops/batch_write_op.cpp b/src/mongo/s/write_ops/batch_write_op.cpp
index 0a3dfd8a055..8f0688eaef2 100644
--- a/src/mongo/s/write_ops/batch_write_op.cpp
+++ b/src/mongo/s/write_ops/batch_write_op.cpp
@@ -449,7 +449,13 @@ namespace mongo {
                 numUpserted = response.sizeUpsertDetails();
             }
             stats->numMatched += ( response.getN() - numUpserted );
-            stats->numModified += response.getNModified();
+            long long numModified = response.getNModified();
+
+            if (numModified >= 0)
+                stats->numModified += numModified;
+            else
+                stats->numModified = -1; // sentinel used to indicate we omit the field downstream
+
             stats->numUpserted += numUpserted;
         }
         else {
@@ -740,8 +746,10 @@ namespace mongo {
         int nValue = _stats->numInserted + _stats->numUpserted + _stats->numMatched
                      + _stats->numDeleted;
         batchResp->setN( nValue );
-        if ( _clientRequest->getBatchType() == BatchedCommandRequest::BatchType_Update )
-            batchResp->setNModified( _stats->numModified );
+        if ( _clientRequest->getBatchType() == BatchedCommandRequest::BatchType_Update &&
+             _stats->numModified >= 0) {
+                batchResp->setNModified( _stats->numModified );
+        }
 
         dassert( batchResp->isValid( NULL ) );
     }
diff --git a/src/mongo/s/write_ops/batch_write_op.h b/src/mongo/s/write_ops/batch_write_op.h
index cf058a5ec23..0237bb6185d 100644
--- a/src/mongo/s/write_ops/batch_write_op.h
+++ b/src/mongo/s/write_ops/batch_write_op.h
@@ -192,6 +192,16 @@ namespace mongo {
         int numModified;
         int numDeleted;
 
+        std::string toString() const {
+            StringBuilder str;
+            str << "numInserted: " << numInserted
+                << " numUpserted: " << numUpserted
+                << " numMatched: " << numMatched
+                << " numModified: " << numModified
+                << " numDeleted: " << numDeleted;
+            return str.str();
+        }
+
     };
 
     /**
diff --git a/src/mongo/s/write_ops/batched_command_response.cpp b/src/mongo/s/write_ops/batched_command_response.cpp
index 7c15c04196d..f412f6d59d9 100644
--- a/src/mongo/s/write_ops/batched_command_response.cpp
+++ b/src/mongo/s/write_ops/batched_command_response.cpp
@@ -39,7 +39,7 @@ namespace mongo {
     const BSONField<int> BatchedCommandResponse::errCode("code", ErrorCodes::UnknownError);
     const BSONField<string> BatchedCommandResponse::errMessage("errmsg");
     const BSONField<long long> BatchedCommandResponse::n("n", 0);
-    const BSONField<long long> BatchedCommandResponse::nModified("nModified", -1);
+    const BSONField<long long> BatchedCommandResponse::nModified("nModified", 0);
     const BSONField<std::vector<BatchedUpsertDetail*> >
         BatchedCommandResponse::upsertDetails("upserted");
     const BSONField<OpTime> BatchedCommandResponse::lastOp("lastOp");
@@ -81,7 +81,7 @@ namespace mongo {
 
         if (_isErrMessageSet) builder.append(errMessage(), _errMessage);
 
-        if (_isNModifiedSet && _nModified > -1) builder.appendNumber(nModified(), _nModified);
+        if (_isNModifiedSet) builder.appendNumber(nModified(), _nModified);
         if (_isNSet) builder.appendNumber(n(), _n);
 
         if (_upsertDetails.get()) {
@@ -203,7 +203,7 @@ namespace mongo {
         _errMessage.clear();
         _isErrMessageSet = false;
 
-        _nModified = -1;
+        _nModified = 0;
         _isNModifiedSet = false;
 
         _n = 0;
diff --git a/src/mongo/shell/assert.js b/src/mongo/shell/assert.js
index fcd7b4e59bb..711f74a6102 100644
--- a/src/mongo/shell/assert.js
+++ b/src/mongo/shell/assert.js
@@ -240,7 +240,7 @@ assert.commandWorked = function(res, msg){
     if (assert._debug && msg) print("in assert for: " + msg);
 
     if (res.ok == 1)
-        return;
+        return res;
     doassert("command failed: " + tojson(res) + " : " + msg);
 }
 
@@ -248,7 +248,7 @@ assert.commandFailed = function(res, msg){
     if (assert._debug && msg) print("in assert for: " + msg);
 
     if (res.ok == 0)
-        return;
+        return res;
     doassert("command worked when it should have failed: " + tojson(res) + " : " + msg);
 }