SERVER-4328 use a mutex of its own for OpTime::now. we stay in the mutex for a while after grabbing an optime so that we dont have to reason too much about different threads grabbing optimes and then using them out of order.

7 files changed, 66 insertions, 65 deletions

diff --git a/src/mongo/db/dbcommands.cpp b/src/mongo/db/dbcommands.cpp
index 49241865972..3344814524e 100644
--- a/src/mongo/db/dbcommands.cpp
+++ b/src/mongo/db/dbcommands.cpp
@@ -639,8 +639,8 @@ namespace mongo {
         virtual LockType locktype() const { return NONE; }
         CmdGetOpTime() : Command("getoptime") { }
         bool run(const string& dbname, BSONObj& cmdObj, int, string& errmsg, BSONObjBuilder& result, bool fromRepl) {
-            writelock l( "" );
-            result.appendDate("optime", OpTime::now().asDate());
+            mutex::scoped_lock lk(OpTime::m);
+            result.appendDate("optime", OpTime::now(lk).asDate());
             return true;
         }
     } cmdgetoptime;
diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp
index bed9de21e2b..231956be1b6 100644
--- a/src/mongo/db/instance.cpp
+++ b/src/mongo/db/instance.cpp
@@ -17,6 +17,7 @@
 */
 
 #include "pch.h"
+#include "../util/time_support.h"
 #include "db.h"
 #include "../bson/util/atomic_int.h"
 #include "introspect.h"
@@ -77,21 +78,40 @@ namespace mongo {
     // see FSyncCommand:
     extern bool lockedForWriting();
 
-    OpTime OpTime::now() {
-        DEV d.dbMutex.assertWriteLocked();
-        return now_inlock();
+    /*static*/ OpTime OpTime::_now() {
+        OpTime result;
+        unsigned t = (unsigned) time(0);
+        if ( last.secs == t ) {
+            last.i++;
+            result = last;
+        }
+        else if ( t < last.secs ) {
+            result = skewed(); // separate function to keep out of the hot code path
+        }
+        else { 
+            last = OpTime(t, 1);
+            result = last;
+        }
+        notifier.notify_all();
+        return last;
     }
-    OpTime OpTime::last_inlock(){
-        DEV d.dbMutex.assertAtLeastReadLocked();
+    OpTime OpTime::now(const mongo::mutex::scoped_lock&) {
+        return _now();
+    }
+    OpTime OpTime::getLast(const mongo::mutex::scoped_lock&) {
         return last;
     }
+    boost::condition OpTime::notifier;
+    mongo::mutex OpTime::m("optime");
 
-    // OpTime::now() uses dbMutex, thus it is in this file not in the cpp files used by drivers and such
+    // OpTime::now() uses mutex, thus it is in this file not in the cpp files used by drivers and such
     void BSONElementManipulator::initTimestamp() {
         massert( 10332 ,  "Expected CurrentTime type", _element.type() == Timestamp );
         unsigned long long &timestamp = *( reinterpret_cast< unsigned long long* >( value() ) );
-        if ( timestamp == 0 )
-            timestamp = OpTime::now().asDate();
+        if ( timestamp == 0 ) {
+            mutex::scoped_lock lk(OpTime::m);
+            timestamp = OpTime::now(lk).asDate();
+        }
     }
     void BSONElementManipulator::SetNumber(double d) {
         if ( _element.type() == NumberDouble )
@@ -578,19 +598,10 @@ namespace mongo {
 
         if (*this != last) return; // check early
 
-        boost::xtime timeout;
-        boost::xtime_get(&timeout, boost::TIME_UTC);
-
-        timeout.nsec += millis * 1000*1000;
-        if (timeout.nsec >= 1000*1000*1000){
-            timeout.nsec -= 1000*1000*1000;
-            timeout.sec += 1;
-        }
-
         do {
             dbtemprelease tmp;
-            boost::mutex::scoped_lock lk(notifyMutex());
-            if (!notifier().timed_wait(lk, timeout))
+            mutex::scoped_lock lk(m);
+            if (!notifier.timed_wait(lk.boost(), boost::posix_time::milliseconds(millis)))
                 return; // timed out
         } while (*this != last);
     }
@@ -618,10 +629,13 @@ namespace mongo {
             try {
                 Client::ReadContext ctx(ns);
                 if (str::startsWith(ns, "local.oplog.")){
-                    if (pass == 0)
-                        last = OpTime::last_inlock();
-                    else
+                    if (pass == 0) {
+                        mutex::scoped_lock lk(OpTime::m);
+                        last = OpTime::getLast(lk);
+                    }
+                    else {
                         last.waitForDifferent(1000/*ms*/);
+                    }
                 }
 
                 // call this readlocked so state can't change
diff --git a/src/mongo/db/jsobjmanipulator.h b/src/mongo/db/jsobjmanipulator.h
index 860e575940e..880fde8b409 100644
--- a/src/mongo/db/jsobjmanipulator.h
+++ b/src/mongo/db/jsobjmanipulator.h
@@ -78,6 +78,7 @@ namespace mongo {
                 if ( e.eoo() )
                     break;
                 if ( e.type() == Timestamp ) {
+                    // performance note, this locks a mutex:
                     BSONElementManipulator( e ).initTimestamp();
                     break;
                 }
diff --git a/src/mongo/db/oplog.cpp b/src/mongo/db/oplog.cpp
index 540b328933d..b3eeb06cf75 100644
--- a/src/mongo/db/oplog.cpp
+++ b/src/mongo/db/oplog.cpp
@@ -125,7 +125,7 @@ namespace mongo {
     // on every logop call.
     static BufBuilder logopbufbuilder(8*1024);
     static void _logOpRS(const char *opstr, const char *ns, const char *logNS, const BSONObj& obj, BSONObj *o2, bool *bb, bool fromMigrate ) {
-        DEV assertInWriteLock();
+        Lock::DBWrite lk1("local");
 
         if ( strncmp(ns, "local.", 6) == 0 ) {
             if ( strncmp(ns, "local.slaves", 12) == 0 )
@@ -133,7 +133,9 @@ namespace mongo {
             return;
         }
 
-        const OpTime ts = OpTime::now();
+        mutex::scoped_lock lk2(OpTime::m);
+
+        const OpTime ts = OpTime::now(lk2);
         long long hashNew;
         if( theReplSet ) {
             massert(13312, "replSet error : logOp() but not primary?", theReplSet->box.getState().primary());
@@ -222,7 +224,7 @@ namespace mongo {
     */
     static void _logOpOld(const char *opstr, const char *ns, const char *logNS, const BSONObj& obj, BSONObj *o2, bool *bb, bool fromMigrate ) {
         Lock::DBWrite lk("local");
-        static BufBuilder bufbuilder(8*1024);
+        static BufBuilder bufbuilder(8*1024); // todo there is likely a mutex on this constructor
 
         if ( strncmp(ns, "local.", 6) == 0 ) {
             if ( strncmp(ns, "local.slaves", 12) == 0 ) {
@@ -231,7 +233,9 @@ namespace mongo {
             return;
         }
 
-        const OpTime ts = OpTime::now();
+        mutex::scoped_lock lk2(OpTime::m);
+
+        const OpTime ts = OpTime::now(lk2);
         Client::Context context("",0,false);
 
         /* we jump through a bunch of hoops here to avoid copying the obj buffer twice --
@@ -565,7 +569,7 @@ namespace mongo {
         void run() {
             OpTime t;
             for ( int i = 0; i < 10; i++ ) {
-                OpTime s = OpTime::now_inlock();
+                OpTime s = OpTime::_now();
                 assert( s != t );
                 t = s;
             }
diff --git a/src/mongo/dbtests/jsobjtests.cpp b/src/mongo/dbtests/jsobjtests.cpp
index 586a11745ab..47171b7c993 100644
--- a/src/mongo/dbtests/jsobjtests.cpp
+++ b/src/mongo/dbtests/jsobjtests.cpp
@@ -314,9 +314,9 @@ namespace JsobjTests {
                 e = i.next();
                 ASSERT( e.eoo() );
 
-                OpTime before = OpTime::now();
+                OpTime before = OpTime::_now();
                 BSONElementManipulator( o.firstElement() ).initTimestamp();
-                OpTime after = OpTime::now();
+                OpTime after = OpTime::_now();
 
                 OpTime test = OpTime( o.firstElement().date() );
                 ASSERT( before < test && test < after );
diff --git a/src/mongo/dbtests/querytests.cpp b/src/mongo/dbtests/querytests.cpp
index c54d3ff0a12..06a9f6afeae 100644
--- a/src/mongo/dbtests/querytests.cpp
+++ b/src/mongo/dbtests/querytests.cpp
@@ -442,9 +442,9 @@ namespace QueryTests {
                                 BSON( "create" << "querytests.OplogReplaySlaveReadTill" <<
                                      "capped" << true << "size" << 8192 ),
                                 info );
-            Date_t one = OpTime::now().asDate();
-            Date_t two = OpTime::now().asDate();
-            Date_t three = OpTime::now().asDate();
+            Date_t one = OpTime::_now().asDate();
+            Date_t two = OpTime::_now().asDate();
+            Date_t three = OpTime::_now().asDate();
             insert( ns, BSON( "ts" << one ) );
             insert( ns, BSON( "ts" << two ) );
             insert( ns, BSON( "ts" << three ) );
@@ -1243,11 +1243,12 @@ namespace QueryTests {
     class CollectionInternalBase : public CollectionBase {
     public:
         CollectionInternalBase( const char *nsLeaf ) :
-        CollectionBase( nsLeaf ),
-        _ctx( ns() ) {
+          CollectionBase( nsLeaf ),
+          _lk( ns() ),
+          _ctx( ns() ) {
         }
     private:
-        //dblock _lk;
+        Lock::DBWrite _lk;
         Client::Context _ctx;
     };
     
diff --git a/src/mongo/util/optime.h b/src/mongo/util/optime.h
index 031ad960d20..ee98d7d2081 100644
--- a/src/mongo/util/optime.h
+++ b/src/mongo/util/optime.h
@@ -42,8 +42,7 @@ namespace mongo {
         static OpTime skewed();
     public:
         static void setLast(const Date_t &date) {
-            notifier().notify_all(); // won't really do anything until write-lock released
-
+            notifier.notify_all(); // won't really do anything until write-lock released
             last = OpTime(date);
         }
         unsigned getSecs() const {
@@ -75,22 +74,13 @@ namespace mongo {
             i = 0;
         }
         // it isn't generally safe to not be locked for this. so use now(). some tests use this.
-        static OpTime now_inlock() {
-            notifier().notify_all(); // won't really do anything until write-lock released
-
-            unsigned t = (unsigned) time(0);
-            if ( last.secs == t ) {
-                last.i++;
-                return last;
-            }
-            if ( t < last.secs ) {
-                return skewed(); // separate function to keep out of the hot code path
-            }
-            last = OpTime(t, 1);
-            return last;
-        }
-        static OpTime now();
-        static OpTime last_inlock();
+        static OpTime _now();
+
+        static mongo::mutex m;
+
+        static OpTime now(const mongo::mutex::scoped_lock&);
+
+        static OpTime getLast(const mongo::mutex::scoped_lock&);
 
         // Waits for global OpTime to be different from *this
         // Must be atLeastReadLocked
@@ -154,16 +144,7 @@ namespace mongo {
             return !(*this < r);
         }
     private:
-
-        // The following functions are to get around the need to define class-level statics in a cpp
-        static boost::condition& notifier() {
-            static boost::condition* holder = new boost::condition();
-            return *holder;
-        };
-        static boost::mutex& notifyMutex() {
-            static boost::mutex* holder = new boost::mutex();
-            return *holder;
-        };
+        static boost::condition notifier;
     };
 #pragma pack()