[SERVER-38631] Restore entire state of a session in the shell if a node is restarted Created: 14/Dec/18  Updated: 29/Oct/23  Resolved: 24/Jan/19

Status: Closed
Project: Core Server
Component/s: Testing Infrastructure
Affects Version/s: None
Fix Version/s: 4.1.8

Type: Task Priority: Major - P3
Reporter: Samyukta Lanka Assignee: Siyuan Zhou
Resolution: Fixed Votes: 0
Labels: prepare_testing
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
is depended on by SERVER-36310 Unblacklist transactions from all jsc... Closed
Backwards Compatibility: Fully Compatible
Sprint: Repl 2019-01-28
Participants:

 Description   

If we restart a node that had an active session, the shell starts a new session instead of restoring the state of the old session. This means that if we were running a prepared transaction before the restart, we can't call commitTransaction or abortTransaction without the shell returning an error saying that there is no active transaction on the session. For now, we've been using a workaround to manually set the txnNumber and lsid so that we can get runCommand to work. However, we still can't run any other functions on the session and expect them to be run on the session we wanted (and we can't use the commitTransaction or abortTransaction functions, we have to use runCommand).

It would be great to be able to restore the entire state of the session beyond just the txnNumber and lsid. This would be particularly useful for passthrough suites that do failover testing for prepared transactions.



 Comments   
Comment by Siyuan Zhou [ 24/Jan/19 ]

After the above commit, we should be able to create a delegating driver session from the one targeting to the old primary.

const session = primary.startSession({causalConsistency: false});
...
// Failover happened and `newPrimary` steps up.
...
const newSession = new _DelegatingDriverSession(newPrimary, session);
newSession.startTransaction({writeConcern: {w: "majority"}});
...
newSession.commitTransaction();

Comment by Siyuan Zhou [ 24/Jan/19 ]

max.hirschhorn, closing the ticket. I'm happy to reopen this ticket or create a new ticket if you think we should remove the reference of client in the constructor of ServerSession as discussed in the code review.

Comment by Githook User [ 24/Jan/19 ]

Author:

{'username': 'visualzhou', 'email': 'siyuan.zhou@mongodb.com', 'name': 'Siyuan Zhou'}

Message: SERVER-38631 Let DriverSession own SessionAwareClient.
Branch: master
https://github.com/mongodb/mongo/commit/4047ee99316dfb0739ad92f351bcc20ffb0aef4a

Comment by Max Hirschhorn [ 24/Jan/19 ]

Max Hirschhorn, do you think we should make ServerSession call into the driver session to get the client?

Yes, I think changing the endTransaction() function to call through to the SessionAwareClient would make the behavior of commands run internally by DriverSession API methods consistent with the behavior of commands run by a user.

diff --git a/jstests/replsets/recover_multiple_prepared_transactions_startup.js b/jstests/replsets/recover_multiple_prepared_transactions_startup.js
index 6630703d6a..e60dea1fc9 100644
--- a/jstests/replsets/recover_multiple_prepared_transactions_startup.js
+++ b/jstests/replsets/recover_multiple_prepared_transactions_startup.js
@@ -58,11 +58,6 @@
 
     jsTestLog("Node was restarted");
 
-    // XXX: The commitTransaction*() and abortTransaction*() helpers access the ServerSession object
-    // directly rather than going through _getSessionAwareClient(). This means they ignore that
-    // we're using a _DelegatingDriverSession.
-    reconnect(primary);
-
     primary = replTest.getPrimary();
     testDB = primary.getDB(dbName);
 
diff --git a/src/mongo/shell/session.js b/src/mongo/shell/session.js
index 62d5a070df..24e4182bfb 100644
--- a/src/mongo/shell/session.js
+++ b/src/mongo/shell/session.js
@@ -822,7 +822,8 @@ var {
             let res;
             try {
                 // run command against the admin database.
-                res = this.client.runCommand(driverSession, "admin", cmd, 0);
+                res = driverSession._getSessionAwareClient().runCommand(
+                    driverSession, "admin", cmd, 0);
             } finally {
                 if (commandName === "commitTransaction") {
                     setTxnState("committed");

Comment by Siyuan Zhou [ 24/Jan/19 ]

+ // XXX: The commitTransaction*() and abortTransaction*() helpers access the ServerSession object
+ // directly rather than going through _getSessionAwareClient(). This means they ignore that
+ // we're using a _DelegatingDriverSession.

max.hirschhorn, do you think we should make ServerSession call into the driver session to get the client?

Comment by Siyuan Zhou [ 20/Dec/18 ]

Thanks max.hirschhorn, this is exactly what we need. Assigned to Repl team to merge your patch.

Comment by Max Hirschhorn [ 18/Dec/18 ]

samy.lanka, siyuan.zhou, restarting a node involves constructing a new Mongo connection object and therefore creating a new chain of DriverSession, DB, and DBCollection objects. I'd expect DelegatingDriverSession to fit what you're interested in doing because you effectively want to use an existing DriverSession object with a different Mongo connection object than it was originally constructed with.

diff --git a/jstests/replsets/recover_multiple_prepared_transactions_startup.js b/jstests/replsets/recover_multiple_prepared_transactions_startup.js
index 21781ce02e..6630703d6a 100644
--- a/jstests/replsets/recover_multiple_prepared_transactions_startup.js
+++ b/jstests/replsets/recover_multiple_prepared_transactions_startup.js
@@ -8,6 +8,7 @@
 (function() {
     "use strict";
     load("jstests/core/txns/libs/prepare_helpers.js");
+    load("jstests/replsets/rslib.js");
 
     const replTest = new ReplSetTest({nodes: 1});
     replTest.startSet();
@@ -48,12 +49,6 @@
     assert.commandWorked(sessionColl2.update({_id: 2}, {_id: 2, a: 1}));
     let prepareTimestamp2 = PrepareHelpers.prepareTransaction(session2);
 
-    const lsid = session.getSessionId();
-    const txnNumber = session.getTxnNumber_forTesting();
-
-    const lsid2 = session2.getSessionId();
-    const txnNumber2 = session2.getTxnNumber_forTesting();
-
     jsTestLog("Restarting node");
 
     // Perform a clean shutdown and restart. Note that the 'disableSnapshotting' failpoint will be
@@ -63,19 +58,22 @@
 
     jsTestLog("Node was restarted");
 
+    // XXX: The commitTransaction*() and abortTransaction*() helpers access the ServerSession object
+    // directly rather than going through _getSessionAwareClient(). This means they ignore that
+    // we're using a _DelegatingDriverSession.
+    reconnect(primary);
+
     primary = replTest.getPrimary();
     testDB = primary.getDB(dbName);
 
-    session = primary.startSession({causalConsistency: false});
-    sessionDB = session.getDatabase(dbName);
+    // Forward operations using the new connection to the primary while using the same lsid and
+    // txnNumber from before the restart. This ensures that we're working with the same session and
+    // transaction.
+    session = new _DelegatingDriverSession(primary, session);
+    session2 = new _DelegatingDriverSession(primary, session2);
 
-    session2 = primary.startSession({causalConsistency: false});
-    sessionDB2 = session.getDatabase(dbName);
-
-    // Force the first session to use the same lsid and txnNumber as from before the restart. This
-    // ensures that we're working with the same session and transaction.
-    session._serverSession.handle.getId = () => lsid;
-    session.setTxnNumber_forTesting(txnNumber);
+    sessionDB = session.getDatabase(dbName);
+    sessionDB2 = session2.getDatabase(dbName);
 
     jsTestLog("Checking that the first transaction is properly prepared");
 
@@ -91,13 +89,7 @@
         ErrorCodes.MaxTimeMSExpired);
 
     // Make sure that we cannot add other operations to the first transaction since it is prepared.
-    assert.commandFailedWithCode(sessionDB.runCommand({
-        insert: collName,
-        documents: [{_id: 3}],
-        txnNumber: NumberLong(txnNumber),
-        stmtId: NumberInt(2),
-        autocommit: false
-    }),
+    assert.commandFailedWithCode(sessionDB.runCommand({insert: collName, documents: [{_id: 3}]}),
                                  ErrorCodes.PreparedTransactionInProgress);
 
     jsTestLog("Committing the first transaction");
@@ -107,15 +99,10 @@
     assert.commandWorked(sessionDB.adminCommand({
         commitTransaction: 1,
         commitTimestamp: commitTimestamp,
-        txnNumber: NumberLong(txnNumber),
+        txnNumber: NumberLong(session.getTxnNumber_forTesting()),
         autocommit: false
     }));
 
-    // Force the second session to use the same lsid and txnNumber as from before the restart.
-    // This ensures that we're working with the same session and transaction.
-    session._serverSession.handle.getId = () => lsid2;
-    session.setTxnNumber_forTesting(txnNumber2);
-
     jsTestLog("Checking that the second transaction is properly prepared");
 
     // Make sure that we can't read changes to the document from the second transaction after
@@ -133,23 +120,19 @@
     assert.commandFailedWithCode(sessionDB2.runCommand({
         insert: collName,
         documents: [{_id: 3}],
-        txnNumber: NumberLong(txnNumber2),
-        stmtId: NumberInt(2),
-        autocommit: false
     }),
                                  ErrorCodes.PreparedTransactionInProgress);
 
     jsTestLog("Aborting the second transaction");
 
     // Make sure we can successfully abort the second transaction after recovery.
-    assert.commandWorked(sessionDB2.adminCommand(
-        {abortTransaction: 1, txnNumber: NumberLong(txnNumber2), autocommit: false}));
+    assert.commandWorked(session2.abortTransaction_forTesting());
 
     jsTestLog("Attempting to run another transction");
 
     // Make sure that we can run another conflicting transaction after recovery without any
     // problems.
-    session.startTransaction();
+    session.startTransaction_forTesting(undefined, {ignoreActiveTxn: true});
     assert.commandWorked(sessionDB[collName].update({_id: 1}, {_id: 1, a: 3}));
     prepareTimestamp = PrepareHelpers.prepareTransaction(session);
     assert.commandWorked(PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp));

Comment by Siyuan Zhou [ 14/Dec/18 ]

This may also be useful to passthrough test suites.

Comment by Samyukta Lanka [ 14/Dec/18 ]

max.hirschhorn Let us know if you think this would be feasible and worth the effort.

Generated at Thu Feb 08 04:49:30 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.