SERVER-29202 Remove unused fetch methods from logical session cache

11 files changed, 34 insertions, 167 deletions

diff --git a/src/mongo/db/logical_session_cache.cpp b/src/mongo/db/logical_session_cache.cpp
index d76c8040325..c026c2281dd 100644
--- a/src/mongo/db/logical_session_cache.cpp
+++ b/src/mongo/db/logical_session_cache.cpp
@@ -103,33 +103,6 @@ LogicalSessionCache::~LogicalSessionCache() {
     }
 }
 
-// TODO: fetch should attempt to update user info, if it is not in the found record.
-
-Status LogicalSessionCache::fetchAndPromote(OperationContext* opCtx, const LogicalSessionId& lsid) {
-    // Search our local cache first
-    auto promoteRes = promote(lsid);
-    if (promoteRes.isOK()) {
-        return promoteRes;
-    }
-
-    // Cache miss, must fetch from the sessions collection.
-    auto res = _sessionsColl->fetchRecord(opCtx, lsid);
-
-    // If we got a valid record, add it to our cache.
-    if (res.isOK()) {
-        auto& record = res.getValue();
-        record.setLastUse(now());
-
-        // Any duplicate records here are actually the same record with different
-        // lastUse times, ignore them.
-        auto oldRecord = _addToCache(record);
-        return Status::OK();
-    }
-
-    // If we could not get a valid record, return the error.
-    return res.getStatus();
-}
-
 Status LogicalSessionCache::promote(LogicalSessionId lsid) {
     stdx::unique_lock<stdx::mutex> lk(_cacheMutex);
     auto it = _cache.find(lsid);
diff --git a/src/mongo/db/logical_session_cache.h b/src/mongo/db/logical_session_cache.h
index 0c903482a47..16fdfb78fb0 100644
--- a/src/mongo/db/logical_session_cache.h
+++ b/src/mongo/db/logical_session_cache.h
@@ -119,15 +119,6 @@ public:
     Status promote(LogicalSessionId lsid);
 
     /**
-     * If the cache contains a record for this LogicalSessionId, promotes it.
-     * Otherwise, attempts to fetch the record for this LogicalSessionId from the
-     * sessions collection, and returns the record if found. Otherwise, returns an error.
-     *
-     * This method may issue networking calls.
-     */
-    Status fetchAndPromote(OperationContext* opCtx, const LogicalSessionId& lsid);
-
-    /**
      * Inserts a new authoritative session record into the cache. This method will
      * insert the authoritative record into the sessions collection. This method
      * should only be used when starting new sessions and should not be used to
diff --git a/src/mongo/db/logical_session_cache_test.cpp b/src/mongo/db/logical_session_cache_test.cpp
index 5fd4ad17bee..83b0278bb41 100644
--- a/src/mongo/db/logical_session_cache_test.cpp
+++ b/src/mongo/db/logical_session_cache_test.cpp
@@ -127,30 +127,6 @@ private:
     Client* _client;
 };
 
-// Test that session cache fetches new records from the sessions collection
-TEST_F(LogicalSessionCacheTest, CacheFetchesNewRecords) {
-    auto lsid = makeLogicalSessionIdForTest();
-
-    // When the record is not present (and not in the sessions collection) returns an error
-    auto res = cache()->fetchAndPromote(opCtx(), lsid);
-    ASSERT(!res.isOK());
-
-    // When the record is not present (but is in the sessions collection) returns it
-    sessions()->add(makeLogicalSessionRecord(lsid, service()->now()));
-    res = cache()->fetchAndPromote(opCtx(), lsid);
-    ASSERT(res.isOK());
-
-    // When the record is present in the cache, returns it
-    sessions()->setFetchHook([](LogicalSessionId id) -> StatusWith<LogicalSessionRecord> {
-        // We should not be querying the sessions collection on the next call
-        ASSERT(false);
-        return {ErrorCodes::NoSuchSession, "no such session"};
-    });
-
-    res = cache()->fetchAndPromote(opCtx(), lsid);
-    ASSERT(res.isOK());
-}
-
 // Test that the getFromCache method does not make calls to the sessions collection
 TEST_F(LogicalSessionCacheTest, TestCacheHitsOnly) {
     auto lsid = makeLogicalSessionIdForTest();
@@ -163,15 +139,10 @@ TEST_F(LogicalSessionCacheTest, TestCacheHitsOnly) {
     sessions()->add(makeLogicalSessionRecord(lsid, service()->now()));
     res = cache()->promote(lsid);
     ASSERT(!res.isOK());
-
-    // When the record is present, returns the owner
-    cache()->fetchAndPromote(opCtx(), lsid).transitional_ignore();
-    res = cache()->promote(lsid);
-    ASSERT(res.isOK());
 }
 
-// Test that fetching from the cache updates the lastUse date of records
-TEST_F(LogicalSessionCacheTest, FetchUpdatesLastUse) {
+// Test that promoting from the cache updates the lastUse date of records
+TEST_F(LogicalSessionCacheTest, PromoteUpdatesLastUse) {
     auto lsid = makeLogicalSessionIdForTest();
 
     auto start = service()->now();
@@ -179,23 +150,23 @@ TEST_F(LogicalSessionCacheTest, FetchUpdatesLastUse) {
     // Insert the record into the sessions collection with 'start'
     ASSERT(cache()->startSession(opCtx(), makeLogicalSessionRecord(lsid, start)).isOK());
 
-    // Fast forward time and fetch
+    // Fast forward time and promote
     service()->fastForward(Milliseconds(500));
     ASSERT(start != service()->now());
-    auto res = cache()->fetchAndPromote(opCtx(), lsid);
+    auto res = cache()->promote(lsid);
     ASSERT(res.isOK());
 
-    // Now that we fetched, lifetime of session should be extended
+    // Now that we promoted, lifetime of session should be extended
     service()->fastForward(kSessionTimeout - Milliseconds(500));
-    res = cache()->fetchAndPromote(opCtx(), lsid);
+    res = cache()->promote(lsid);
     ASSERT(res.isOK());
 
-    // We fetched again, so lifetime extended again
+    // We promoted again, so lifetime extended again
     service()->fastForward(kSessionTimeout - Milliseconds(10));
-    res = cache()->fetchAndPromote(opCtx(), lsid);
+    res = cache()->promote(lsid);
     ASSERT(res.isOK());
 
-    // Fast forward and hit-only fetch
+    // Fast forward and promote
     service()->fastForward(kSessionTimeout - Milliseconds(10));
     res = cache()->promote(lsid);
     ASSERT(res.isOK());
@@ -280,7 +251,7 @@ TEST_F(LogicalSessionCacheTest, CacheRefreshesOwnRecords) {
 
     // Use one of the records
     setOpCtx();
-    auto res = cache()->fetchAndPromote(opCtx(), record1.getId());
+    auto res = cache()->promote(record1.getId());
     ASSERT(res.isOK());
 
     // Advance time so that one record expires
diff --git a/src/mongo/db/sessions_collection.h b/src/mongo/db/sessions_collection.h
index 79a9b23535f..553a74224a4 100644
--- a/src/mongo/db/sessions_collection.h
+++ b/src/mongo/db/sessions_collection.h
@@ -54,13 +54,6 @@ public:
     static constexpr StringData kSessionsFullNS = "admin.system.sessions"_sd;
 
     /**
-     * Returns a LogicalSessionRecord for the given session id. This method
-     * may run networking operations on the calling thread.
-     */
-    virtual StatusWith<LogicalSessionRecord> fetchRecord(OperationContext* opCtx,
-                                                         const LogicalSessionId& id) = 0;
-
-    /**
      * Updates the last-use times on the given sessions to be greater than
      * or equal to the given time. Returns an error if a networking issue occurred.
      */
diff --git a/src/mongo/db/sessions_collection_mock.cpp b/src/mongo/db/sessions_collection_mock.cpp
index 09deaaa3b1f..b70f8e015bf 100644
--- a/src/mongo/db/sessions_collection_mock.cpp
+++ b/src/mongo/db/sessions_collection_mock.cpp
@@ -34,16 +34,11 @@ namespace mongo {
 
 MockSessionsCollectionImpl::MockSessionsCollectionImpl()
     : _sessions(),
-      _fetch(stdx::bind(&MockSessionsCollectionImpl::_fetchRecord, this, stdx::placeholders::_1)),
       _refresh(
           stdx::bind(&MockSessionsCollectionImpl::_refreshSessions, this, stdx::placeholders::_1)),
       _remove(
           stdx::bind(&MockSessionsCollectionImpl::_removeRecords, this, stdx::placeholders::_1)) {}
 
-void MockSessionsCollectionImpl::setFetchHook(FetchHook hook) {
-    _fetch = std::move(hook);
-}
-
 void MockSessionsCollectionImpl::setRefreshHook(RefreshHook hook) {
     _refresh = std::move(hook);
 }
@@ -53,17 +48,11 @@ void MockSessionsCollectionImpl::setRemoveHook(RemoveHook hook) {
 }
 
 void MockSessionsCollectionImpl::clearHooks() {
-    _fetch = stdx::bind(&MockSessionsCollectionImpl::_fetchRecord, this, stdx::placeholders::_1);
     _refresh =
         stdx::bind(&MockSessionsCollectionImpl::_refreshSessions, this, stdx::placeholders::_1);
     _remove = stdx::bind(&MockSessionsCollectionImpl::_removeRecords, this, stdx::placeholders::_1);
 }
 
-StatusWith<LogicalSessionRecord> MockSessionsCollectionImpl::fetchRecord(
-    const LogicalSessionId& id) {
-    return _fetch(id);
-}
-
 Status MockSessionsCollectionImpl::refreshSessions(const LogicalSessionRecordSet& sessions) {
     return _refresh(sessions);
 }
@@ -96,19 +85,6 @@ const MockSessionsCollectionImpl::SessionMap& MockSessionsCollectionImpl::sessio
     return _sessions;
 }
 
-StatusWith<LogicalSessionRecord> MockSessionsCollectionImpl::_fetchRecord(
-    const LogicalSessionId& id) {
-    stdx::unique_lock<stdx::mutex> lk(_mutex);
-
-    // If we do not have this record, return an error
-    auto it = _sessions.find(id);
-    if (it == _sessions.end()) {
-        return {ErrorCodes::NoSuchSession, "No matching record in the sessions collection"};
-    }
-
-    return it->second;
-}
-
 Status MockSessionsCollectionImpl::_refreshSessions(const LogicalSessionRecordSet& sessions) {
     for (auto& record : sessions) {
         if (!has(record.getId())) {
diff --git a/src/mongo/db/sessions_collection_mock.h b/src/mongo/db/sessions_collection_mock.h
index 5456e0731a2..cf0a88c324c 100644
--- a/src/mongo/db/sessions_collection_mock.h
+++ b/src/mongo/db/sessions_collection_mock.h
@@ -58,12 +58,10 @@ public:
 
     MockSessionsCollectionImpl();
 
-    using FetchHook = stdx::function<StatusWith<LogicalSessionRecord>(const LogicalSessionId&)>;
     using RefreshHook = stdx::function<Status(const LogicalSessionRecordSet&)>;
     using RemoveHook = stdx::function<Status(const LogicalSessionIdSet&)>;
 
     // Set custom hooks to override default behavior
-    void setFetchHook(FetchHook hook);
     void setRefreshHook(RefreshHook hook);
     void setRemoveHook(RemoveHook hook);
 
@@ -71,7 +69,6 @@ public:
     void clearHooks();
 
     // Forwarding methods from the MockSessionsCollection
-    StatusWith<LogicalSessionRecord> fetchRecord(const LogicalSessionId& id);
     Status refreshSessions(const LogicalSessionRecordSet& sessions);
     Status removeRecords(const LogicalSessionIdSet& sessions);
 
@@ -84,14 +81,12 @@ public:
 
 private:
     // Default implementations, may be overridden with custom hooks.
-    StatusWith<LogicalSessionRecord> _fetchRecord(const LogicalSessionId& id);
     Status _refreshSessions(const LogicalSessionRecordSet& sessions);
     Status _removeRecords(const LogicalSessionIdSet& sessions);
 
     stdx::mutex _mutex;
     SessionMap _sessions;
 
-    FetchHook _fetch;
     RefreshHook _refresh;
     RemoveHook _remove;
 };
@@ -106,11 +101,6 @@ public:
     explicit MockSessionsCollection(std::shared_ptr<MockSessionsCollectionImpl> impl)
         : _impl(std::move(impl)) {}
 
-    StatusWith<LogicalSessionRecord> fetchRecord(OperationContext* opCtx,
-                                                 const LogicalSessionId& id) override {
-        return _impl->fetchRecord(id);
-    }
-
     Status refreshSessions(OperationContext* opCtx,
                            const LogicalSessionRecordSet& sessions,
                            Date_t refreshTime) override {
diff --git a/src/mongo/db/sessions_collection_rs.cpp b/src/mongo/db/sessions_collection_rs.cpp
index 58dc3bd0ad0..f153d31f385 100644
--- a/src/mongo/db/sessions_collection_rs.cpp
+++ b/src/mongo/db/sessions_collection_rs.cpp
@@ -98,23 +98,6 @@ Status runIfStandaloneOrPrimary(OperationContext* opCtx, Callback callback) {
 
 }  // namespace
 
-StatusWith<LogicalSessionRecord> SessionsCollectionRS::fetchRecord(OperationContext* opCtx,
-                                                                   const LogicalSessionId& lsid) {
-
-    DBDirectClient client(opCtx);
-    auto cursor = client.query(kSessionsFullNS.toString(), lsidQuery(lsid), 1);
-    if (!cursor->more()) {
-        return {ErrorCodes::NoSuchSession, "No matching record in the sessions collection"};
-    }
-
-    try {
-        IDLParserErrorContext ctx("LogicalSessionRecord");
-        return LogicalSessionRecord::parse(ctx, cursor->next());
-    } catch (...) {
-        return exceptionToStatus();
-    }
-}
-
 Status SessionsCollectionRS::refreshSessions(OperationContext* opCtx,
                                              const LogicalSessionRecordSet& sessions,
                                              Date_t refreshTime) {
diff --git a/src/mongo/db/sessions_collection_rs.h b/src/mongo/db/sessions_collection_rs.h
index 7c08ae2121c..799f4564291 100644
--- a/src/mongo/db/sessions_collection_rs.h
+++ b/src/mongo/db/sessions_collection_rs.h
@@ -54,13 +54,6 @@ public:
     SessionsCollectionRS() = default;
 
     /**
-     * Returns a LogicalSessionRecord for the given session id, or an error if
-     * no such record was found.
-     */
-    StatusWith<LogicalSessionRecord> fetchRecord(OperationContext* opCtx,
-                                                 const LogicalSessionId& lsid) override;
-
-    /**
      * Updates the last-use times on the given sessions to be greater than
      * or equal to the current time.
      */
diff --git a/src/mongo/db/sessions_collection_standalone.cpp b/src/mongo/db/sessions_collection_standalone.cpp
index bbff1db1757..79f36e66fd6 100644
--- a/src/mongo/db/sessions_collection_standalone.cpp
+++ b/src/mongo/db/sessions_collection_standalone.cpp
@@ -44,22 +44,6 @@ BSONObj lsidQuery(const LogicalSessionId& lsid) {
 }
 }  // namespace
 
-StatusWith<LogicalSessionRecord> SessionsCollectionStandalone::fetchRecord(
-    OperationContext* opCtx, const LogicalSessionId& lsid) {
-    DBDirectClient client(opCtx);
-    auto cursor = client.query(kSessionsFullNS.toString(), lsidQuery(lsid), 1);
-    if (!cursor->more()) {
-        return {ErrorCodes::NoSuchSession, "No matching record in the sessions collection"};
-    }
-
-    try {
-        IDLParserErrorContext ctx("LogicalSessionRecord");
-        return LogicalSessionRecord::parse(ctx, cursor->next());
-    } catch (...) {
-        return exceptionToStatus();
-    }
-}
-
 Status SessionsCollectionStandalone::refreshSessions(OperationContext* opCtx,
                                                      const LogicalSessionRecordSet& sessions,
                                                      Date_t refreshTime) {
diff --git a/src/mongo/db/sessions_collection_standalone.h b/src/mongo/db/sessions_collection_standalone.h
index 949d7ec2e0a..a164117fe44 100644
--- a/src/mongo/db/sessions_collection_standalone.h
+++ b/src/mongo/db/sessions_collection_standalone.h
@@ -43,13 +43,6 @@ class OperationContext;
 class SessionsCollectionStandalone : public SessionsCollection {
 public:
     /**
-     * Returns a LogicalSessionRecord for the given session id, or an error if
-     * no such record was found.
-     */
-    StatusWith<LogicalSessionRecord> fetchRecord(OperationContext* opCtx,
-                                                 const LogicalSessionId& lsid) override;
-
-    /**
      * Updates the last-use times on the given sessions to be greater than
      * or equal to the current time.
      */
diff --git a/src/mongo/dbtests/logical_sessions_tests.cpp b/src/mongo/dbtests/logical_sessions_tests.cpp
index 93696fce152..9c8d773b6a3 100644
--- a/src/mongo/dbtests/logical_sessions_tests.cpp
+++ b/src/mongo/dbtests/logical_sessions_tests.cpp
@@ -65,6 +65,26 @@ Status insertRecord(OperationContext* opCtx, LogicalSessionRecord record) {
     return {ErrorCodes::DuplicateSession, errorString};
 }
 
+BSONObj lsidQuery(const LogicalSessionId& lsid) {
+    return BSON(LogicalSessionRecord::kIdFieldName << lsid.toBSON());
+}
+
+StatusWith<LogicalSessionRecord> fetchRecord(OperationContext* opCtx,
+                                             const LogicalSessionId& lsid) {
+    DBDirectClient client(opCtx);
+    auto cursor = client.query(kTestNS.toString(), lsidQuery(lsid), 1);
+    if (!cursor->more()) {
+        return {ErrorCodes::NoSuchSession, "No matching record in the sessions collection"};
+    }
+
+    try {
+        IDLParserErrorContext ctx("LogicalSessionRecord");
+        return LogicalSessionRecord::parse(ctx, cursor->next());
+    } catch (...) {
+        return exceptionToStatus();
+    }
+}
+
 }  // namespace
 
 class SessionsCollectionStandaloneTest {
@@ -115,10 +135,10 @@ public:
         res = collection()->removeRecords(opCtx(), {record1.getId()});
         ASSERT_OK(res);
 
-        auto swRecord = collection()->fetchRecord(opCtx(), record1.getId());
+        auto swRecord = fetchRecord(opCtx(), record1.getId());
         ASSERT(!swRecord.isOK());
 
-        swRecord = collection()->fetchRecord(opCtx(), record2.getId());
+        swRecord = fetchRecord(opCtx(), record2.getId());
         ASSERT(swRecord.isOK());
     }
 };
@@ -144,7 +164,7 @@ public:
         ASSERT(resRefresh.isOK());
 
         // The timestamp on the refreshed record should be updated.
-        auto swRecord = collection()->fetchRecord(opCtx(), record1.getId());
+        auto swRecord = fetchRecord(opCtx(), record1.getId());
         ASSERT(swRecord.isOK());
         ASSERT_EQ(swRecord.getValue().getLastUse(), now);
 
@@ -156,7 +176,7 @@ public:
         resRefresh = collection()->refreshSessions(opCtx(), {record2}, now);
         ASSERT(resRefresh.isOK());
 
-        swRecord = collection()->fetchRecord(opCtx(), record2.getId());
+        swRecord = fetchRecord(opCtx(), record2.getId());
         ASSERT(swRecord.isOK());
 
         // Clear the collection.