SERVER-16520 ClientCursor keeps ptr to owning CollectionCursorCache

Changes ClientCursor::_collection to ClientCursor::_cursorCache, and
updates ClientCursor/ClientCursorPin interfaces accordingly.

10 files changed, 48 insertions, 40 deletions

diff --git a/src/mongo/db/catalog/collection_cursor_cache.h b/src/mongo/db/catalog/collection_cursor_cache.h
index 9b0de262ceb..e031c4abee1 100644
--- a/src/mongo/db/catalog/collection_cursor_cache.h
+++ b/src/mongo/db/catalog/collection_cursor_cache.h
@@ -54,6 +54,8 @@ namespace mongo {
 
         // -----------------
 
+        std::string ns() { return _nss.ns(); }
+
         /**
          * @param collectionGoingAway Pass as true if the Collection instance is going away.
          *                            This could be because the db is being closed, or the
diff --git a/src/mongo/db/clientcursor.cpp b/src/mongo/db/clientcursor.cpp
index dde63e71169..1a2e35992d5 100644
--- a/src/mongo/db/clientcursor.cpp
+++ b/src/mongo/db/clientcursor.cpp
@@ -69,12 +69,12 @@ namespace mongo {
         return cursorStatsOpen.get();
     }
 
-    ClientCursor::ClientCursor(const Collection* collection,
+    ClientCursor::ClientCursor(CollectionCursorCache* cursorCache,
                                PlanExecutor* exec,
                                int qopts,
                                const BSONObj query,
                                bool isAggCursor)
-        : _collection(collection),
+        : _cursorCache(cursorCache),
           _countedYet(false),
           _isAggCursor(isAggCursor),
           _unownedRU(NULL) {
@@ -84,14 +84,14 @@ namespace mongo {
         _query = query;
         _queryOptions = qopts;
         if (exec->collection()) {
-            invariant(collection == exec->collection());
+            invariant(cursorCache == exec->collection()->cursorCache());
         }
         init();
     }
 
-    ClientCursor::ClientCursor(const Collection* collection)
-        : _ns(collection->ns().ns()),
-          _collection(collection),
+    ClientCursor::ClientCursor(CollectionCursorCache* cursorCache)
+        : _ns(cursorCache->ns()),
+          _cursorCache(cursorCache),
           _countedYet(false),
           _queryOptions(QueryOption_NoCursorTimeout),
           _isAggCursor(false),
@@ -100,7 +100,7 @@ namespace mongo {
     }
 
     void ClientCursor::init() {
-        invariant( _collection );
+        invariant( _cursorCache );
 
         _isPinned = false;
         _isNoTimeout = false;
@@ -116,7 +116,7 @@ namespace mongo {
             cursorStatsOpenNoTimeout.increment();
         }
 
-        _cursorid = _collection->cursorCache()->registerCursor( this );
+        _cursorid = _cursorCache->registerCursor( this );
 
         cursorStatsOpen.increment();
         _countedYet = true;
@@ -138,13 +138,13 @@ namespace mongo {
                 cursorStatsOpenNoTimeout.decrement();
         }
 
-        if ( _collection ) {
+        if ( _cursorCache ) {
             // this could be null if kill() was killed
-            _collection->cursorCache()->deregisterCursor( this );
+            _cursorCache->deregisterCursor( this );
         }
 
         // defensive:
-        _collection = NULL;
+        _cursorCache = NULL;
         _cursorid = INVALID_CURSOR_ID;
         _pos = -2;
         _isNoTimeout = false;
@@ -154,7 +154,7 @@ namespace mongo {
         if ( _exec.get() )
             _exec->kill();
 
-        _collection = NULL;
+        _cursorCache = NULL;
     }
 
     //
@@ -218,10 +218,10 @@ namespace mongo {
     // deleted from underneath us, so we can save the pointer and ignore the ID.
     //
 
-    ClientCursorPin::ClientCursorPin( const Collection* collection, long long cursorid )
+    ClientCursorPin::ClientCursorPin( CollectionCursorCache* cursorCache, long long cursorid )
         : _cursor( NULL ) {
         cursorStatsOpenPinned.increment();
-        _cursor = collection->cursorCache()->find( cursorid, true );
+        _cursor = cursorCache->find( cursorid, true );
     }
 
     ClientCursorPin::~ClientCursorPin() {
@@ -235,14 +235,14 @@ namespace mongo {
 
         invariant( _cursor->isPinned() );
 
-        if ( _cursor->collection() == NULL ) {
+        if ( _cursor->cursorCache() == NULL ) {
             // The ClientCursor was killed while we had it.  Therefore, it is our responsibility to
             // kill it.
             deleteUnderlying();
         }
         else {
             // Unpin the cursor under the collection cursor cache lock.
-            _cursor->collection()->cursorCache()->unpin( _cursor );
+            _cursor->cursorCache()->unpin( _cursor );
         }
     }
 
@@ -255,8 +255,8 @@ namespace mongo {
         //   error to unpin a registered cursor without holding the cursor cache lock (note that we
         //   can't simply unpin with the cursor cache lock here, since we need to guarantee
         //   exclusive ownership of the cursor when we are deleting it).
-        if ( _cursor->collection() ) {
-            _cursor->collection()->cursorCache()->deregisterCursor( _cursor );
+        if ( _cursor->cursorCache() ) {
+            _cursor->cursorCache()->deregisterCursor( _cursor );
             _cursor->kill();
         }
         _cursor->unsetPinned();
diff --git a/src/mongo/db/clientcursor.h b/src/mongo/db/clientcursor.h
index 1c8e694e063..356c33f2bc0 100644
--- a/src/mongo/db/clientcursor.h
+++ b/src/mongo/db/clientcursor.h
@@ -43,6 +43,7 @@ namespace mongo {
     typedef boost::recursive_mutex::scoped_lock recursive_scoped_lock;
     class ClientCursor;
     class Collection;
+    class CollectionCursorCache;
     class CurOp;
     class Database;
     class NamespaceDetails;
@@ -61,7 +62,7 @@ namespace mongo {
         /**
          * This ClientCursor constructor creates a cursorid that can be getMore'd
          */
-        ClientCursor(const Collection* collection,
+        ClientCursor(CollectionCursorCache* cursorCache,
                      PlanExecutor* exec,
                      int qopts = 0,
                      const BSONObj query = BSONObj(),
@@ -70,7 +71,7 @@ namespace mongo {
         /**
          * This ClientCursor is used to track sharding state.
          */
-        ClientCursor(const Collection* collection);
+        ClientCursor(CollectionCursorCache* cursorCache);
 
         //
         // Basic accessors
@@ -78,7 +79,7 @@ namespace mongo {
 
         CursorId cursorid() const { return _cursorid; }
         std::string ns() const { return _ns; }
-        const Collection* collection() const { return _collection; }
+        CollectionCursorCache* cursorCache() const { return _cursorCache; }
         bool isAggCursor() const { return _isAggCursor; }
 
         //
@@ -220,7 +221,7 @@ namespace mongo {
         // The namespace we're operating on.
         std::string _ns;
 
-        const Collection* _collection;
+        CollectionCursorCache* _cursorCache;
 
         // if we've added it to the total open counter yet
         bool _countedYet;
@@ -288,7 +289,7 @@ namespace mongo {
     */
     class ClientCursorPin : boost::noncopyable {
     public:
-        ClientCursorPin( const Collection* collection, long long cursorid );
+        ClientCursorPin( CollectionCursorCache* cursorCache, long long cursorid );
         ~ClientCursorPin();
         // This just releases the pin, does not delete the underlying
         // unless ownership has passed to us after kill
diff --git a/src/mongo/db/commands/parallel_collection_scan.cpp b/src/mongo/db/commands/parallel_collection_scan.cpp
index fed47ad30d2..e36ad731d83 100644
--- a/src/mongo/db/commands/parallel_collection_scan.cpp
+++ b/src/mongo/db/commands/parallel_collection_scan.cpp
@@ -137,7 +137,8 @@ namespace mongo {
                 for (size_t i = 0; i < execs.size(); i++) {
                     // transfer ownership of an executor to the ClientCursor (which manages its own
                     // lifetime).
-                    ClientCursor* cc = new ClientCursor( collection, execs.releaseAt(i) );
+                    ClientCursor* cc = new ClientCursor( collection->cursorCache(),
+                                                         execs.releaseAt(i) );
 
                     // we are mimicking the aggregation cursor output here
                     // that is why there are ns, ok and empty firstBatch
diff --git a/src/mongo/db/commands/pipeline_command.cpp b/src/mongo/db/commands/pipeline_command.cpp
index ae6ab10004a..18daf7facbd 100644
--- a/src/mongo/db/commands/pipeline_command.cpp
+++ b/src/mongo/db/commands/pipeline_command.cpp
@@ -275,9 +275,10 @@ namespace mongo {
                 if (collection) {
                     // XXX
                     const bool isAggCursor = true; // enable special locking behavior
-                    ClientCursor* cursor = new ClientCursor(collection, execHolder.release(), 0,
-                                                            BSONObj(), isAggCursor);
-                    pin.reset(new ClientCursorPin(collection, cursor->cursorid()));
+                    ClientCursor* cursor = new ClientCursor(collection->cursorCache(),
+                                                            execHolder.release(), 0, BSONObj(),
+                                                            isAggCursor);
+                    pin.reset(new ClientCursorPin(collection->cursorCache(), cursor->cursorid()));
                     // Don't add any code between here and the start of the try block.
                 }
             }
diff --git a/src/mongo/db/commands/repair_cursor.cpp b/src/mongo/db/commands/repair_cursor.cpp
index a2247aab680..c968b03c1be 100644
--- a/src/mongo/db/commands/repair_cursor.cpp
+++ b/src/mongo/db/commands/repair_cursor.cpp
@@ -102,7 +102,7 @@ namespace mongo {
 
             // ClientCursors' constructor inserts them into a global map that manages their
             // lifetimes. That is why the next line isn't leaky.
-            ClientCursor* cc = new ClientCursor(collection, exec.release());
+            ClientCursor* cc = new ClientCursor(collection->cursorCache(), exec.release());
 
             BSONObjBuilder cursorObj(result.subobjStart("cursor"));
             cursorObj.append("id", cc->cursorid());
diff --git a/src/mongo/db/query/find.cpp b/src/mongo/db/query/find.cpp
index c1929c57f7d..24f77315a27 100644
--- a/src/mongo/db/query/find.cpp
+++ b/src/mongo/db/query/find.cpp
@@ -211,7 +211,7 @@ namespace mongo {
         // A pin performs a CC lookup and if there is a CC, increments the CC's pin value so it
         // doesn't time out.  Also informs ClientCursor that there is somebody actively holding the
         // CC, so don't delete it.
-        ClientCursorPin ccPin(collection, cursorid);
+        ClientCursorPin ccPin(collection->cursorCache(), cursorid);
         ClientCursor* cc = ccPin.c();
 
         // If we're not being called from DBDirectClient we want to associate the RecoveryUnit
@@ -824,7 +824,7 @@ namespace mongo {
 
             // Allocate a new ClientCursor.  We don't have to worry about leaking it as it's
             // inserted into a global map by its ctor.
-            ClientCursor* cc = new ClientCursor(collection, exec.get(),
+            ClientCursor* cc = new ClientCursor(collection->cursorCache(), exec.get(),
                                                 pq.getOptions().toInt(),
                                                 pq.getFilter());
             ccId = cc->cursorid();
diff --git a/src/mongo/db/range_preserver.h b/src/mongo/db/range_preserver.h
index 9dc40844a9c..626d541f76d 100644
--- a/src/mongo/db/range_preserver.h
+++ b/src/mongo/db/range_preserver.h
@@ -52,10 +52,10 @@ namespace mongo {
             // Empty collections don't have any data we need to preserve
             if (collection) {
                 // Not a memory leak.  Cached in a static structure by CC's ctor.
-                ClientCursor* cc = new ClientCursor(collection);
+                ClientCursor* cc = new ClientCursor(collection->cursorCache());
 
                 // Pin keeps the CC from being deleted while it's in scope.  We delete it ourselves.
-                _pin.reset(new ClientCursorPin(collection, cc->cursorid()));
+                _pin.reset(new ClientCursorPin(collection->cursorCache(), cc->cursorid()));
             }
         }
 
diff --git a/src/mongo/dbtests/query_plan_executor.cpp b/src/mongo/dbtests/query_plan_executor.cpp
index 79f63a4a842..ca3ac770f78 100644
--- a/src/mongo/dbtests/query_plan_executor.cpp
+++ b/src/mongo/dbtests/query_plan_executor.cpp
@@ -415,7 +415,7 @@ namespace QueryPlanExecutor {
                 PlanExecutor* exec = makeCollScanExec(coll,filterObj);
 
                 // Make a client cursor from the runner.
-                new ClientCursor(coll, exec, 0, BSONObj());
+                new ClientCursor(coll->cursorCache(), exec, 0, BSONObj());
 
                 // There should be one cursor before invalidation,
                 // and zero cursors after invalidation.
@@ -441,8 +441,8 @@ namespace QueryPlanExecutor {
                 PlanExecutor* exec = makeCollScanExec(collection, filterObj);
 
                 // Make a client cursor from the runner.
-                ClientCursor* cc = new ClientCursor(collection, exec, 0, BSONObj());
-                ClientCursorPin ccPin(collection, cc->cursorid());
+                ClientCursor* cc = new ClientCursor(collection->cursorCache(), exec, 0, BSONObj());
+                ClientCursorPin ccPin(collection->cursorCache(), cc->cursorid());
 
                 // If the cursor is pinned, it sticks around,
                 // even after invalidation.
@@ -481,7 +481,7 @@ namespace QueryPlanExecutor {
                     PlanExecutor* exec = makeCollScanExec(collection, filterObj);
 
                     // Make a client cursor from the runner.
-                    new ClientCursor(collection, exec, 0, BSONObj());
+                    new ClientCursor(collection->cursorCache(), exec, 0, BSONObj());
                 }
 
                 // There should be one cursor before timeout,
diff --git a/src/mongo/dbtests/querytests.cpp b/src/mongo/dbtests/querytests.cpp
index dcaf796374d..84647587496 100644
--- a/src/mongo/dbtests/querytests.cpp
+++ b/src/mongo/dbtests/querytests.cpp
@@ -265,7 +265,7 @@ namespace QueryTests {
             {
                 // Check internal server handoff to getmore.
                 Client::WriteContext ctx(&_txn,  ns);
-                ClientCursorPin clientCursor( ctx.getCollection(), cursorId );
+                ClientCursorPin clientCursor( ctx.getCollection()->cursorCache(), cursorId );
                 // pq doesn't exist if it's a runner inside of the clientcursor.
                 // ASSERT( clientCursor.c()->pq );
                 // ASSERT_EQUALS( 2, clientCursor.c()->pq->getNumToReturn() );
@@ -665,7 +665,8 @@ namespace QueryTests {
             ASSERT_EQUALS( two, c->next()["ts"].Date() );
             long long cursorId = c->getCursorId();
             
-            ClientCursorPin clientCursor( ctx.db()->getCollection( &_txn, ns ), cursorId );
+            ClientCursorPin clientCursor( ctx.db()->getCollection( &_txn, ns )->cursorCache(),
+                                          cursorId );
             ASSERT_EQUALS( three.millis, clientCursor.c()->getSlaveReadTill().asDate() );
         }
     };
@@ -1515,7 +1516,7 @@ namespace QueryTests {
             ClientCursor *clientCursor = 0;
             {
                 AutoGetCollectionForRead ctx(&_txn, ns());
-                ClientCursorPin clientCursorPointer(ctx.getCollection(), cursorId);
+                ClientCursorPin clientCursorPointer(ctx.getCollection()->cursorCache(), cursorId);
                 clientCursor = clientCursorPointer.c();
                 // clientCursorPointer destructor unpins the cursor.
             }
@@ -1552,7 +1553,9 @@ namespace QueryTests {
             
             {
                 Client::WriteContext ctx(&_txn,  ns() );
-                ClientCursorPin pinCursor( ctx.ctx().db()->getCollection( &_txn, ns() ), cursorId );
+                ClientCursorPin pinCursor( ctx.ctx().db()->getCollection( &_txn,
+                                                                          ns())->cursorCache(),
+                                                                          cursorId );
                 string expectedAssertion =
                         str::stream() << "Cannot kill active cursor " << cursorId; 
                 ASSERT_THROWS_WHAT(CollectionCursorCache::eraseCursorGlobal(&_txn, cursorId),