SERVER-56668 Create and drop the 'config.image_collection' table on FCV upgrade/downgrade

5 files changed, 119 insertions, 26 deletions

diff --git a/jstests/multiVersion/create_and_drop_image_collection_on_fcv_upgrade_downgrade.js b/jstests/multiVersion/create_and_drop_image_collection_on_fcv_upgrade_downgrade.js
new file mode 100644
index 00000000000..316f6c1c315
--- /dev/null
+++ b/jstests/multiVersion/create_and_drop_image_collection_on_fcv_upgrade_downgrade.js
@@ -0,0 +1,54 @@
+/**
+ * Tests that upgrading and downgrading the FCV will create and drop the 'config.image_collection'
+ * table respectively.
+ */
+
+(function() {
+    "use strict";
+
+    load("jstests/replsets/rslib.js");
+    load("jstests/libs/feature_compatibility_version.js");
+
+    const rst = new ReplSetTest({nodes: [{}, {rsConfig: {priority: 0}}]});
+    rst.startSet();
+    rst.initiate();
+    const primary = rst.getPrimary();
+    const secondary = rst.getSecondary();
+
+    const collName = 'image_collection';
+    const primaryAdminDB = primary.getDB('admin');
+    const secondaryAdminDB = secondary.getDB('admin');
+    const primaryConfigDB = primary.getDB('config');
+    const secondaryConfigDB = secondary.getDB('config');
+    // The collection exists on startup in the latest FCV.
+    checkFCV(primaryAdminDB, latestFCV);
+    checkFCV(secondaryAdminDB, latestFCV);
+    assert(primaryConfigDB[collName].exists());
+    assert(secondaryConfigDB[collName].exists());
+
+    assert.commandWorked(
+        primaryAdminDB.adminCommand({setFeatureCompatibilityVersion: lastStableFCV}));
+    checkFCV(primaryAdminDB, lastStableFCV);
+    checkFCV(secondaryAdminDB, lastStableFCV);
+    assert(!primaryConfigDB[collName].exists());
+    assert(!secondaryConfigDB[collName].exists());
+
+    // Restart the primary to make sure the collection is not created on startup while in the
+    // downgraded FCV.
+    rst.restart(primary);
+    rst.getPrimary();
+    reconnect(primary);
+    reconnect(secondary);
+
+    checkFCV(primaryAdminDB, lastStableFCV);
+    checkFCV(secondaryAdminDB, lastStableFCV);
+    assert(!primaryConfigDB[collName].exists());
+    assert(!secondaryConfigDB[collName].exists());
+
+    assert.commandWorked(primaryAdminDB.adminCommand({setFeatureCompatibilityVersion: latestFCV}));
+    checkFCV(primaryAdminDB, latestFCV);
+    checkFCV(secondaryAdminDB, latestFCV);
+    assert(primaryConfigDB[collName].exists());
+    assert(secondaryConfigDB[collName].exists());
+    rst.stopSet();
+})();
\ No newline at end of file
diff --git a/jstests/noPassthrough/store_retryable_find_and_modify_images_in_side_collection.js b/jstests/noPassthrough/store_retryable_find_and_modify_images_in_side_collection.js
index b1bb008c285..6b5d355c053 100644
--- a/jstests/noPassthrough/store_retryable_find_and_modify_images_in_side_collection.js
+++ b/jstests/noPassthrough/store_retryable_find_and_modify_images_in_side_collection.js
@@ -1,6 +1,8 @@
 /**
  * Test that retryable findAndModify commands will store pre- and post- images in the appropriate
  * collections according to the parameter value of `storeFindAndModifyImagesInSideCollection`.
+ *
+ * @tags: [requires_replication]
  */
 (function() {
     "use strict";
@@ -284,13 +286,7 @@
     const lsid = UUID();
     // Test that retryable findAndModifys will store pre- and post- images in the oplog when
     // 'storeFindAndModifyImagesInSideCollection'=false.
-    // Initiate the replica set with 'storeFindAndModifyImagesInSideCollection'=true to guarantee
-    // that the 'config.image_collection' exists. This is needed until we commit to a contract
-    // between the lifetime of this side collection, the parameter value, and FCV.
-    const rst = new ReplSetTest({
-        nodes: numNodes,
-        nodeOptions: {setParameter: {storeFindAndModifyImagesInSideCollection: true}}
-    });
+    const rst = new ReplSetTest({nodes: numNodes});
     rst.startSet();
     rst.initiate();
     runTests(lsid, rst.getPrimary(), rst.getPrimary(), rst.getSecondary(), true, 40);
@@ -298,12 +294,15 @@
     rst.stopSet();
     // Test that retryable findAndModifys will store pre- and post- images in the
     // 'config.image_collection' table.
-    // TODO: Uncomment this block to get code coverage for sharded clusters. Shard servers will
-    // start with downgraded FCV, meaning the 'image_collection' won't be available on startup.
-    // const st = new ShardingTest({shards: {rs0: {nodes: numNodes, setParameter:
-    // {storeFindAndModifyImagesInSideCollection: true}}}});
-    // runTests(lsid, st.s, st.rs0.getPrimary(), st.rs0.getSecondary(), true, 60);
-    // runTests(lsid, st.s, st.rs0.getPrimary(), st.rs0.getSecondary(), true, 70);
-    // st.stop();
-    // runTests(true);
+    const st = new ShardingTest({
+        shards: {
+            rs0: {
+                nodes: numNodes,
+                setParameter: {storeFindAndModifyImagesInSideCollection: true}
+            }
+        }
+    });
+    runTests(lsid, st.s, st.rs0.getPrimary(), st.rs0.getSecondary(), true, 60);
+    runTests(lsid, st.s, st.rs0.getPrimary(), st.rs0.getSecondary(), true, 70);
+    st.stop();
 })();
diff --git a/jstests/replsets/initial_sync_test_fixture_test.js b/jstests/replsets/initial_sync_test_fixture_test.js
index 5c0df7cb3bf..d52ea5d3417 100644
--- a/jstests/replsets/initial_sync_test_fixture_test.js
+++ b/jstests/replsets/initial_sync_test_fixture_test.js
@@ -165,14 +165,16 @@
     assert(!initialSyncTest.step());
     checkLogForOplogApplicationMsg(secondary, 9);
     assert(!initialSyncTest.step());
-    checkLogForOplogApplicationMsg(secondary, 1);
+    // Stepping up in FCV4.0 creates the `config.image_collection` table which adds to the batch
+    // size.
+    checkLogForOplogApplicationMsg(secondary, 3);
 
     assert(initialSyncTest.step(), "Expected initial sync to have completed, but it did not");
 
     // Confirm that node can be read from and that it has the inserts that were made while the node
     // was in initial sync.
-    assert.eq(secondary.getDB("test").foo.find().count(), 6);
-    assert.eq(secondary.getDB("test").bar.find().count(), 6);
+    assert.eq(secondary.getDB("test").foo.find().count(), 7);
+    assert.eq(secondary.getDB("test").bar.find().count(), 7);
 
     // Do data consistency checks at the end.
     initialSyncTest.stop();
diff --git a/src/mongo/db/commands/set_feature_compatibility_version_command.cpp b/src/mongo/db/commands/set_feature_compatibility_version_command.cpp
index 64537333761..a74b9374089 100644
--- a/src/mongo/db/commands/set_feature_compatibility_version_command.cpp
+++ b/src/mongo/db/commands/set_feature_compatibility_version_command.cpp
@@ -43,6 +43,7 @@
 #include "mongo/db/commands/feature_compatibility_version_parser.h"
 #include "mongo/db/concurrency/d_concurrency.h"
 #include "mongo/db/db_raii.h"
+#include "mongo/db/dbdirectclient.h"
 #include "mongo/db/logical_clock.h"
 #include "mongo/db/repl/repl_client_info.h"
 #include "mongo/db/s/config/sharding_catalog_manager.h"
@@ -61,6 +62,40 @@ namespace mongo {
 
 namespace {
 
+void dropImageCollectionOnDowngrade(OperationContext* opCtx) {
+    DBDirectClient client(opCtx);
+
+    BSONObj result;
+    client.dropCollection(
+        NamespaceString::kConfigImagesNamespace.ns(), WriteConcernOptions(), &result);
+    const auto status = getStatusFromCommandResult(result);
+    if (status != ErrorCodes::NamespaceNotFound) {
+        uassertStatusOKWithContext(status,
+                                   str::stream() << "Failed to drop the "
+                                                 << NamespaceString::kConfigImagesNamespace.ns()
+                                                 << " collection");
+    }
+}
+
+void createImageCollectionOnUpgrade(OperationContext* opCtx) {
+    DBDirectClient client(opCtx);
+
+    const size_t initialExtentSize = 0;
+    const bool capped = false;
+    const bool maxSize = 0;
+
+    BSONObj result;
+    client.createCollection(
+        NamespaceString::kConfigImagesNamespace.ns(), initialExtentSize, capped, maxSize, &result);
+    const auto status = getStatusFromCommandResult(result);
+    if (status != ErrorCodes::NamespaceExists) {
+        uassertStatusOKWithContext(status,
+                                   str::stream() << "Failed to create the "
+                                                 << NamespaceString::kConfigImagesNamespace.ns()
+                                                 << " collection");
+    }
+}
+
 MONGO_FAIL_POINT_DEFINE(featureCompatibilityDowngrade);
 MONGO_FAIL_POINT_DEFINE(featureCompatibilityUpgrade);
 MONGO_FAIL_POINT_DEFINE(hangWhileUpgrading);
@@ -179,6 +214,7 @@ public:
             }
 
             updateUniqueIndexesOnUpgrade(opCtx);
+            createImageCollectionOnUpgrade(opCtx);
 
             // Upgrade shards before config finishes its upgrade.
             if (serverGlobalParams.clusterRole == ClusterRole::ConfigServer) {
@@ -267,6 +303,8 @@ public:
                 Lock::GlobalLock lk(opCtx, MODE_S);
             }
 
+            dropImageCollectionOnDowngrade(opCtx);
+
             // Downgrade shards before config finishes its downgrade.
             if (serverGlobalParams.clusterRole == ClusterRole::ConfigServer) {
                 uassertStatusOK(
diff --git a/src/mongo/db/session_catalog.cpp b/src/mongo/db/session_catalog.cpp
index c9a6e615131..21b52a2af6f 100644
--- a/src/mongo/db/session_catalog.cpp
+++ b/src/mongo/db/session_catalog.cpp
@@ -38,7 +38,6 @@
 
 #include "mongo/db/db_raii.h"
 #include "mongo/db/dbdirectclient.h"
-#include "mongo/db/dbdirectclient.h"
 #include "mongo/db/kill_sessions_common.h"
 #include "mongo/db/namespace_string.h"
 #include "mongo/db/op_observer.h"
@@ -208,9 +207,8 @@ void SessionCatalog::onStepUp(OperationContext* opCtx) {
 
     BSONObj result;
 
-    if (storeFindAndModifyImagesInSideCollection.load() &&
-        serverGlobalParams.featureCompatibility.getVersion() >=
-            ServerGlobalParams::FeatureCompatibility::Version::kUpgradingTo40) {
+    if (serverGlobalParams.featureCompatibility.getVersion() >=
+        ServerGlobalParams::FeatureCompatibility::Version::kUpgradingTo40) {
         BSONObj imageResult;
         client.createCollection(NamespaceString::kConfigImagesNamespace.ns(),
                                 initialExtentSize,
@@ -218,10 +216,12 @@ void SessionCatalog::onStepUp(OperationContext* opCtx) {
                                 maxSize,
                                 &imageResult);
         const auto status = getStatusFromCommandResult(imageResult);
-        uassertStatusOKWithContext(status,
-                                   str::stream() << "Failed to create the "
-                                                 << NamespaceString::kConfigImagesNamespace.ns()
-                                                 << " collection");
+        if (status != ErrorCodes::NamespaceExists) {
+            uassertStatusOKWithContext(status,
+                                       str::stream() << "Failed to create the "
+                                                     << NamespaceString::kConfigImagesNamespace.ns()
+                                                     << " collection");
+        }
     }
 
     if (client.createCollection(NamespaceString::kSessionTransactionsTableNamespace.ns(),