From 4774b56bf203db930dfd6493e9cb721070ae88bb Mon Sep 17 00:00:00 2001 From: Eliot Horowitz Date: Wed, 21 Oct 2009 15:18:21 -0400 Subject: [PATCH] refactoring updateObjects return to be cleaner and so can work with multi-object update SERVER-268 --- db/db.cpp | 4 ++-- db/dbhelpers.cpp | 2 +- db/instance.cpp | 4 ++-- db/query.h | 26 +++++++++++++++++++++++++- db/repl.cpp | 12 ++++++------ db/update.cpp | 41 ++++++++++++++--------------------------- 6 files changed, 50 insertions(+), 39 deletions(-) diff --git a/db/db.cpp b/db/db.cpp index 6a5e07efca2..a7541776f07 100644 --- a/db/db.cpp +++ b/db/db.cpp @@ -96,8 +96,8 @@ namespace mongo { deleteObjects("sys.unittest.delete", j1, false); theDataFileMgr.insert("sys.unittest.delete", &js1, sizeof(js1)); deleteObjects("sys.unittest.delete", j1, false); - updateObjects("sys.unittest.delete", j1, j1, true,ss); - updateObjects("sys.unittest.delete", j1, j1, false,ss); + updateObjects("sys.unittest.delete", j1, j1, true,false,ss,true); + updateObjects("sys.unittest.delete", j1, j1, false,false,ss,true); auto_ptr c = theDataFileMgr.findAll("sys.unittest.pdfile"); while ( c->ok() ) { diff --git a/db/dbhelpers.cpp b/db/dbhelpers.cpp index 76a71dc8509..2ea17498736 100644 --- a/db/dbhelpers.cpp +++ b/db/dbhelpers.cpp @@ -119,7 +119,7 @@ namespace mongo { void Helpers::putSingleton(const char *ns, BSONObj obj) { DBContext context(ns); stringstream ss; - updateObjects(ns, obj, /*pattern=*/BSONObj(), /*upsert=*/true, ss); + updateObjects(ns, obj, /*pattern=*/BSONObj(), /*upsert=*/true, /*multi=*/false, ss, /*logop=*/true ); } void Helpers::emptyCollection(const char *ns) { diff --git a/db/instance.cpp b/db/instance.cpp index 8b039cb2d7e..df9e8243b58 100644 --- a/db/instance.cpp +++ b/db/instance.cpp @@ -358,8 +358,8 @@ namespace mongo { CurOp& currentOp = *client.curop(); strncpy(currentOp.query, s.c_str(), sizeof(currentOp.query)-2); } - bool updatedExisting = updateObjects(ns, toupdate, query, upsert, ss, multi); - recordUpdate( updatedExisting, ( upsert || updatedExisting ) ? 1 : 0 ); // for getlasterror + UpdateResult res = updateObjects(ns, toupdate, query, upsert, multi, ss, true); + recordUpdate( res.existing , res.num ); // for getlasterror } void receivedDelete(Message& m, stringstream &ss) { diff --git a/db/query.h b/db/query.h index 3a4f975ba51..16be7c44f55 100644 --- a/db/query.h +++ b/db/query.h @@ -74,10 +74,34 @@ namespace mongo { // for an existing query (ie a ClientCursor), send back additional information. QueryResult* getMore(const char *ns, int ntoreturn, long long cursorid , stringstream& ss); + struct UpdateResult { + bool existing; + bool mod; + unsigned long long num; + + UpdateResult( bool e, bool m, unsigned long long n ) + : existing(e) , mod(m), num(n ){} + + int oldCode(){ + if ( ! num ) + return 0; + + if ( existing ){ + if ( mod ) + return 2; + return 1; + } + + if ( mod ) + return 3; + return 4; + } + }; + /* returns true if an existing object was updated, false if no existing object was found. multi - update multiple objects - mostly useful with things like $set */ - bool updateObjects(const char *ns, BSONObj updateobj, BSONObj pattern, bool upsert, stringstream& ss, bool multi = false); + UpdateResult updateObjects(const char *ns, BSONObj updateobj, BSONObj pattern, bool upsert, bool multi , stringstream& ss, bool logop ); // If justOne is true, deletedId is set to the id of the deleted object. int deleteObjects(const char *ns, BSONObj pattern, bool justOne, bool logop = false, bool god=false); diff --git a/db/repl.cpp b/db/repl.cpp index 92de5f02a50..574087be00e 100644 --- a/db/repl.cpp +++ b/db/repl.cpp @@ -49,7 +49,6 @@ namespace mongo { extern boost::recursive_mutex &dbMutex; - int _updateObjects(const char *ns, BSONObj updateobj, BSONObj pattern, bool upsert, stringstream& ss, bool logOp=false); void ensureHaveIdIndex(const char *ns); /* if 1 sync() is running */ @@ -545,8 +544,9 @@ namespace mongo { stringstream ss; setClient("local.sources"); - int u = _updateObjects("local.sources", o, pattern, true/*upsert for pair feature*/, ss); - assert( u == 1 || u == 4 ); + UpdateResult res = updateObjects("local.sources", o, pattern, true/*upsert for pair feature*/, false,ss,false); + assert( ! res.mod ); + assert( res.num == 1 ); cc().clearns(); if ( replacing ) { @@ -780,7 +780,7 @@ namespace mongo { if( !o.getObjectID(_id) ) { /* No _id. This will be very slow. */ Timer t; - _updateObjects(ns, o, o, true, ss); + updateObjects(ns, o, o, true, false, ss, false ); if( t.millis() >= 2 ) { RARELY OCCASIONALLY log() << "warning, repl doing slow updates (no _id field) for " << ns << endl; } @@ -792,13 +792,13 @@ namespace mongo { /* erh 10/16/2009 - this is probably not relevant any more since its auto-created, but not worth removing */ RARELY ensureHaveIdIndex(ns); // otherwise updates will be slow - _updateObjects(ns, o, b.done(), true, ss); + updateObjects(ns, o, b.done(), true, false, ss, false ); } } } else if ( *opType == 'u' ) { RARELY ensureHaveIdIndex(ns); // otherwise updates will be super slow - _updateObjects(ns, o, op.getObjectField("o2"), op.getBoolField("b"), ss); + updateObjects(ns, o, op.getObjectField("o2"), op.getBoolField("b"), false, ss, false ); } else if ( *opType == 'd' ) { if ( opType[1] == 0 ) diff --git a/db/update.cpp b/db/update.cpp index ae43eb92026..9e0ded33637 100644 --- a/db/update.cpp +++ b/db/update.cpp @@ -648,8 +648,11 @@ namespace mongo { long long nscanned_; auto_ptr< KeyValJSMatcher > matcher_; }; + - int __updateObjects(const char *ns, BSONObj updateobj, BSONObj &pattern, bool upsert, stringstream& ss, bool logop=false) { + UpdateResult updateObjects(const char *ns, BSONObj updateobj, BSONObj pattern, bool upsert, bool multi, stringstream& ss, bool logop ) { + uassert("multi not coded yet", !multi); + int profile = cc().database()->profile; uassert("cannot update reserved $ collection", strchr(ns, '$') == 0 ); @@ -711,18 +714,20 @@ namespace mongo { mods.appendSizeSpecForArrayDepMods( patternBuilder ); pattern = patternBuilder.obj(); } - logOp("u", ns, updateobj, &pattern ); - return 5; } + logOp("u", ns, updateobj, &pattern ); } - return 2; - } else { + return UpdateResult( 1 , 1 , 1 ); + } + else { BSONElementManipulator::lookForTimestamps( updateobj ); checkNoMods( updateobj ); } theDataFileMgr.update(ns, r, c->currLoc(), updateobj.objdata(), updateobj.objsize(), ss); - return 1; + if ( logop ) + logOp("u", ns, updateobj, &pattern ); + return UpdateResult( 1 , 0 , 1 ); } if ( profile ) @@ -746,7 +751,7 @@ namespace mongo { ss << " fastmodinsert "; if ( logop ) logOp( "i", ns, newObj ); - return 3; + return UpdateResult( 0 , 1 , 1 ); } checkNoMods( updateobj ); if ( profile ) @@ -754,27 +759,9 @@ namespace mongo { theDataFileMgr.insert(ns, updateobj); if ( logop ) logOp( "i", ns, updateobj ); - return 4; + return UpdateResult( 0 , 0 , 1 ); } - return 0; + return UpdateResult( 0 , 0 , 0 ); } - /* todo: - _ smart requery find record immediately - (clean return codes up later...) - */ - int _updateObjects(const char *ns, BSONObj updateobj, BSONObj pattern, bool upsert, stringstream& ss, bool logop=false) { - return __updateObjects( ns, updateobj, pattern, upsert, ss, logop ); - } - - /* multi means multiple updates. this is not implemented yet, but stubbing out for future work */ - /* todo - clean up these crazy __updateobjects return codes! */ - bool updateObjects(const char *ns, BSONObj updateobj, BSONObj pattern, bool upsert, stringstream& ss, bool multi) { - uassert("multi not coded yet", !multi); - int rc = __updateObjects(ns, updateobj, pattern, upsert, ss, true); - /* todo: why is there a logOp here when __updateObjects also does a bunch of logOps? */ - if ( rc != 5 && rc != 0 && rc != 4 && rc != 3 ) - logOp("u", ns, updateobj, &pattern, &upsert); - return ( rc == 1 || rc == 2 || rc == 5 ); - } }