Revert "SERVER-31114 Perform targeted session invalidation on direct writes to `config.transactions`"

This reverts commit c8cb9cc374af47f862d81e52ad4bc33d96239ef0.

8 files changed, 23 insertions, 138 deletions

diff --git a/jstests/replsets/retryable_writes_direct_write_to_config_transactions.js b/jstests/replsets/retryable_writes_direct_write_to_config_transactions.js
deleted file mode 100644
index 7929f90354a..00000000000
--- a/jstests/replsets/retryable_writes_direct_write_to_config_transactions.js
+++ /dev/null
@@ -1,81 +0,0 @@
-// Validates the expected behaviour of direct writes against the `config.transactions` collection
-(function() {
-    'use strict';
-
-    var replTest = new ReplSetTest({nodes: 2});
-    replTest.startSet();
-    replTest.initiate();
-
-    var priConn = replTest.getPrimary();
-    var db = priConn.getDB('TestDB');
-    var config = priConn.getDB('config');
-
-    assert.writeOK(db.user.insert({_id: 0}));
-    assert.writeOK(db.user.insert({_id: 1}));
-
-    const lsid1 = UUID();
-    const lsid2 = UUID();
-
-    const cmdObj1 = {
-        update: 'user',
-        updates: [{q: {_id: 0}, u: {$inc: {x: 1}}}],
-        lsid: {id: lsid1},
-        txnNumber: NumberLong(1)
-    };
-    assert.commandWorked(db.runCommand(cmdObj1));
-    assert.eq(1, db.user.find({_id: 0}).toArray()[0].x);
-
-    const cmdObj2 = {
-        update: 'user',
-        updates: [{q: {_id: 1}, u: {$inc: {x: 1}}}],
-        lsid: {id: lsid2},
-        txnNumber: NumberLong(1)
-    };
-    assert.commandWorked(db.runCommand(cmdObj2));
-    assert.eq(1, db.user.find({_id: 1}).toArray()[0].x);
-
-    assert.eq(1, config.transactions.find({'_id.id': lsid1}).itcount());
-    assert.eq(1, config.transactions.find({'_id.id': lsid2}).itcount());
-
-    // Invalidating lsid1 doesn't impact lsid2, but allows same statement to be executed again
-    assert.writeOK(config.transactions.remove({'_id.id': lsid1}));
-    assert.commandWorked(db.runCommand(cmdObj1));
-    assert.eq(2, db.user.find({_id: 0}).toArray()[0].x);
-    assert.commandWorked(db.runCommand(cmdObj2));
-    assert.eq(1, db.user.find({_id: 1}).toArray()[0].x);
-
-    // Ensure lsid1 is properly tracked after the recreate
-    assert.commandWorked(db.runCommand(cmdObj1));
-    assert.eq(2, db.user.find({_id: 0}).toArray()[0].x);
-
-    // Ensure garbage data cannot be written to the `config.transactions` collection
-    assert.writeError(config.transactions.insert({_id: 'String'}));
-    assert.writeError(config.transactions.insert({_id: {UnknownField: 'Garbage'}}));
-
-    // Ensure inserting an invalid session record manually without all the required fields causes
-    // the session to not work anymore for retryable writes for that session, but not for any other
-    const lsidManual = config.transactions.find({'_id.id': lsid1}).toArray()[0]._id;
-    assert.writeOK(config.transactions.remove({'_id.id': lsid1}));
-    assert.writeOK(config.transactions.insert({_id: lsidManual}));
-
-    const lsid3 = UUID();
-    assert.commandWorked(db.runCommand({
-        update: 'user',
-        updates: [{q: {_id: 2}, u: {$inc: {x: 1}}, upsert: true}],
-        lsid: {id: lsid3},
-        txnNumber: NumberLong(1)
-    }));
-    assert.eq(1, db.user.find({_id: 2}).toArray()[0].x);
-
-    // Ensure dropping the `config.transactions` collection breaks the retryable writes feature, but
-    // doesn't crash the server
-    assert(config.transactions.drop());
-    assert.commandFailed(db.runCommand(cmdObj2));
-    assert.eq(1, db.user.find({_id: 1}).toArray()[0].x);
-
-    assert(config.dropDatabase());
-    assert.commandFailed(db.runCommand(cmdObj2));
-    assert.eq(1, db.user.find({_id: 1}).toArray()[0].x);
-
-    replTest.stopSet();
-})();
diff --git a/jstests/sharding/retryable_writes.js b/jstests/sharding/retryable_writes.js
index cd1914e83bb..adaccecc104 100644
--- a/jstests/sharding/retryable_writes.js
+++ b/jstests/sharding/retryable_writes.js
@@ -344,7 +344,7 @@
     }
 
     // Tests for replica set
-    var replTest = new ReplSetTest({nodes: 2});
+    var replTest = new ReplSetTest({nodes: 1});
     replTest.startSet();
     replTest.initiate();
 
diff --git a/src/mongo/db/op_observer_impl.cpp b/src/mongo/db/op_observer_impl.cpp
index be7b8c9be8c..160c1fac2d6 100644
--- a/src/mongo/db/op_observer_impl.cpp
+++ b/src/mongo/db/op_observer_impl.cpp
@@ -79,7 +79,14 @@ void onWriteOpCompleted(OperationContext* opCtx,
     if (lastStmtIdWriteTs.isNull())
         return;
 
-    if (session) {
+    if (nss == NamespaceString::kSessionTransactionsTableNamespace) {
+        uassert(40528,
+                str::stream() << "Direct writes against "
+                              << NamespaceString::kSessionTransactionsTableNamespace.ns()
+                              << "  cannot be performed using a transaction or on a session.",
+                !opCtx->getLogicalSessionId());
+        SessionCatalog::get(opCtx)->resetSessions();
+    } else if (session) {
         session->onWriteOpCompletedOnPrimary(
             opCtx, *opCtx->getTxnNumber(), std::move(stmtIdsWritten), lastStmtIdWriteTs);
     }
@@ -284,10 +291,6 @@ void OpObserverImpl::onInserts(OperationContext* opCtx,
         for (auto it = begin; it != end; it++) {
             FeatureCompatibilityVersion::onInsertOrUpdate(opCtx, it->doc);
         }
-    } else if (nss == NamespaceString::kSessionTransactionsTableNamespace && !lastOpTime.isNull()) {
-        for (auto it = begin; it != end; it++) {
-            SessionCatalog::get(opCtx)->invalidateSessions(opCtx, it->doc);
-        }
     }
 
     std::vector<StmtId> stmtIdsWritten;
@@ -321,9 +324,6 @@ void OpObserverImpl::onUpdate(OperationContext* opCtx, const OplogUpdateEntryArg
         DurableViewCatalog::onExternalChange(opCtx, args.nss);
     } else if (args.nss.ns() == FeatureCompatibilityVersion::kCollection) {
         FeatureCompatibilityVersion::onInsertOrUpdate(opCtx, args.updatedDoc);
-    } else if (args.nss == NamespaceString::kSessionTransactionsTableNamespace &&
-               !opTime.isNull()) {
-        SessionCatalog::get(opCtx)->invalidateSessions(opCtx, args.updatedDoc);
     }
 
     onWriteOpCompleted(opCtx, args.nss, session, std::vector<StmtId>{args.stmtId}, opTime);
@@ -348,6 +348,7 @@ void OpObserverImpl::onDelete(OperationContext* opCtx,
     }
 
     Session* const session = opCtx->getTxnNumber() ? OperationContextSession::get(opCtx) : nullptr;
+
     const auto opTime =
         replLogDelete(opCtx, nss, uuid, session, stmtId, deleteState, fromMigrate, deletedDoc);
 
@@ -365,8 +366,6 @@ void OpObserverImpl::onDelete(OperationContext* opCtx,
         DurableViewCatalog::onExternalChange(opCtx, nss);
     } else if (nss.ns() == FeatureCompatibilityVersion::kCollection) {
         FeatureCompatibilityVersion::onDelete(opCtx, deleteState.documentKey);
-    } else if (nss == NamespaceString::kSessionTransactionsTableNamespace && !opTime.isNull()) {
-        SessionCatalog::get(opCtx)->invalidateSessions(opCtx, deleteState.documentKey);
     }
 
     onWriteOpCompleted(opCtx, nss, session, std::vector<StmtId>{stmtId}, opTime);
@@ -484,8 +483,6 @@ void OpObserverImpl::onDropDatabase(OperationContext* opCtx, const std::string&
 
     if (dbName == FeatureCompatibilityVersion::kDatabase) {
         FeatureCompatibilityVersion::onDropCollection(opCtx);
-    } else if (dbName == NamespaceString::kSessionTransactionsTableNamespace.db()) {
-        SessionCatalog::get(opCtx)->invalidateSessions(opCtx, boost::none);
     }
 
     NamespaceUUIDCache::get(opCtx).evictNamespacesInDatabase(dbName);
@@ -509,10 +506,10 @@ repl::OpTime OpObserverImpl::onDropCollection(OperationContext* opCtx,
 
     if (collectionName.coll() == DurableViewCatalog::viewsCollectionName()) {
         DurableViewCatalog::onExternalChange(opCtx, collectionName);
-    } else if (collectionName.ns() == FeatureCompatibilityVersion::kCollection) {
+    }
+
+    if (collectionName.ns() == FeatureCompatibilityVersion::kCollection) {
         FeatureCompatibilityVersion::onDropCollection(opCtx);
-    } else if (collectionName == NamespaceString::kSessionTransactionsTableNamespace) {
-        SessionCatalog::get(opCtx)->invalidateSessions(opCtx, boost::none);
     }
 
     AuthorizationManager::get(opCtx->getServiceContext())
diff --git a/src/mongo/db/repl/rollback_impl.cpp b/src/mongo/db/repl/rollback_impl.cpp
index c3a192c8d7a..1b791b63d9c 100644
--- a/src/mongo/db/repl/rollback_impl.cpp
+++ b/src/mongo/db/repl/rollback_impl.cpp
@@ -243,7 +243,7 @@ void RollbackImpl::_resetSessions(OperationContext* opCtx) {
 
     log() << "resetting in-memory state of active sessions";
 
-    SessionCatalog::get(opCtx)->invalidateSessions(opCtx, boost::none);
+    SessionCatalog::get(opCtx)->resetSessions();
 }
 
 void RollbackImpl::_transitionFromRollbackToSecondary(OperationContext* opCtx) {
diff --git a/src/mongo/db/repl/rs_rollback.cpp b/src/mongo/db/repl/rs_rollback.cpp
index 9af9794174f..6887a962baf 100644
--- a/src/mongo/db/repl/rs_rollback.cpp
+++ b/src/mongo/db/repl/rs_rollback.cpp
@@ -1346,7 +1346,7 @@ void rollback_internal::syncFixUp(OperationContext* opCtx,
 
     // If necessary, clear the memory of existing sessions.
     if (fixUpInfo.refetchTransactionDocs) {
-        SessionCatalog::get(opCtx)->invalidateSessions(opCtx, boost::none);
+        SessionCatalog::get(opCtx)->resetSessions();
     }
 
     // Reload the lastAppliedOpTime and lastDurableOpTime value in the replcoord and the
diff --git a/src/mongo/db/session.cpp b/src/mongo/db/session.cpp
index 9e3bece6063..d2e7c775619 100644
--- a/src/mongo/db/session.cpp
+++ b/src/mongo/db/session.cpp
@@ -323,6 +323,7 @@ UpdateRequest Session::_makeUpdateRequest(WithLock,
                                           TxnNumber newTxnNumber,
                                           Timestamp newLastWriteTs) const {
     UpdateRequest updateRequest(NamespaceString::kSessionTransactionsTableNamespace);
+    updateRequest.setUpsert(true);
 
     if (_lastWrittenSessionRecord) {
         updateRequest.setQuery(_lastWrittenSessionRecord->toBSON());
@@ -342,7 +343,6 @@ UpdateRequest Session::_makeUpdateRequest(WithLock,
 
         updateRequest.setQuery(updateBSON);
         updateRequest.setUpdates(updateBSON);
-        updateRequest.setUpsert(true);
     }
 
     return updateRequest;
diff --git a/src/mongo/db/session_catalog.cpp b/src/mongo/db/session_catalog.cpp
index 8f73410fe1c..f155d10639e 100644
--- a/src/mongo/db/session_catalog.cpp
+++ b/src/mongo/db/session_catalog.cpp
@@ -104,7 +104,7 @@ boost::optional<UUID> SessionCatalog::getTransactionTableUUID(OperationContext*
 }
 
 void SessionCatalog::onStepUp(OperationContext* opCtx) {
-    invalidateSessions(opCtx, boost::none);
+    resetSessions();
 
     DBDirectClient client(opCtx);
 
@@ -172,35 +172,10 @@ ScopedSession SessionCatalog::getOrCreateSession(OperationContext* opCtx,
     return ss;
 }
 
-void SessionCatalog::invalidateSessions(OperationContext* opCtx,
-                                        boost::optional<BSONObj> singleSessionDoc) {
-    uassert(40528,
-            str::stream() << "Direct writes against "
-                          << NamespaceString::kSessionTransactionsTableNamespace.ns()
-                          << " cannot be performed using a transaction or on a session.",
-            !opCtx->getLogicalSessionId());
-
-    const auto invalidateSessionFn = [&](WithLock, SessionRuntimeInfoMap::iterator it) {
-        auto& sri = it->second;
-        sri->txnState.invalidate();
-        _txnTable.erase(it);
-    };
-
+void SessionCatalog::resetSessions() {
     stdx::lock_guard<stdx::mutex> lg(_mutex);
-
-    if (singleSessionDoc) {
-        const auto lsid = LogicalSessionId::parse(IDLParserErrorContext("lsid"),
-                                                  singleSessionDoc->getField("_id").Obj());
-
-        auto it = _txnTable.find(lsid);
-        if (it != _txnTable.end()) {
-            invalidateSessionFn(lg, it);
-        }
-    } else {
-        auto it = _txnTable.begin();
-        while (it != _txnTable.end()) {
-            invalidateSessionFn(lg, it++);
-        }
+    for (const auto& it : _txnTable) {
+        it.second->txnState.invalidate();
     }
 }
 
diff --git a/src/mongo/db/session_catalog.h b/src/mongo/db/session_catalog.h
index e0432c439f5..3e03c97d83d 100644
--- a/src/mongo/db/session_catalog.h
+++ b/src/mongo/db/session_catalog.h
@@ -112,16 +112,10 @@ public:
     ScopedSession getOrCreateSession(OperationContext* opCtx, const LogicalSessionId& lsid);
 
     /**
-     * Callback to be invoked when it is suspected that the on-disk session contents might not be in
-     * sync with what is in the sessions cache.
-     *
-     * If no specific document is available, the method will invalidate all sessions. Otherwise if
-     * one is avaiable (which is the case for insert/update/delete), it must contain _id field with
-     * a valid session entry, in which case only that particular session will be invalidated. If the
-     * _id field is missing or doesn't contain a valid serialization of logical session, the method
-     * will throw. This prevents invalid entries from making it in the collection.
+     * Resets all created sessions and increments their generation, forcing each to be reloaded by
+     * subsequent write commands. Invoked after rollback.
      */
-    void invalidateSessions(OperationContext* opCtx, boost::optional<BSONObj> singleSessionDoc);
+    void resetSessions();
 
 private:
     struct SessionRuntimeInfo {