SERVER-63498 Upconvert InternalTransactionNotSupported errors during fcv downgrade

2 files changed, 120 insertions, 14 deletions

diff --git a/jstests/concurrency/fsm_workloads/random_internal_transactions_setFCV_operations.js b/jstests/concurrency/fsm_workloads/random_internal_transactions_setFCV_operations.js
index ad6edbf5b51..84acc9fcd50 100644
--- a/jstests/concurrency/fsm_workloads/random_internal_transactions_setFCV_operations.js
+++ b/jstests/concurrency/fsm_workloads/random_internal_transactions_setFCV_operations.js
@@ -26,20 +26,6 @@ var $config = extendWorkload($config, function($config, $super) {
     // with a setFCV command, so we want to be able to catch those as acceptable killSession errors.
     $config.data.retryOnKilledSession = true;
 
-    const baseIsUpdateShardKeyErrorAcceptable =
-        $config.data.isUpdateShardKeyErrorAcceptable.bind($config.data);
-
-    $config.data.isUpdateShardKeyErrorAcceptable = function isUpdateShardKeyAcceptable(
-        errCode, errMsg, errorLabels) {
-        // TODO: SERVER-63498 Remove this when InternalTransactionNotSupported errors are
-        // upconverted to a retryable error.
-        if (this.areInternalTransactionsEnabled &&
-            errCode == ErrorCodes.InternalTransactionNotSupported) {
-            return true;
-        }
-        return baseIsUpdateShardKeyErrorAcceptable(errCode, errMsg, errorLabels);
-    };
-
     $config.states.setFCV = function(db, collName, connCache) {
         const fcvValues = [lastLTSFCV, lastContinuousFCV, latestFCV];
         const targetFCV = fcvValues[Random.randInt(3)];
diff --git a/jstests/multiVersion/targetedTestsLastLtsFeatures/internal_transactions_transient_error_after_setFCV.js b/jstests/multiVersion/targetedTestsLastLtsFeatures/internal_transactions_transient_error_after_setFCV.js
new file mode 100644
index 00000000000..a8350292aec
--- /dev/null
+++ b/jstests/multiVersion/targetedTestsLastLtsFeatures/internal_transactions_transient_error_after_setFCV.js
@@ -0,0 +1,120 @@
+/*
+ * Tests that we send the proper error back to the client to retry the transaction if there's a
+ * failover and FCV change that causes the server to lose previous transaction metadata.
+ *
+ * @tags: [featureFlagInternalTransactions]
+ */
+
+(function() {
+'use strict';
+
+load("jstests/libs/uuid_util.js");
+
+const kDbName = "testDb";
+const kCollName = "testColl";
+
+const ns = kDbName + "." + kCollName;
+
+// A monotonically increasing value to deduplicate document insertions.
+let docVal = 5;
+
+const st = new ShardingTest({shards: 2});
+
+let coordinator = st.shard0;
+let participant1 = st.shard1;
+
+// Create a sharded collection with a chunk on each shard:
+// shard0: [-inf, 0)
+// shard1: [0, +inf)
+assert.commandWorked(st.s.adminCommand({enableSharding: kDbName}));
+assert.commandWorked(st.s.adminCommand({movePrimary: kDbName, to: coordinator.shardName}));
+assert.commandWorked(st.s.adminCommand({shardCollection: ns, key: {_id: 1}}));
+assert.commandWorked(st.s.adminCommand({split: ns, middle: {_id: 0}}));
+assert.commandWorked(
+    st.s.adminCommand({moveChunk: ns, find: {_id: 0}, to: participant1.shardName}));
+
+st.refreshCatalogCacheForNs(st.s, ns);
+
+function runTest(lsid) {
+    jsTest.log("Test that the correct error response is propagated upon losing in memory " +
+               "transaction metadata and durable metadata in the config.transactions collection " +
+               "after FCV change and failover with lsid: " + tojson(lsid));
+
+    // Upgrade fcv to make sure cluster is on the latestFCV before starting any transactions.
+    assert.commandWorked(st.s.adminCommand({setFeatureCompatibilityVersion: latestFCV}));
+
+    // Inserts are split to guarantee that shard0 will be chosen as the coordinator.
+    assert.commandWorked(st.s.getDB(kDbName).runCommand({
+        insert: kCollName,
+        documents: [{_id: -1 * docVal}],
+        lsid: lsid,
+        txnNumber: NumberLong(0),
+        stmtId: NumberInt(0),
+        startTransaction: true,
+        autocommit: false,
+    }));
+
+    assert.commandWorked(st.s.getDB(kDbName).runCommand({
+        insert: kCollName,
+        documents: [{_id: docVal}],
+        lsid: lsid,
+        txnNumber: NumberLong(0),
+        stmtId: NumberInt(1),
+        autocommit: false,
+    }));
+
+    assert.commandWorked(st.s.adminCommand(
+        {commitTransaction: 1, lsid: lsid, txnNumber: NumberLong(0), autocommit: false}));
+
+    // Downgrading should remove config.transactions entries for internal transactions.
+    assert.commandWorked(st.s.adminCommand({setFeatureCompatibilityVersion: lastLTSFCV}));
+
+    // Restarting should clear out in memory transaction metadata.
+    st.shard0.rs.stopSet(null /* signal */, true /* forRestart */);
+    st.shard0.rs.startSet({restart: true});
+
+    // For the following two statements, we are emulating the retryability behavior that mongos will
+    // display. Without config.transactions metadata, we shouldn't be able to get information on
+    // previous transactions so further commits should throw NoSuchTransaction. Since
+    // NoSuchTransaction is a TransientTransactionError, we would retry the transaction, which
+    // ultimately should fail with InternalTransactionNotSupported (since we've just downgraded),
+    // which the client should deem as a retryable write error.
+    assert.commandFailedWithCode(
+        st.s.adminCommand(
+            {commitTransaction: 1, lsid: lsid, txnNumber: NumberLong(0), autocommit: false}),
+        ErrorCodes.NoSuchTransaction);
+
+    let res = assert.commandFailedWithCode(st.s.getDB(kDbName).runCommand({
+        insert: kCollName,
+        documents: [{_id: -1 * docVal}],
+        lsid: lsid,
+        txnNumber: NumberLong(1),
+        stmtId: NumberInt(0),
+        startTransaction: true,
+        autocommit: false,
+    }),
+                                           ErrorCodes.InternalTransactionNotSupported);
+
+    assert(ErrorCodes.isRetriableError(res.code));
+
+    docVal++;
+}
+
+const testFuncs = [runTest];
+
+for (const testFunc of testFuncs) {
+    const lsidCombinations = [
+        {
+            id: UUID(),
+            txnUUID: UUID(),
+        },
+        {id: UUID(), txnUUID: UUID(), txnNumber: NumberLong(0)}
+    ];
+
+    for (const lsid of lsidCombinations) {
+        testFunc(lsid);
+    }
+}
+
+st.stop();
+})();