SERVER-11681 mongos upconverts all writes by default

6 files changed, 69 insertions, 37 deletions

diff --git a/jstests/sharding/authCommands.js b/jstests/sharding/authCommands.js
index bd92fbd1f45..2027937ee53 100644
--- a/jstests/sharding/authCommands.js
+++ b/jstests/sharding/authCommands.js
@@ -117,6 +117,8 @@ var checkReadOps = function( hasReadAuth ) {
         print( "Checking read operations, should work" );
         assert.eq( 1000, testDB.foo.find().itcount() );
         assert.eq( 1000, testDB.foo.count() );
+        // NOTE: This is an explicit check that GLE can be run with read prefs, not the result of
+        // above.
         assert.eq( null, testDB.runCommand({getlasterror : 1}).err );
         checkCommandSucceeded( testDB, {dbstats : 1} );
         checkCommandSucceeded( testDB, {collstats : 'foo'} );
diff --git a/jstests/sharding/geo_shardedgeonear.js b/jstests/sharding/geo_shardedgeonear.js
index a59b4b87146..269531f4caa 100644
--- a/jstests/sharding/geo_shardedgeonear.js
+++ b/jstests/sharding/geo_shardedgeonear.js
@@ -27,7 +27,7 @@ function test(db, sharded, indexType) {
         var lat = 90 - Random.rand() * 180;
         var lng = 180 - Random.rand() * 360;
         db[coll].insert({rand:Math.random(), loc: [lng, lat]})
-        assert(!db.getLastError());
+        assert.eq(null, db.getLastError());
     }
     assert.eq(db[coll].count(), numPts);
 
diff --git a/jstests/sharding/gle_error_message.js b/jstests/sharding/gle_error_message.js
index d53cf5108ea..d02a9778db3 100644
--- a/jstests/sharding/gle_error_message.js
+++ b/jstests/sharding/gle_error_message.js
@@ -67,7 +67,8 @@ jsTest.log( "GLE : " + tojson( coll.getDB().getLastErrorObj() ) )
 
 st.shard0 = MongoRunner.runMongod( st.shard0 )
 
-
+// NEED TO WAIT 5s so the connection pool knows we're down and up
+sleep( 5 * 1000 );
 
 jsTest.log( "Testing GLE when main host goes down..." )
 
@@ -89,12 +90,13 @@ catch( e ){
     jsTest.log( "GLE : " + e )
     
     // Stupid string exceptions
-    assert( /could not get last error/.test( e + "") )
+    assert( /could not get last error|socket exception/.test( e + "") )
 }
 
 st.shard1 = MongoRunner.runMongod( st.shard1 )
 
-
+// NEED TO WAIT 5s so the connection pool knows we're down and up
+sleep( 5 * 1000 );
 
 jsTest.log( "Testing multi GLE for multi-host writes..." )
 
@@ -124,12 +126,13 @@ catch( e ){
     jsTest.log( "GLE : " + e )
     
     // Stupid string exceptions
-    assert( /could not get last error/.test( e + "") )
+    assert( /could not get last error|socket exception/.test( e + "") )
 }
 
 st.shard0 = MongoRunner.runMongod( st.shard0 )
 
-
+// NEED TO WAIT 5s so the connection pool knows we're down
+sleep( 5 * 1000 );
 
 jsTest.log( "Testing stale version GLE when host goes down..." )
 
diff --git a/jstests/sharding/shard_gle_insert.js b/jstests/sharding/shard_gle_insert.js
index a174d54e45d..f67d92709ba 100644
--- a/jstests/sharding/shard_gle_insert.js
+++ b/jstests/sharding/shard_gle_insert.js
@@ -29,8 +29,8 @@ try{
     
     // for example -- err : "socket exception [SEND_ERROR] for 127.0.0.1:30001"
     //  or            err : "socket exception [CONNECT_ERROR] for localhost:30001"
-    
-    if (err && !/socket exception/.test(err)) {
+
+    if (err && !/socket exception|connect failed/.test(err)) {
         gle_state = 1;
         print( "Test failure -- received response from getLastError:" + err );
     }
diff --git a/jstests/sharding/shard_key_immutable.js b/jstests/sharding/shard_key_immutable.js
index fbccc6fc809..956f5a69dcd 100644
--- a/jstests/sharding/shard_key_immutable.js
+++ b/jstests/sharding/shard_key_immutable.js
@@ -1,13 +1,36 @@
 /**
- * This file test that updates on a sharded collection obeys the shard key invariants.
- * The invariant boils down into these rules:
+ * Shard key invariant:
  *
- * 1. If the update is a replacement style update or an upsert, the full shard key must
- *    be present in the update object.
- * 2. If the update object contains a shard key, the value in the query object should
- *    be equal.
- * 3. If the update object contains a shard key but is not present in the query, then
- *    the matching documents should have the same value.
+ * A document must be created with a full non-array-or-regex shard key, and the value of that shard
+ * key can never change.
+ *
+ * To enforce this invariant, we have the following mongos rule:
+ *
+ * - Upserts must always contain the full shard key and must only be targeted* to the applicable shard.
+ *
+ * and the following mongod rules:
+ *
+ * - Upserted shard key values must not be arrays (or regexes).
+ * - If a shard key value is present in the update query, upserts must only insert documents which
+ *   match this value.
+ * - Updates must not modify shard keys.
+ *
+ * *Updates are targeted by the update query if $op-style, or the update document if replacement-style.
+ *
+ * NOTE: The above is enough to ensure that shard keys do not change.  It is not enough to ensure 
+ * uniqueness of an upserted document based on the upsert query.  This is necessary due to the save()
+ * style operation:
+ * db.coll.update({ _id : xxx }, { _id : xxx, shard : xxx, key : xxx, other : xxx }, { upsert : true })
+ *
+ * TODO: Minimize the impact of this hole by disallowing anything but save-style upserts of this form.
+ * Save-style upserts of this form are not safe (duplicate _ids can be created) but the user is
+ * explicitly responsible for this for the _id field.  
+ *
+ * In addition, there is an rule where non-multi updates can only affect 0 or 1 documents.
+ *
+ * To enforce this, we have the following mongos rule:
+ *
+ * - Non-multi updates must be targeted based on an exact _id query or the full shard key.
  *
  * Test setup:
  * - replacement style updates have the multiUpdate flag set to false.
@@ -401,14 +424,17 @@ doc = compoundColl.findOne();
 delete doc._id;
 assert(friendlyEqual(doc, { a: 100, b: 100 }), 'doc changed: ' + tojson(doc));
 
+// Inspecting query and update alone is not enough to tell whether a shard key will change.
+/*
 compoundColl.remove({}, false);
 compoundColl.insert({ a: 100, b: 100 });
 compoundColl.update({ b: 100 }, { a: 100 }, false);
 gle = db.runCommand({ getLastError: 1 });
-assert(gle.err != null, 'gleObj: ' + tojson(gle));
+assert(gle.err == null, 'gleObj: ' + tojson(gle));
 doc = compoundColl.findOne();
 delete doc._id;
 assert(friendlyEqual(doc, { a: 100, b: 100 }), 'doc changed: ' + tojson(doc));
+*/
 
 compoundColl.remove({}, false);
 compoundColl.insert({ a: 100, b: 100 });
@@ -629,19 +655,19 @@ compoundColl.remove({}, false);
 compoundColl.insert({ a: 100, b: 100 });
 compoundColl.update({ a: 100, b: 100 }, { $set: { b: 100, c: 1 }}, false, true);
 gle = db.runCommand({ getLastError: 1 });
-assert(gle.err != null, 'gleObj: ' + tojson(gle));
+assert(gle.err == null, 'gleObj: ' + tojson(gle));
 doc = compoundColl.findOne();
 delete doc._id;
-assert(friendlyEqual(doc, { a: 100, b: 100 }), 'doc changed: ' + tojson(doc));
+assert(friendlyEqual(doc, { a: 100, b: 100, c: 1 }), 'doc did not change: ' + tojson(doc));
 
 compoundColl.remove({}, false);
 compoundColl.insert({ a: 100, b: 100 });
 compoundColl.update({ a: 100, b: 100 }, { $set: { a: 100, b: 100, c: 1 }}, false, true);
 gle = db.runCommand({ getLastError: 1 });
-assert(gle.err != null, 'gleObj: ' + tojson(gle));
+assert(gle.err == null, 'gleObj: ' + tojson(gle));
 doc = compoundColl.findOne();
 delete doc._id;
-assert(friendlyEqual(doc, { a: 100, b: 100 }), 'doc changed: ' + tojson(doc));
+assert(friendlyEqual(doc, { a: 100, b: 100, c: 1 }), 'doc did not change: ' + tojson(doc));
 
 // Cannot modify _id!
 compoundColl.remove({}, false);
@@ -708,24 +734,24 @@ assert(doc == null, 'doc was upserted: ' + tojson(doc));
 compoundColl.remove({}, false);
 compoundColl.update({ a: 100, b: 100 }, { $set: { b: 100, c: 1 }}, true, true);
 gle = db.runCommand({ getLastError: 1 });
-assert(gle.err != null, 'gleObj: ' + tojson(gle));
+assert(gle.err == null, 'gleObj: ' + tojson(gle));
 doc = compoundColl.findOne();
-assert(doc == null, 'doc was upserted: ' + tojson(doc));
+assert(doc != null, 'doc was not upserted: ' + tojson(doc));
 
 compoundColl.remove({}, false);
 compoundColl.update({ a: 100, b: 100 }, { $set: { a: 100, b: 100, c: 1 }}, true, true);
 gle = db.runCommand({ getLastError: 1 });
-assert(gle.err != null, 'gleObj: ' + tojson(gle));
+assert(gle.err == null, 'gleObj: ' + tojson(gle));
 doc = compoundColl.findOne();
-assert(doc == null, 'doc was upserted: ' + tojson(doc));
+assert(doc != null, 'doc was not upserted: ' + tojson(doc));
 
-// Cannot modify _id!
+// Can upsert with new _id
 compoundColl.remove({}, false);
 compoundColl.update({ a: 100, b: 100 }, { $set: { a: 100, b: 100, _id: 1 }}, true, true);
 gle = db.runCommand({ getLastError: 1 });
-assert(gle.err != null, 'gleObj: ' + tojson(gle));
+assert(gle.err == null, 'gleObj: ' + tojson(gle));
 doc = compoundColl.findOne();
-assert(doc == null, 'doc was upserted: ' + tojson(doc));
+assert(doc != null, 'doc was not upserted: ' + tojson(doc));
 
 compoundColl.remove({}, false);
 compoundColl.update({ a: 100, b: 100 }, { $set: { a: 100, b: 2, c: 1 }}, true, true);
@@ -856,12 +882,15 @@ assert(gle.err != null, 'gleObj: ' + tojson(gle));
 doc = compoundColl.findOne();
 assert(doc == null, 'doc was upserted: ' + tojson(doc));
 
+/*
 compoundColl.remove({}, false);
 compoundColl.update({ _id: 1 }, { $set: { a: 1, b: 1 }}, true, true);
 gle = db.runCommand({ getLastError: 1 });
-assert(gle.err != null, 'gleObj: ' + tojson(gle));
+assert(gle.err == null, 'gleObj: ' + tojson(gle));
 doc = compoundColl.findOne();
-assert(doc == null, 'doc was upserted: ' + tojson(doc));
+delete doc._id;
+assert(friendlyEqual(doc, { a: 1, b: 1 }), 'bad doc: ' + tojson(doc));
+*/
 
 //
 // Dotted query update
@@ -871,10 +900,10 @@ dotColl.remove({}, false);
 dotColl.insert({ x: { a: 100 }});
 dotColl.update({ 'x.a': 100 }, { x: { a: 100, b: 2 }});
 gle = db.runCommand({ getLastError: 1 });
-assert(gle.err != null, 'gleObj: ' + tojson(gle));
+assert(gle.err == null, 'gleObj: ' + tojson(gle));
 doc = dotColl.findOne();
 delete doc._id;
-assert(friendlyEqual(doc, { x: { a: 100 }}), 'doc changed: ' + tojson(doc));
+assert(friendlyEqual(doc, { x: { a: 100, b: 2 }}), 'doc did not change: ' + tojson(doc));
 
 // Dotted field names in the resulting objects should not be allowed.
 // This check currently resides in the client drivers.
@@ -1001,9 +1030,9 @@ assert(friendlyEqual(doc, { x: { a: 100, b: 200 }}), 'doc did not change: ' + to
 dotColl.remove({}, false);
 dotColl.update({ 'x.a': 100 }, { x: { a: 100, b: 2 }}, true);
 gle = db.runCommand({ getLastError: 1 });
-assert(gle.err != null, 'gleObj: ' + tojson(gle));
+assert(gle.err == null, 'gleObj: ' + tojson(gle));
 doc = dotColl.findOne();
-assert(doc == null, 'doc was upserted: ' + tojson(doc));
+assert(doc != null, 'doc was not upserted: ' + tojson(doc));
 
 // Dotted field names in the resulting objects should not be allowed.
 // This check currently resides in the client drivers.
@@ -1047,7 +1076,7 @@ gle = db.runCommand({ getLastError: 1 });
 assert(gle.err == null, 'gleObj: ' + tojson(gle));
 doc = dotColl.findOne();
 delete doc._id;
-assert(friendlyEqual(doc, { x: { a: 100, b: 2 }}), 'bad doc: ' + tojson(doc));
+assert(friendlyEqual(doc, { x: { a: 100, 2: 3 }}), 'bad doc: ' + tojson(doc));
 */
 
 dotColl.remove({}, false);
diff --git a/jstests/sharding/update1.js b/jstests/sharding/update1.js
index 73fe8a8bbdc..96a1df2d861 100644
--- a/jstests/sharding/update1.js
+++ b/jstests/sharding/update1.js
@@ -45,8 +45,6 @@ for(i=0; i < 2; i++){
     coll.update({_id:1, key:1}, {$set: {key:2}});
     err = db.getLastErrorObj();
     assert.eq(coll.findOne({_id:1}).key, 1, 'key unchanged');
-    assert.eq(err.code, 13123, 'key error code 1');
-    assert.eq(err.code, 13123, 'key error code 2');
 
     coll.update({_id:1, key:1}, {$set: {foo:2}});
     assert.isnull(db.getLastError(), 'getLastError reset');