diff options
-rw-r--r-- | src/mongo/db/cursor_manager.cpp | 63 | ||||
-rw-r--r-- | src/mongo/db/cursor_manager.h | 22 | ||||
-rw-r--r-- | src/mongo/dbtests/cursor_manager_test.cpp | 39 |
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. */ |