SERVER-7958 - when an old operation has a writeback,

              make sure not to use that info in the user writeback
              just block

5 files changed, 133 insertions, 15 deletions

diff --git a/jstests/sharding/writeback_server7958.js b/jstests/sharding/writeback_server7958.js
new file mode 100644
index 00000000000..20064ef53d1
--- /dev/null
+++ b/jstests/sharding/writeback_server7958.js
@@ -0,0 +1,94 @@
+jsTest.log("Starting sharded cluster for wrong duplicate error setup");
+
+s = new ShardingTest( name="writeback_server7958", shards = 2, verbose=0, mongos = 4 );
+
+var mongosA=s.s0;
+var mongosB=s.s1;
+var mongosC=s.s2;
+var mongosD=s.s3;
+
+ns1 = "test.trans";
+ns2 = "test.node";
+
+adminSA = mongosA.getDB( "admin" );
+adminSB = mongosB.getDB( "admin" );
+adminSD = mongosD.getDB( "admin" );
+adminSA.runCommand({ enableSharding : "test"});
+adminSA.runCommand({ shardCollection : ns1, key : { owner : 1 }, unique: true });
+//adminSA.runCommand({ shardCollection : ns1, key : { owner : 1 } });
+
+try {
+   s.stopBalancer();
+} catch (e) {
+   print("coundn't stop balancer via command");
+}
+
+adminSA.settings.update({ _id: 'balancer' }, { $set: { stopped: true }});
+
+var db = mongosA.getDB( "test" );
+var dbB = mongosB.getDB( "test" );
+var dbC = mongosC.getDB( "test" );
+var dbD = mongosD.getDB( "test" );
+var trans = db.trans;
+var node = db.node;
+var transB = dbB.trans;
+var nodeB = dbB.node;
+var transC = dbC.trans;
+var nodeC = dbC.node;
+var transD = dbD.trans;
+var nodeD = dbD.node;
+
+var primary = s.getServerName("test");
+var shard1 = s._shardNames[0];
+var shard2 = s._shardNames[1];
+if (primary == shard1) {
+    other = shard2;
+} else {
+    other = shard1;
+}
+
+
+trans.insert({"owner":NumberLong("1234567890"),"tid":"2c4ba280-450a-11e2-bcfd-0800200c9a66"});
+db.runCommand({getLastError:1, j:1});
+
+node.insert({"owner":NumberLong("1234567890"),"parent":NumberLong("0"),_id:NumberLong("1234567890"), "counts":0});
+db.runCommand({getLastError:1, j:1});
+for (var i=0; i<1000; i++) {
+   trans.insert({"owner":NumberLong(i),"tid":"2c4ba280-450a-11e2-bcfd-0800200c9a66"});
+   node.insert({"owner":NumberLong(i),"parent":NumberLong(i+1000),_id:NumberLong(i+1234567890), "counts":0});
+}
+
+transB.insert({"owner":NumberLong("1234567890"),"tid":"2c4ba280-450a-11e2-bcfd-0800200c9a66"});
+var r1=dbB.runCommand( { getLastError: 1, w: 1 } );
+assert( r1.n == 0 && r1.err.length > 0 && r1.hasOwnProperty("code"), tojson( r1 ) );
+
+jsTest.log("Inserted dup (failed), now split chunks and move data");
+
+adminSD.runCommand( { split: ns1, middle : { owner : 100} });
+adminSD.runCommand( { movechunk: ns1, find : { owner : 105}, to: other});
+
+jsTest.log("Kicking off dup inserts and updates");
+
+errors=[];
+i=0;
+trans.insert({"owner":NumberLong("1234567890"),"tid":"2c4ba280-450a-11e2-bcfd-0800200c9a66"});
+var r1=db.runCommand( { getLastError: 1, w: 1 } );
+assert( r1.n == 0 && r1.err.length > 0 && r1.hasOwnProperty("code"), tojson( r1 ) );
+transB.insert({"owner":NumberLong("1234567890"),"tid":"2c4ba280-450a-11e2-bcfd-0800200c9a66"});
+var rB1=dbB.runCommand( { getLastError: 1, w: 1 } );
+assert( rB1.n == 0 && rB1.err.length > 0 && rB1.hasOwnProperty("code"), tojson( r1 ) );
+
+nodeB.update({"owner":NumberLong("1234567890"),"parent":NumberLong("0"),_id:NumberLong("1234567890")},{"$inc":{"counts":1}});
+var resultB = dbB.runCommand( { getLastError: 1, w: 1 } )
+node.update({"owner":NumberLong("1234567890"),"parent":NumberLong("0"),_id:NumberLong("1234567890")},{"$inc":{"counts":1}});
+var result = db.runCommand( { getLastError: 1, w: 1 } )
+
+assert.eq( 2, node.findOne().counts );
+
+printjson( result )
+printjson( resultB )
+
+assert( result.n==1 && result.updatedExisting==true && result.err == null, "update succeeded on collection node on mongos A but GLE was\nn=" + result.n + ",\nupdatedExisting=" + result.updatedExisting + ",\nerr=" + result.err);
+assert( resultB.n==1 && resultB.updatedExisting==true && resultB.err == null, "update succeeded on collection node on mongos B but GLE was\nn=" + resultB.n + ",\nupdatedExisting=" + resultB.updatedExisting + ",\nerr=" + resultB.err);
+
+s.stop();
diff --git a/src/mongo/db/lasterror.cpp b/src/mongo/db/lasterror.cpp
index 4f232f5436b..a3741cbcb7b 100644
--- a/src/mongo/db/lasterror.cpp
+++ b/src/mongo/db/lasterror.cpp
@@ -48,6 +48,7 @@ namespace mongo {
     void LastError::appendSelfStatus( BSONObjBuilder &b ) {
         if ( writebackId.isSet() ) {
             b.append( "writeback" , writebackId );
+            b.append( "writebackSince", writebackSince );
             b.append( "instanceIdent" , prettyHostName() ); // this can be any unique string
         }
     }
@@ -55,7 +56,7 @@ namespace mongo {
     bool LastError::appendSelf( BSONObjBuilder &b , bool blankErr ) {
 
         appendSelfStatus( b );
-        
+
         if ( !valid ) {
             if ( blankErr )
                 b.appendNull( "err" );
diff --git a/src/mongo/db/lasterror.h b/src/mongo/db/lasterror.h
index ea60ab629b2..02340471e1b 100644
--- a/src/mongo/db/lasterror.h
+++ b/src/mongo/db/lasterror.h
@@ -29,6 +29,7 @@ namespace mongo {
         enum UpdatedExistingType { NotUpdate, True, False } updatedExisting;
         OID upsertedId;
         OID writebackId; // this shouldn't get reset so that old GLE are handled
+        int writebackSince;
         long long nObjects;
         int nPrev;
         bool valid;
@@ -36,6 +37,7 @@ namespace mongo {
         void writeback( OID& oid ) {
             reset( true );
             writebackId = oid;
+            writebackSince = 0;
         }
         void raiseError(int _code , const char *_msg) {
             reset( true );
@@ -56,6 +58,7 @@ namespace mongo {
         }
         LastError() {
             reset();
+            writebackSince = 0;
         }
         void reset( bool _valid = false ) {
             code = 0;
@@ -63,6 +66,7 @@ namespace mongo {
             updatedExisting = NotUpdate;
             nObjects = 0;
             nPrev = 1;
+            writebackSince++;
             valid = _valid;
             disabled = false;
             upsertedId.clear();
diff --git a/src/mongo/s/client_info.cpp b/src/mongo/s/client_info.cpp
index 2b3994ff822..d304e5eb25d 100644
--- a/src/mongo/s/client_info.cpp
+++ b/src/mongo/s/client_info.cpp
@@ -92,7 +92,7 @@ namespace mongo {
         _lastAccess = 0;
     }
 
-    void ClientInfo::_addWriteBack( vector<WBInfo>& all , const BSONObj& gle ) {
+    void ClientInfo::_addWriteBack( vector<WBInfo>& all, const BSONObj& gle, bool fromLastOperation ) {
         BSONElement w = gle["writeback"];
 
         if ( w.type() != jstOID )
@@ -109,10 +109,12 @@ namespace mongo {
         if ( gle["instanceIdent"].type() == String )
             ident = gle["instanceIdent"].String();
 
-        all.push_back( WBInfo( WriteBackListener::ConnectionIdent( ident , cid.numberLong() ) , w.OID() ) );
+        all.push_back( WBInfo( WriteBackListener::ConnectionIdent( ident , cid.numberLong() ),
+                               w.OID(),
+                               fromLastOperation ) );
     }
 
-    vector<BSONObj> ClientInfo::_handleWriteBacks( vector<WBInfo>& all , bool fromWriteBackListener ) {
+    vector<BSONObj> ClientInfo::_handleWriteBacks( const vector<WBInfo>& all , bool fromWriteBackListener ) {
         vector<BSONObj> res;
 
         if ( all.size() == 0 )
@@ -190,7 +192,7 @@ namespace mongo {
                 conn.done();
             }
 
-            _addWriteBack( writebacks , res );
+            _addWriteBack( writebacks, res, true );
 
             LOG(4) << "gathering writebacks from " << sinceLastGetError().size() << " hosts for"
                    << " gle (" << theShard << ")" << endl;
@@ -206,7 +208,7 @@ namespace mongo {
                 try {
                     ShardConnection conn( temp , "" );
                     ON_BLOCK_EXIT_OBJ( conn, &ShardConnection::done );
-                    _addWriteBack( writebacks , conn->getLastErrorDetailed() );
+                    _addWriteBack( writebacks, conn->getLastErrorDetailed(), false );
 
                 }
                 catch( std::exception &e ){
@@ -232,10 +234,25 @@ namespace mongo {
                     // since that's the current write
                     // but we block for all
                     verify( v.size() >= 1 );
-                    result.appendElements( v[0] );
-                    result.appendElementsUnique( res );
-                    result.append( "writebackGLE" , v[0] );
-                    result.append( "initialGLEHost" , theShard );
+
+                    if ( res["writebackSince"].numberInt() > 0 ) {
+                        // got writeback from older op
+                        // ignore the result from it, just needed to wait
+                        result.appendElements( res );
+                    }
+                    else if ( writebacks[0].fromLastOperation ) {
+                        result.appendElements( v[0] );
+                        result.appendElementsUnique( res );
+                        result.append( "writebackGLE" , v[0] );
+                        result.append( "initialGLEHost" , theShard );
+                        result.append( "initialGLE", res );
+                    }
+                    else {
+                        // there was a writeback
+                        // but its from an old operations
+                        // so all that's important is that we block, not that we return stats
+                        result.appendElements( res );
+                    }
                 }
             }
             else {
@@ -288,7 +305,7 @@ namespace mongo {
                 return false;
             }
 
-            _addWriteBack( writebacks, res );
+            _addWriteBack( writebacks, res, true );
 
             string temp = DBClientWithCommands::getLastErrorString( res );
             if ( (*conn)->type() != ConnectionString::SYNC && ( ok == false || temp.size() ) ) {
@@ -327,7 +344,7 @@ namespace mongo {
 
             ShardConnection conn( temp , "" );
             try {
-                _addWriteBack( writebacks, conn->getLastErrorDetailed() );
+                _addWriteBack( writebacks, conn->getLastErrorDetailed(), false );
             }
             catch( std::exception &e ){
                 warning() << "could not clear last error from a shard " << temp << causedBy( e ) << endl;
diff --git a/src/mongo/s/client_info.h b/src/mongo/s/client_info.h
index bb036e1e698..504ac0e3a68 100644
--- a/src/mongo/s/client_info.h
+++ b/src/mongo/s/client_info.h
@@ -107,14 +107,16 @@ namespace mongo {
     private:
         AuthenticationInfo _ai;
         struct WBInfo {
-            WBInfo( const WriteBackListener::ConnectionIdent& c , OID o ) : ident( c ) , id( o ) {}
+            WBInfo( const WriteBackListener::ConnectionIdent& c, OID o, bool fromLastOperation )
+                : ident( c ), id( o ), fromLastOperation( fromLastOperation ) {}
             WriteBackListener::ConnectionIdent ident;
             OID id;
+            bool fromLastOperation;
         };
 
         // for getLastError
-        void _addWriteBack( vector<WBInfo>& all , const BSONObj& o );
-        vector<BSONObj> _handleWriteBacks( vector<WBInfo>& all , bool fromWriteBackListener );
+        void _addWriteBack( vector<WBInfo>& all , const BSONObj& o, bool fromLastOperation );
+        vector<BSONObj> _handleWriteBacks( const vector<WBInfo>& all , bool fromWriteBackListener );
 
 
         int _id; // unique client id