diff options
author | Scott Hernandez <scotthernandez@gmail.com> | 2014-03-18 18:40:41 -0400 |
---|---|---|
committer | Scott Hernandez <scotthernandez@gmail.com> | 2014-03-19 19:04:23 -0400 |
commit | 1c5e4d72d290388f2d1cb7e60c9f1e63aa24893c (patch) | |
tree | b690eb8a281faf0d57bbe1d930e9b9bc509f2cfa | |
parent | 670a13d965a5bf949e33309ef47b49a23326a59c (diff) | |
download | mongo-1c5e4d72d290388f2d1cb7e60c9f1e63aa24893c.tar.gz |
SERVER-13001: fixup mongos side with mixed version shards
(cherry picked from commit 971bbb28a0c25ae26abe9da38e2b02b4cac36d9b)
-rw-r--r-- | jstests/multiVersion/batch_write_commands_update_sharded.js | 131 | ||||
-rw-r--r-- | src/mongo/s/write_ops/batch_downconvert.cpp | 3 | ||||
-rw-r--r-- | src/mongo/s/write_ops/batch_write_op.cpp | 14 | ||||
-rw-r--r-- | src/mongo/s/write_ops/batch_write_op.h | 10 | ||||
-rw-r--r-- | src/mongo/s/write_ops/batched_command_response.cpp | 6 | ||||
-rw-r--r-- | src/mongo/shell/assert.js | 4 |
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); } |