SERVER-57044 Add multiversion tests for retryable findAndModify

4 files changed, 331 insertions, 13 deletions

diff --git a/jstests/multiVersion/retry_find_and_modify_across_downgrade_fcv.js b/jstests/multiVersion/retry_find_and_modify_across_downgrade_fcv.js
new file mode 100644
index 00000000000..bbe82cfbdbc
--- /dev/null
+++ b/jstests/multiVersion/retry_find_and_modify_across_downgrade_fcv.js
@@ -0,0 +1,132 @@
+/**
+ * Tests that when storeFindAndModifyImagesInSideCollection=true, retrying a findAndModify after an
+ * FCV downgrade will return an error indicating that no write history was found for the
+ * transaction.
+ */
+
+(function() {
+    "use strict";
+
+    load("jstests/replsets/rslib.js");
+    load("jstests/libs/feature_compatibility_version.js");
+
+    const rst = new ReplSetTest(
+        {nodes: 1, nodeOptions: {setParameter: {storeFindAndModifyImagesInSideCollection: true}}});
+    rst.startSet();
+    rst.initiate();
+    const primary = rst.getPrimary();
+
+    let docId = 50;
+    let txnNumber = NumberLong(1);
+    const lsid = UUID();
+    const collName = 'image_collection';
+    const primaryAdminDB = primary.getDB('admin');
+    const primaryConfigDB = primary.getDB('config');
+
+    function incrementTxnNumber() {
+        txnNumber = NumberLong(txnNumber + 1);
+    }
+
+    function runCommandAndRetryInDowngradeFCV(cmd) {
+        // Upgrade the FCV since storeFindAndModifyImagesInSideCollection is only truly enabled
+        // when in FCV >= 4.0.
+        assert.commandWorked(
+            primaryAdminDB.adminCommand({setFeatureCompatibilityVersion: latestFCV}));
+        checkFCV(primaryAdminDB, latestFCV);
+        // Execute the findAndModify. An image entry should have been created.
+        assert.commandWorked(primary.getDB('test').runCommand(cmd));
+        assert(primaryConfigDB[collName].findOne({'_id.id': lsid, txnNum: txnNumber}));
+        // Downgrading the FCV will drop the 'config.image_collection' table.
+        assert.commandWorked(
+            primaryAdminDB.adminCommand({setFeatureCompatibilityVersion: lastStableFCV}));
+        checkFCV(primaryAdminDB, lastStableFCV);
+        assert.commandFailedWithCode(primary.getDB('test').runCommand(cmd),
+                                     ErrorCodes.IncompleteTransactionHistory);
+    }
+
+    incrementTxnNumber();
+
+    // Do an upsert to populate the data.
+    let cmd = {
+        findAndModify: 'user',
+        query: {_id: docId},
+        update: {$set: {x: 1}},
+        new: true,
+        upsert: true,
+        lsid: {id: lsid},
+        txnNumber: txnNumber,
+        writeConcern: {w: 1},
+    };
+    assert.commandWorked(primary.getDB('test').runCommand(cmd));
+
+    // Test in-place update (return pre-image).
+    incrementTxnNumber();
+    cmd = {
+        findAndModify: 'user',
+        query: {_id: docId},
+        update: {$inc: {x: 1}},
+        new: false,
+        upsert: false,
+        lsid: {id: lsid},
+        txnNumber: txnNumber,
+        writeConcern: {w: 1},
+    };
+    runCommandAndRetryInDowngradeFCV(cmd);
+
+    // Test in-place update (return post-image).
+    incrementTxnNumber();
+    cmd = {
+        findAndModify: 'user',
+        query: {_id: docId},
+        update: {$inc: {x: 1}},
+        new: true,
+        upsert: false,
+        lsid: {id: lsid},
+        txnNumber: txnNumber,
+        writeConcern: {w: 1},
+    };
+    runCommandAndRetryInDowngradeFCV(cmd);
+
+    // Test replacement update (return pre-image).
+    incrementTxnNumber();
+    cmd = {
+        findAndModify: 'user',
+        query: {_id: docId},
+        update: {y: 1},
+        new: false,
+        upsert: false,
+        lsid: {id: lsid},
+        txnNumber: txnNumber,
+        writeConcern: {w: 1},
+    };
+    runCommandAndRetryInDowngradeFCV(cmd);
+
+    // Test replacement update (return post-image).
+    incrementTxnNumber();
+    cmd = {
+        findAndModify: 'user',
+        query: {_id: docId},
+        update: {z: 1},
+        new: true,
+        upsert: false,
+        lsid: {id: lsid},
+        txnNumber: txnNumber,
+        writeConcern: {w: 1},
+    };
+    runCommandAndRetryInDowngradeFCV(cmd);
+
+    // Test remove (return pre-image).
+    incrementTxnNumber();
+    cmd = {
+        findAndModify: 'user',
+        query: {_id: docId},
+        remove: true,
+        new: false,
+        lsid: {id: lsid},
+        txnNumber: txnNumber,
+        writeConcern: {w: 1},
+    };
+    runCommandAndRetryInDowngradeFCV(cmd);
+
+    rst.stopSet();
+})();
\ No newline at end of file
diff --git a/jstests/multiVersion/store_find_and_modify_in_oplog_on_downgrade_fcv.js b/jstests/multiVersion/store_find_and_modify_in_oplog_on_downgrade_fcv.js
new file mode 100644
index 00000000000..224de2e76ca
--- /dev/null
+++ b/jstests/multiVersion/store_find_and_modify_in_oplog_on_downgrade_fcv.js
@@ -0,0 +1,171 @@
+/**
+ * Tests that retryable findAndModify will store images in the oplog while the server is in the
+ * downgraded FCV, even if storeFindAndModifyImagesInSideCollection=true.
+ */
+
+(function() {
+    "use strict";
+
+    load("jstests/replsets/rslib.js");
+    load("jstests/libs/feature_compatibility_version.js");
+
+    const rst = new ReplSetTest({nodes: 1});
+    rst.startSet();
+    rst.initiate();
+    const primary = rst.getPrimary();
+
+    let docId = 50;
+    let txnNumber = NumberLong(1);
+    const lsid = UUID();
+    const oplog = primary.getDB('local').oplog.rs;
+
+    const collName = 'image_collection';
+    const primaryAdminDB = primary.getDB('admin');
+    const primaryConfigDB = primary.getDB('config');
+
+    function incrementTxnNumber() {
+        txnNumber = NumberLong(txnNumber + 1);
+    }
+
+    function incrementDocId() {
+        docId += 10;
+    }
+
+    function checkImageStorageLocation(storeInOplog, expectedImage) {
+        if (storeInOplog) {
+            // The image only exists in the oplog and not the side collection.
+            assert(oplog.findOne({op: 'n', o: expectedImage, 'lsid.id': lsid}));
+            assert.eq(null, primaryConfigDB[collName].findOne({'_id.id': lsid, txnNum: txnNumber}));
+        } else {
+            // The image only exists in the side collection and not the oplog.
+            const imageDoc = primaryConfigDB[collName].findOne({'_id.id': lsid, txnNum: txnNumber});
+            assert.eq(expectedImage, imageDoc.image);
+            assert.eq(null, oplog.findOne({op: 'n', o: expectedImage, 'lsid.id': lsid}));
+        }
+    }
+
+    function runTest(storeInOplog) {
+        incrementDocId();
+        incrementTxnNumber();
+
+        // Do an upsert to populate the data.
+        let cmd = {
+            findAndModify: 'user',
+            query: {_id: docId},
+            update: {$set: {x: 1}},
+            new: true,
+            upsert: true,
+            lsid: {id: lsid},
+            txnNumber: txnNumber,
+            writeConcern: {w: 1},
+        };
+        assert.commandWorked(primary.getDB('test').runCommand(cmd));
+
+        // Test in-place update (return pre-image).
+        incrementTxnNumber();
+        cmd = {
+            findAndModify: 'user',
+            query: {_id: docId},
+            update: {$inc: {x: 1}},
+            new: false,
+            upsert: false,
+            lsid: {id: lsid},
+            txnNumber: txnNumber,
+            writeConcern: {w: 1},
+        };
+        let expectedPreImage = primary.getDB('test').user.findOne({_id: docId});
+        assert.commandWorked(primary.getDB('test').runCommand(cmd));
+        checkImageStorageLocation(storeInOplog, expectedPreImage);
+
+        // Test in-place update (return post-image).
+        incrementTxnNumber();
+        cmd = {
+            findAndModify: 'user',
+            query: {_id: docId},
+            update: {$inc: {x: 1}},
+            new: true,
+            upsert: false,
+            lsid: {id: lsid},
+            txnNumber: txnNumber,
+            writeConcern: {w: 1},
+        };
+        assert.commandWorked(primary.getDB('test').runCommand(cmd));
+        let expectedPostImage = primary.getDB('test').user.findOne({_id: docId});
+        checkImageStorageLocation(storeInOplog, expectedPostImage);
+
+        // Test replacement update (return pre-image).
+        incrementTxnNumber();
+        cmd = {
+            findAndModify: 'user',
+            query: {_id: docId},
+            update: {y: 1},
+            new: false,
+            upsert: false,
+            lsid: {id: lsid},
+            txnNumber: txnNumber,
+            writeConcern: {w: 1},
+        };
+        expectedPreImage = primary.getDB('test').user.findOne({_id: docId});
+        assert.commandWorked(primary.getDB('test').runCommand(cmd));
+        checkImageStorageLocation(storeInOplog, expectedPreImage);
+
+        // Test replacement update (return post-image).
+        incrementTxnNumber();
+        cmd = {
+            findAndModify: 'user',
+            query: {_id: docId},
+            update: {z: 1},
+            new: true,
+            upsert: false,
+            lsid: {id: lsid},
+            txnNumber: txnNumber,
+            writeConcern: {w: 1},
+        };
+        assert.commandWorked(primary.getDB('test').runCommand(cmd));
+        expectedPostImage = primary.getDB('test').user.findOne({_id: docId});
+        checkImageStorageLocation(storeInOplog, expectedPostImage);
+
+        // Test remove (return pre-image).
+        incrementTxnNumber();
+        cmd = {
+            findAndModify: 'user',
+            query: {_id: docId},
+            remove: true,
+            new: false,
+            lsid: {id: lsid},
+            txnNumber: txnNumber,
+            writeConcern: {w: 1},
+        };
+        expectedPreImage = primary.getDB('test').user.findOne({_id: docId});
+        assert.commandWorked(primary.getDB('test').runCommand(cmd));
+        checkImageStorageLocation(storeInOplog, expectedPreImage);
+    }
+
+    checkFCV(primaryAdminDB, latestFCV);
+    // By default, the parameter is set to false.
+    const result = assert.commandWorked(
+        primaryAdminDB.runCommand({getParameter: 1, storeFindAndModifyImagesInSideCollection: 1}));
+    assert.eq(false, result.storeFindAndModifyImagesInSideCollection, result);
+    // findAndModify writes store images in the oplog while the parameter is turned off.
+    runTest(true /* storeInOplog */);
+
+    // Setting the parameter to true will store images in the side collection rather than the oplog.
+    assert.commandWorked(primaryAdminDB.runCommand(
+        {setParameter: 1, storeFindAndModifyImagesInSideCollection: true}));
+    runTest(false /* storeInOplog */);
+
+    // Downgrading the FCV to 3.6 will only allow storing the image in the oplog, even if the
+    // parameter is set to true.
+    assert.commandWorked(
+        primaryAdminDB.adminCommand({setFeatureCompatibilityVersion: lastStableFCV}));
+    checkFCV(primaryAdminDB, lastStableFCV);
+    runTest(true /* storeInOplog */);
+
+    // Upgrading the FCV to 4.0 will allow us to start storing the image in the side collection
+    // again.
+    assert.commandWorked(primaryAdminDB.adminCommand({setFeatureCompatibilityVersion: latestFCV}));
+    checkFCV(primaryAdminDB, latestFCV);
+    runTest(false /* storeInOplog */);
+
+    rst.stopSet();
+})();
\ No newline at end of file
diff --git a/src/mongo/db/op_observer_impl.cpp b/src/mongo/db/op_observer_impl.cpp
index 58dc8f6f6cc..ac55ffe50d4 100644
--- a/src/mongo/db/op_observer_impl.cpp
+++ b/src/mongo/db/op_observer_impl.cpp
@@ -513,7 +513,7 @@ void OpObserverImpl::onUpdate(OperationContext* opCtx, const OplogUpdateEntryArg
         const auto storeImagesInSideCollection = storeFindAndModifyImagesInSideCollection.load() &&
             serverGlobalParams.featureCompatibility.isVersionInitialized() &&
             serverGlobalParams.featureCompatibility.getVersion() >=
-                ServerGlobalParams::FeatureCompatibility::Version::kUpgradingTo40;
+                ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo40;
         opTime = replLogUpdate(opCtx, session, args, storeImagesInSideCollection);
         if (storeImagesInSideCollection && opCtx->getTxnNumber() &&
             args.storeDocOption != OplogUpdateEntryArgs::StoreDocOption::None) {
@@ -588,7 +588,7 @@ void OpObserverImpl::onDelete(OperationContext* opCtx,
     } else {
         const auto storeImagesInSideCollection = storeFindAndModifyImagesInSideCollection.load() &&
             serverGlobalParams.featureCompatibility.getVersion() >=
-                ServerGlobalParams::FeatureCompatibility::Version::kUpgradingTo40;
+                ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo40;
         opTime = replLogDelete(opCtx,
                                nss,
                                uuid,
diff --git a/src/mongo/db/ops/write_ops_retryability.cpp b/src/mongo/db/ops/write_ops_retryability.cpp
index ade41c4e059..613da803d01 100644
--- a/src/mongo/db/ops/write_ops_retryability.cpp
+++ b/src/mongo/db/ops/write_ops_retryability.cpp
@@ -32,13 +32,13 @@
 
 #include "mongo/platform/basic.h"
 
-#include "mongo/db/ops/write_ops_retryability.h"
-
 #include "mongo/db/dbdirectclient.h"
 #include "mongo/db/namespace_string.h"
 #include "mongo/db/ops/find_and_modify_result.h"
+#include "mongo/db/ops/write_ops_retryability.h"
 #include "mongo/db/query/find_and_modify_request.h"
 #include "mongo/db/repl/image_collection_entry_gen.h"
+#include "mongo/db/server_options.h"
 #include "mongo/logger/redaction.h"
 #include "mongo/util/log.h"
 
@@ -132,15 +132,30 @@ BSONObj extractPreOrPostImage(OperationContext* opCtx, const repl::OplogEntry& o
         Timestamp ts = oplog.getTimestamp();
         const auto query = BSON("_id" << sessionIdBson);
         auto imageDoc = client.findOne(NamespaceString::kConfigImagesNamespace.ns(), query);
-        uassert(5637601,
-                str::stream()
-                    << "image collection no longer contains the complete write history of this "
-                       "transaction, record with sessionId "
-                    << sessionIdBson.toString()
-                    << " and txnNumber: "
-                    << txnNumber
-                    << " cannot be found",
-                !imageDoc.isEmpty());
+        if (serverGlobalParams.featureCompatibility.getVersion() <
+            ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo40) {
+            // We expect to have lost the transaction history since we drop the
+            // 'config.image_collection' table on downgrade FCV.
+            uassert(ErrorCodes::IncompleteTransactionHistory,
+                    str::stream() << "Incomplete transaction history for sessionId: "
+                                  << sessionIdBson
+                                  << " txnNumber: "
+                                  << txnNumber,
+                    !imageDoc.isEmpty());
+        } else {
+            // Other than a real code bug, it is possible to be missing expected transaction
+            // history if the server has gone through an FCV downgrade and then a subsequent
+            // FCV upgrade before retrying.
+            uassert(5637601,
+                    str::stream()
+                        << "image collection no longer contains the complete write history of this "
+                           "transaction, record with sessionId "
+                        << sessionIdBson.toString()
+                        << " and txnNumber: "
+                        << txnNumber
+                        << " cannot be found",
+                    !imageDoc.isEmpty());
+        }
 
         auto entry =
             repl::ImageEntry::parse(IDLParserErrorContext("ImageEntryForRequest"), imageDoc);