SERVER-37452 Remove concept of globally managed cursor IDs.

3 files changed, 26 insertions, 98 deletions

diff --git a/src/mongo/db/cursor_manager.cpp b/src/mongo/db/cursor_manager.cpp
index 6659347026f..d8ed3a9e380 100644
--- a/src/mongo/db/cursor_manager.cpp
+++ b/src/mongo/db/cursor_manager.cpp
@@ -65,19 +65,6 @@ using std::vector;
 constexpr int CursorManager::kNumPartitions;
 
 namespace {
-uint32_t idFromCursorId(CursorId id) {
-    uint64_t x = static_cast<uint64_t>(id);
-    x = x >> 32;
-    return static_cast<uint32_t>(x);
-}
-
-CursorId cursorIdFromParts(uint32_t collectionIdentifier, uint32_t cursor) {
-    // The leading two bits of a non-global CursorId should be 0.
-    invariant((collectionIdentifier & (0b11 << 30)) == 0);
-    CursorId x = static_cast<CursorId>(collectionIdentifier) << 32;
-    x |= cursor;
-    return x;
-}
 
 class GlobalCursorIdCache {
 public:
@@ -484,35 +471,38 @@ size_t CursorManager::numCursors() const {
 
 CursorId CursorManager::allocateCursorId_inlock() {
     for (int i = 0; i < 10000; i++) {
-        // The leading two bits of a CursorId are used to determine if the cursor is registered on
-        // the global cursor manager.
-        CursorId id;
-        if (isGlobalManager()) {
-            // This is the global cursor manager, so generate a random number and make sure the
-            // first two bits are 01.
-            uint64_t mask = 0x3FFFFFFFFFFFFFFF;
-            uint64_t bitToSet = 1ULL << 62;
-            id = ((_random->nextInt64() & mask) | bitToSet);
-        } else {
-            // The first 2 bits are 0, the next 30 bits are the collection identifier, the next 32
-            // bits are random.
-            uint32_t myPart = static_cast<uint32_t>(_random->nextInt32());
-            id = cursorIdFromParts(_collectionCacheRuntimeId, myPart);
+        CursorId id = _random->nextInt64();
+
+        // A cursor id of zero is reserved to indicate that the cursor has been closed. If the
+        // random number generator gives us zero, then try again.
+        if (id == 0) {
+            continue;
         }
+
+        // Avoid negative cursor ids by taking the absolute value. If the cursor id is the minimum
+        // representable negative number, then just generate another random id.
+        if (id == std::numeric_limits<CursorId>::min()) {
+            continue;
+        }
+        id = std::abs(id);
+
         auto partition = _cursorMap->lockOnePartition(id);
-        if (partition->count(id) == 0)
+        if (partition->count(id) == 0) {
+            // The cursor id is not already in use, so return it. Even though we drop the lock on
+            // the '_cursorMap' partition, another thread cannot register a cursor with the same id
+            // because we still hold '_registrationLock'.
             return id;
+        }
+
+        // The cursor id is already in use. Generate another random id.
     }
+
+    // We failed to generate a unique cursor id.
     fassertFailed(17360);
 }
 
 ClientCursorPin CursorManager::registerCursor(OperationContext* opCtx,
                                               ClientCursorParams&& cursorParams) {
-    // TODO SERVER-37455: Cursors should only ever be registered against the global cursor manager.
-    // Follow-up work is required to actually delete the concept of a per-collection cursor manager
-    // from the code base.
-    invariant(isGlobalManager());
-
     // Avoid computing the current time within the critical section.
     auto now = opCtx->getServiceContext()->getPreciseClockSource()->now();
 
@@ -564,8 +554,7 @@ Status CursorManager::killCursor(OperationContext* opCtx, CursorId id, bool shou
     auto it = lockedPartition->find(id);
     if (it == lockedPartition->end()) {
         if (shouldAudit) {
-            audit::logKillCursorsAuthzCheck(
-                opCtx->getClient(), _nss, id, ErrorCodes::CursorNotFound);
+            audit::logKillCursorsAuthzCheck(opCtx->getClient(), {}, id, ErrorCodes::CursorNotFound);
         }
         return {ErrorCodes::CursorNotFound, str::stream() << "Cursor id not found: " << id};
     }
@@ -582,14 +571,14 @@ Status CursorManager::killCursor(OperationContext* opCtx, CursorId id, bool shou
         }
 
         if (shouldAudit) {
-            audit::logKillCursorsAuthzCheck(opCtx->getClient(), _nss, id, ErrorCodes::OK);
+            audit::logKillCursorsAuthzCheck(opCtx->getClient(), cursor->nss(), id, ErrorCodes::OK);
         }
         return Status::OK();
     }
     std::unique_ptr<ClientCursor, ClientCursor::Deleter> ownedCursor(cursor);
 
     if (shouldAudit) {
-        audit::logKillCursorsAuthzCheck(opCtx->getClient(), _nss, id, ErrorCodes::OK);
+        audit::logKillCursorsAuthzCheck(opCtx->getClient(), cursor->nss(), id, ErrorCodes::OK);
     }
 
     deregisterAndDestroyCursor(std::move(lockedPartition), opCtx, std::move(ownedCursor));
diff --git a/src/mongo/db/cursor_manager.h b/src/mongo/db/cursor_manager.h
index 7a4dadba465..d9435da334d 100644
--- a/src/mongo/db/cursor_manager.h
+++ b/src/mongo/db/cursor_manager.h
@@ -98,18 +98,6 @@ public:
     static std::pair<Status, int> killCursorsWithMatchingSessions(
         OperationContext* opCtx, const SessionKiller::Matcher& matcher);
 
-    /**
-     * This method is deprecated. Do not add new call sites.
-     *
-     * TODO SERVER-37452: Delete this method.
-     */
-    static bool isGloballyManagedCursor(CursorId cursorId) {
-        // The first two bits are 01 for globally managed cursors, and 00 for cursors owned by a
-        // collection. The leading bit is always 0 so that CursorIds do not appear as negative.
-        const long long mask = static_cast<long long>(0b11) << 62;
-        return (cursorId & mask) == (static_cast<long long>(0b01) << 62);
-    }
-
     static int killCursorGlobalIfAuthorized(OperationContext* opCtx, int n, const char* ids);
 
     static bool killCursorGlobalIfAuthorized(OperationContext* opCtx, CursorId id);
@@ -235,16 +223,6 @@ private:
 
     bool cursorShouldTimeout_inlock(const ClientCursor* cursor, Date_t now);
 
-    bool isGlobalManager() const {
-        return _nss.isEmpty();
-    }
-
-    // No locks are needed to consult these data members.
-    //
-    // TODO SERVER-37452: Delete these data members.
-    const NamespaceString _nss;
-    const uint32_t _collectionCacheRuntimeId = 0;
-
     // A CursorManager holds a pointer to all open ClientCursors. ClientCursors are owned by the
     // CursorManager, except when they are in use by a ClientCursorPin. When in use by a pin, an
     // unowned pointer remains to ensure they still receive kill notifications while in use.
diff --git a/src/mongo/dbtests/cursor_manager_test.cpp b/src/mongo/dbtests/cursor_manager_test.cpp
index 8e0e5edac41..866881009e0 100644
--- a/src/mongo/dbtests/cursor_manager_test.cpp
+++ b/src/mongo/dbtests/cursor_manager_test.cpp
@@ -56,31 +56,6 @@ namespace mongo {
 namespace {
 const NamespaceString kTestNss{"test.collection"};
 
-TEST(CursorManagerTest, IsGloballyManagedCursorShouldReturnFalseIfLeadingBitsAreZeroes) {
-    ASSERT_FALSE(CursorManager::isGloballyManagedCursor(0x0000000000000000));
-    ASSERT_FALSE(CursorManager::isGloballyManagedCursor(0x000000000FFFFFFF));
-    ASSERT_FALSE(CursorManager::isGloballyManagedCursor(0x000000007FFFFFFF));
-    ASSERT_FALSE(CursorManager::isGloballyManagedCursor(0x0FFFFFFFFFFFFFFF));
-    ASSERT_FALSE(CursorManager::isGloballyManagedCursor(0x3FFFFFFFFFFFFFFF));
-    ASSERT_FALSE(CursorManager::isGloballyManagedCursor(0x3dedbeefdeadbeef));
-}
-
-TEST(CursorManagerTest, IsGloballyManagedCursorShouldReturnTrueIfLeadingBitsAreZeroAndOne) {
-    ASSERT_TRUE(CursorManager::isGloballyManagedCursor(0x4FFFFFFFFFFFFFFF));
-    ASSERT_TRUE(CursorManager::isGloballyManagedCursor(0x5FFFFFFFFFFFFFFF));
-    ASSERT_TRUE(CursorManager::isGloballyManagedCursor(0x6FFFFFFFFFFFFFFF));
-    ASSERT_TRUE(CursorManager::isGloballyManagedCursor(0x7FFFFFFFFFFFFFFF));
-    ASSERT_TRUE(CursorManager::isGloballyManagedCursor(0x4000000000000000));
-    ASSERT_TRUE(CursorManager::isGloballyManagedCursor(0x4dedbeefdeadbeef));
-}
-
-TEST(CursorManagerTest, IsGloballyManagedCursorShouldReturnFalseIfLeadingBitIsAOne) {
-    ASSERT_FALSE(CursorManager::isGloballyManagedCursor(~0LL));
-    ASSERT_FALSE(CursorManager::isGloballyManagedCursor(0xFFFFFFFFFFFFFFFF));
-    ASSERT_FALSE(CursorManager::isGloballyManagedCursor(0x8FFFFFFFFFFFFFFF));
-    ASSERT_FALSE(CursorManager::isGloballyManagedCursor(0x8dedbeefdeadbeef));
-}
-
 class CursorManagerTest : public unittest::Test {
 public:
     CursorManagerTest()
@@ -146,20 +121,6 @@ class CursorManagerTestCustomOpCtx : public CursorManagerTest {
     }
 };
 
-TEST_F(CursorManagerTest, GlobalCursorManagerShouldReportOwnershipOfCursorsItCreated) {
-    for (int i = 0; i < 1000; i++) {
-        auto cursorPin = CursorManager::getGlobalCursorManager()->registerCursor(
-            _opCtx.get(),
-            {makeFakePlanExecutor(),
-             NamespaceString{"test.collection"},
-             {},
-             repl::ReadConcernArgs(repl::ReadConcernLevel::kLocalReadConcern),
-             BSONObj(),
-             ClientCursorParams::LockPolicy::kLocksInternally});
-        ASSERT_TRUE(CursorManager::isGloballyManagedCursor(cursorPin.getCursor()->cursorid()));
-    }
-}
-
 /**
  * Test that an attempt to kill a pinned cursor succeeds.
  */