SERVER-35173 Added autocommit value to currentOp's transaction sub-document

6 files changed, 151 insertions, 22 deletions

diff --git a/src/mongo/db/curop.cpp b/src/mongo/db/curop.cpp
index 08fb567b729..bc61bb30574 100644
--- a/src/mongo/db/curop.cpp
+++ b/src/mongo/db/curop.cpp
@@ -257,11 +257,6 @@ void CurOp::reportCurrentOpForClient(OperationContext* opCtx,
             lsid->serialize(&lsidBuilder);
         }
 
-        if (auto txnNumber = clientOpCtx->getTxnNumber()) {
-            infoBuilder->append("transaction",
-                                BSON("parameters" << BSON("txnNumber" << *txnNumber)));
-        }
-
         CurOp::get(clientOpCtx)->reportState(infoBuilder, truncateOps);
     }
 }
diff --git a/src/mongo/db/pipeline/pipeline_d.cpp b/src/mongo/db/pipeline/pipeline_d.cpp
index 18e442ea6f5..5a59d138851 100644
--- a/src/mongo/db/pipeline/pipeline_d.cpp
+++ b/src/mongo/db/pipeline/pipeline_d.cpp
@@ -964,8 +964,14 @@ BSONObj PipelineD::MongoDInterface::_reportCurrentOpForClient(
     CurOp::reportCurrentOpForClient(
         opCtx, client, (truncateOps == CurrentOpTruncateMode::kTruncateOps), &builder);
 
-    // Append lock stats before returning.
-    if (auto clientOpCtx = client->getOperationContext()) {
+    OperationContext* clientOpCtx = client->getOperationContext();
+
+    if (clientOpCtx) {
+        if (auto opCtxSession = OperationContextSession::get(clientOpCtx)) {
+            opCtxSession->reportUnstashedState(&builder);
+        }
+
+        // Append lock stats before returning.
         if (auto lockerInfo = clientOpCtx->lockState()->getLockerInfo()) {
             fillLockerInfo(*lockerInfo, builder);
         }
diff --git a/src/mongo/db/session.cpp b/src/mongo/db/session.cpp
index 0eff4580d3d..116dca3658f 100644
--- a/src/mongo/db/session.cpp
+++ b/src/mongo/db/session.cpp
@@ -1104,8 +1104,9 @@ void Session::reportStashedState(BSONObjBuilder* builder) const {
                 BSONObjBuilder lsid(builder->subobjStart("lsid"));
                 getSessionId().serialize(&lsid);
             }
-            builder->append("transaction",
-                            BSON("parameters" << BSON("txnNumber" << _activeTxnNumber)));
+            BSONObjBuilder transactionBuilder;
+            _reportTransactionStats(ls, &transactionBuilder);
+            builder->append("transaction", transactionBuilder.obj());
             builder->append("waitingForLock", false);
             builder->append("active", false);
             fillLockerInfo(*lockerInfo, *builder);
@@ -1113,6 +1114,30 @@ void Session::reportStashedState(BSONObjBuilder* builder) const {
     }
 }
 
+void Session::reportUnstashedState(BSONObjBuilder* builder) const {
+    stdx::lock_guard<stdx::mutex> ls(_mutex);
+
+    if (!_txnResourceStash) {
+        BSONObjBuilder transactionBuilder;
+        _reportTransactionStats(ls, &transactionBuilder);
+        builder->append("transaction", transactionBuilder.obj());
+    }
+}
+
+void Session::_reportTransactionStats(WithLock wl, BSONObjBuilder* builder) const {
+    BSONObjBuilder parametersBuilder(builder->subobjStart("parameters"));
+    parametersBuilder.append("txnNumber", _activeTxnNumber);
+
+    if (!_txnState.inMultiDocumentTransaction(wl)) {
+        // For retryable writes, we only include the txnNumber.
+        parametersBuilder.done();
+        return;
+    }
+    parametersBuilder.append("autocommit", _autocommit);
+
+    parametersBuilder.done();
+}
+
 void Session::_checkValid(WithLock) const {
     uassert(ErrorCodes::ConflictingOperationInProgress,
             str::stream() << "Session " << getSessionId()
diff --git a/src/mongo/db/session.h b/src/mongo/db/session.h
index 4e39a5295ed..f4e81465137 100644
--- a/src/mongo/db/session.h
+++ b/src/mongo/db/session.h
@@ -362,6 +362,13 @@ public:
     void reportStashedState(BSONObjBuilder* builder) const;
 
     /**
+     * If this session is not holding stashed locks in _txnResourceStash (transaction is active),
+     * reports the current state of the session using the provided builder. Locks the session
+     * object's mutex while running.
+     */
+    void reportUnstashedState(BSONObjBuilder* builder) const;
+
+    /**
      * Convenience method which creates and populates a BSONObj containing the stashed state.
      * Returns an empty BSONObj if this session has no stashed resources.
      */
@@ -541,6 +548,10 @@ private:
     // truncated because it was too old.
     bool _hasIncompleteHistory{false};
 
+    // Reports transaction stats for both active and inactive transactions using the provided
+    // builder.
+    void _reportTransactionStats(WithLock wl, BSONObjBuilder* builder) const;
+
     // Caches what is known to be the last written transaction record for the session
     boost::optional<SessionTxnRecord> _lastWrittenSessionRecord;
 
diff --git a/src/mongo/db/session_catalog.cpp b/src/mongo/db/session_catalog.cpp
index 6dd27f2f501..62f01f567a1 100644
--- a/src/mongo/db/session_catalog.cpp
+++ b/src/mongo/db/session_catalog.cpp
@@ -254,6 +254,9 @@ OperationContextSession::OperationContextSession(OperationContext* opCtx,
     auto& checkedOutSession = operationSessionDecoration(opCtx);
     if (!checkedOutSession) {
         auto sessionTransactionTable = SessionCatalog::get(opCtx);
+        // We acquire a Client lock here to guard the construction of this session so that
+        // references to this session are safe to use while the lock is held.
+        stdx::lock_guard<Client> lk(*opCtx->getClient());
         checkedOutSession.emplace(sessionTransactionTable->checkOutSession(opCtx));
     } else {
         // The only reason to be trying to check out a session when you already have a session
@@ -281,6 +284,9 @@ OperationContextSession::~OperationContextSession() {
     }
 
     auto& checkedOutSession = operationSessionDecoration(_opCtx);
+    // We acquire a Client lock here to guard the destruction of this session so that references to
+    // this session are safe to use while the lock is held.
+    stdx::lock_guard<Client> lk(*_opCtx->getClient());
     checkedOutSession.reset();
 }
 
diff --git a/src/mongo/db/session_test.cpp b/src/mongo/db/session_test.cpp
index 5052e0150bd..cb05eb9cdfc 100644
--- a/src/mongo/db/session_test.cpp
+++ b/src/mongo/db/session_test.cpp
@@ -728,6 +728,7 @@ TEST_F(SessionTest, StashAndUnstashResources) {
 TEST_F(SessionTest, ReportStashedResources) {
     const auto sessionId = makeLogicalSessionIdForTest();
     const TxnNumber txnNum = 20;
+    const bool autocommit = false;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
 
@@ -737,7 +738,7 @@ TEST_F(SessionTest, ReportStashedResources) {
     Session session(sessionId);
     session.refreshFromStorageIfNeeded(opCtx());
 
-    session.beginOrContinueTxn(opCtx(), txnNum, false, true, "testDB", "find");
+    session.beginOrContinueTxn(opCtx(), txnNum, autocommit, true, "testDB", "find");
 
     repl::ReadConcernArgs readConcernArgs;
     ASSERT_OK(readConcernArgs.initialize(BSON("find"
@@ -757,18 +758,20 @@ TEST_F(SessionTest, ReportStashedResources) {
     const auto lockerInfo = opCtx()->lockState()->getLockerInfo();
     ASSERT(lockerInfo);
 
-    auto txnDoc = BSON("parameters" << BSON("txnNumber" << txnNum));
-    auto reportBuilder =
-        std::move(BSONObjBuilder() << "host" << getHostNameCachedAndPort() << "desc"
-                                   << "inactive transaction"
-                                   << "lsid"
-                                   << sessionId.toBSON()
-                                   << "transaction"
-                                   << txnDoc
-                                   << "waitingForLock"
-                                   << false
-                                   << "active"
-                                   << false);
+    BSONObjBuilder reportBuilder;
+    reportBuilder.append("host", getHostNameCachedAndPort());
+    reportBuilder.append("desc", "inactive transaction");
+    reportBuilder.append("lsid", sessionId.toBSON());
+    // Build the transaction parameters sub-document.
+    BSONObjBuilder transactionBuilder(reportBuilder.subobjStart("transaction"));
+    BSONObjBuilder parametersBuilder(transactionBuilder.subobjStart("parameters"));
+    parametersBuilder.append("txnNumber", txnNum);
+    parametersBuilder.append("autocommit", autocommit);
+    parametersBuilder.done();
+    transactionBuilder.done();
+
+    reportBuilder.append("waitingForLock", false);
+    reportBuilder.append("active", false);
     fillLockerInfo(*lockerInfo, reportBuilder);
 
     // Stash resources. The original Locker and RecoveryUnit now belong to the stash.
@@ -793,6 +796,89 @@ TEST_F(SessionTest, ReportStashedResources) {
     session.commitTransaction(opCtx());
 }
 
+TEST_F(SessionTest, ReportUnstashedResources) {
+    const auto sessionId = makeLogicalSessionIdForTest();
+    const TxnNumber txnNum = 20;
+    const bool autocommit = false;
+    opCtx()->setLogicalSessionId(sessionId);
+    opCtx()->setTxnNumber(txnNum);
+
+    ASSERT(opCtx()->lockState());
+    ASSERT(opCtx()->recoveryUnit());
+
+    Session session(sessionId);
+    session.refreshFromStorageIfNeeded(opCtx());
+
+    session.beginOrContinueTxn(opCtx(), txnNum, autocommit, true, "testDB", "find");
+
+    repl::ReadConcernArgs readConcernArgs;
+    ASSERT_OK(readConcernArgs.initialize(BSON("find"
+                                              << "test"
+                                              << repl::ReadConcernArgs::kReadConcernFieldName
+                                              << BSON(repl::ReadConcernArgs::kLevelFieldName
+                                                      << "snapshot"))));
+    repl::ReadConcernArgs::get(opCtx()) = readConcernArgs;
+
+    // Perform initial unstash which sets up a WriteUnitOfWork.
+    session.unstashTransactionResources(opCtx(), "find");
+    ASSERT(opCtx()->getWriteUnitOfWork());
+    ASSERT(opCtx()->lockState()->isLocked());
+
+    // Build a BSONObj containing the details which we expect to see reported when we call
+    // Session::reportUnstashedState.
+    BSONObjBuilder reportBuilder;
+    BSONObjBuilder transactionBuilder(reportBuilder.subobjStart("transaction"));
+    BSONObjBuilder parametersBuilder(transactionBuilder.subobjStart("parameters"));
+    parametersBuilder.append("txnNumber", txnNum);
+    parametersBuilder.append("autocommit", autocommit);
+    parametersBuilder.done();
+    transactionBuilder.done();
+
+    // Verify that the Session's report of its own unstashed state aligns with our expectations.
+    BSONObjBuilder unstashedStateBuilder;
+    session.reportUnstashedState(&unstashedStateBuilder);
+    ASSERT_BSONOBJ_EQ(unstashedStateBuilder.obj(), reportBuilder.obj());
+
+    // Stash resources. The original Locker and RecoveryUnit now belong to the stash.
+    session.stashTransactionResources(opCtx());
+    ASSERT(!opCtx()->getWriteUnitOfWork());
+
+    // With the resources stashed, verify that the Session reports an empty unstashed state.
+    BSONObjBuilder builder;
+    session.reportUnstashedState(&builder);
+    ASSERT(builder.obj().isEmpty());
+}
+
+TEST_F(SessionTest, ReportUnstashedResourcesForARetryableWrite) {
+    const auto sessionId = makeLogicalSessionIdForTest();
+    const TxnNumber txnNum = 20;
+    opCtx()->setLogicalSessionId(sessionId);
+    opCtx()->setTxnNumber(txnNum);
+
+    ASSERT(opCtx()->lockState());
+    ASSERT(opCtx()->recoveryUnit());
+
+    Session session(sessionId);
+    session.refreshFromStorageIfNeeded(opCtx());
+
+    session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none, "testDB", "find");
+    session.unstashTransactionResources(opCtx(), "find");
+
+    // Build a BSONObj containing the details which we expect to see reported when we call
+    // Session::reportUnstashedState. For a retryable write, we should only include the txnNumber.
+    BSONObjBuilder reportBuilder;
+    BSONObjBuilder transactionBuilder(reportBuilder.subobjStart("transaction"));
+    BSONObjBuilder parametersBuilder(transactionBuilder.subobjStart("parameters"));
+    parametersBuilder.append("txnNumber", txnNum);
+    parametersBuilder.done();
+    transactionBuilder.done();
+
+    // Verify that the Session's report of its own unstashed state aligns with our expectations.
+    BSONObjBuilder unstashedStateBuilder;
+    session.reportUnstashedState(&unstashedStateBuilder);
+    ASSERT_BSONOBJ_EQ(unstashedStateBuilder.obj(), reportBuilder.obj());
+}
+
 TEST_F(SessionTest, CannotSpecifyStartTransactionOnInProgressTxn) {
     const auto sessionId = makeLogicalSessionIdForTest();
     Session session(sessionId);