SERVER-25417 harden checks around using --shardSvr, --overrideShardIdentity, and queryableBackupMode

4 files changed, 118 insertions, 80 deletions

diff --git a/src/mongo/db/s/collection_sharding_state.cpp b/src/mongo/db/s/collection_sharding_state.cpp
index 84752d81e96..01fc23bd9b1 100644
--- a/src/mongo/db/s/collection_sharding_state.cpp
+++ b/src/mongo/db/s/collection_sharding_state.cpp
@@ -67,8 +67,7 @@ public:
 
     void commit() override {
         fassertNoTrace(40071,
-                       ShardingState::get(_txn)->initializeFromShardIdentity(
-                           _txn, _shardIdentity, Date_t::max()));
+                       ShardingState::get(_txn)->initializeFromShardIdentity(_txn, _shardIdentity));
     }
 
     void rollback() override {}
diff --git a/src/mongo/db/s/sharding_state.cpp b/src/mongo/db/s/sharding_state.cpp
index bfba2236515..04097e28e84 100644
--- a/src/mongo/db/s/sharding_state.cpp
+++ b/src/mongo/db/s/sharding_state.cpp
@@ -364,48 +364,11 @@ void ShardingState::initializeFromConfigConnString(OperationContext* txn, const
     uassertStatusOK(updateConfigServerOpTimeFromMetadata(txn));
 }
 
-Status ShardingState::initializeFromShardIdentity(OperationContext* txn) {
-    invariant(!txn->lockState()->isLocked());
-
-    if (serverGlobalParams.clusterRole != ClusterRole::ShardServer) {
-        return Status::OK();
-    }
-
-    BSONObj shardIdentityBSON;
-    try {
-        AutoGetCollection autoColl(txn, NamespaceString::kConfigCollectionNamespace, MODE_IS);
-        if (!Helpers::findOne(txn,
-                              autoColl.getCollection(),
-                              BSON("_id"
-                                   << "shardIdentity"),
-                              shardIdentityBSON)) {
-            return Status::OK();
-        }
-    } catch (const DBException& ex) {
-        return ex.toStatus();
-    }
-
-    auto parseStatus = ShardIdentityType::fromBSON(shardIdentityBSON);
-    if (!parseStatus.isOK()) {
-        return parseStatus.getStatus();
-    }
-
-    auto status = initializeFromShardIdentity(txn, parseStatus.getValue(), txn->getDeadline());
-    if (!status.isOK()) {
-        return status;
-    }
-
-    return reloadShardRegistryUntilSuccess(txn);
-}
-
 // NOTE: This method can be called inside a database lock so it should never take any database
 // locks, perform I/O, or any long running operations.
 Status ShardingState::initializeFromShardIdentity(OperationContext* txn,
-                                                  const ShardIdentityType& shardIdentity,
-                                                  Date_t deadline) {
-    if (serverGlobalParams.clusterRole != ClusterRole::ShardServer) {
-        return Status::OK();
-    }
+                                                  const ShardIdentityType& shardIdentity) {
+    invariant(serverGlobalParams.clusterRole == ClusterRole::ShardServer);
 
     Status validationStatus = shardIdentity.validate();
     if (!validationStatus.isOK()) {
@@ -422,10 +385,10 @@ Status ShardingState::initializeFromShardIdentity(OperationContext* txn,
 
     // TODO: remove after v3.4.
     // This is for backwards compatibility with old style initialization through metadata
-    // commands/setShardVersion. As well as all assignments to _initializationStatus and
-    // _setInitializationState_inlock in this method.
+    // commands/setShardVersion, which can happen concurrently with an insert of a
+    // shardIdentity document to admin.system.version.
     if (_getInitializationState() == InitializationState::kInitializing) {
-        auto waitStatus = _waitForInitialization_inlock(deadline, lk);
+        auto waitStatus = _waitForInitialization_inlock(Date_t::max(), lk);
         if (!waitStatus.isOK()) {
             return waitStatus;
         }
@@ -440,6 +403,9 @@ Status ShardingState::initializeFromShardIdentity(OperationContext* txn,
 
     auto configSvrConnStr = shardIdentity.getConfigsvrConnString();
 
+    // TODO: remove after v3.4.
+    // This is for backwards compatibility with old style initialization through metadata
+    // commands/setShardVersion, which sets the shardName and configsvrConnectionString.
     if (_getInitializationState() == InitializationState::kInitialized) {
         if (_shardName != shardIdentity.getShardName()) {
             return {ErrorCodes::InconsistentShardIdentity,
@@ -464,7 +430,7 @@ Status ShardingState::initializeFromShardIdentity(OperationContext* txn,
                                   << configSvrConnStr.toString()};
         }
 
-        // clusterId will only be unset if sharding state was initialized via the sharding
+        // The clusterId will only be unset if sharding state was initialized via the sharding
         // metadata commands.
         if (!_clusterId.isSet()) {
             _clusterId = shardIdentity.getClusterId();
@@ -484,7 +450,10 @@ Status ShardingState::initializeFromShardIdentity(OperationContext* txn,
         try {
             Status status = _globalInit(txn, configSvrConnStr, generateDistLockProcessId(txn));
 
-            // For backwards compatibility with old style inits from metadata commands.
+            // TODO: remove after v3.4.
+            // This is for backwards compatibility with old style initialization through metadata
+            // commands/setShardVersion, which can happen concurrently with an insert of a
+            // shardIdentity document to admin.system.version.
             if (status.isOK()) {
                 _setInitializationState_inlock(InitializationState::kInitialized);
                 ReplicaSetMonitor::setSynchronousConfigChangeHook(
@@ -603,6 +572,94 @@ void ShardingState::_signalInitializationComplete(Status status) {
     _initializationFinishedCondition.notify_all();
 }
 
+Status ShardingState::initializeShardingAwarenessIfNeeded(OperationContext* txn) {
+    // In sharded readOnly mode, we ignore the shardIdentity document on disk and instead *require*
+    // a shardIdentity document to be passed through --overrideShardIdentity.
+    if (storageGlobalParams.readOnly) {
+        if (serverGlobalParams.clusterRole == ClusterRole::ShardServer) {
+            if (serverGlobalParams.overrideShardIdentity.isEmpty()) {
+                return {ErrorCodes::InvalidOptions,
+                        "If started with --shardsvr in queryableBackupMode, a shardIdentity "
+                        "document must be provided through --overrideShardIdentity"};
+            }
+            auto swOverrideShardIdentity =
+                ShardIdentityType::fromBSON(serverGlobalParams.overrideShardIdentity);
+            if (!swOverrideShardIdentity.isOK()) {
+                return swOverrideShardIdentity.getStatus();
+            }
+            auto status = initializeFromShardIdentity(txn, swOverrideShardIdentity.getValue());
+            if (!status.isOK()) {
+                return status;
+            }
+            return reloadShardRegistryUntilSuccess(txn);
+        } else {
+            // Warn if --overrideShardIdentity is used but *not* started with --shardsvr.
+            if (!serverGlobalParams.overrideShardIdentity.isEmpty()) {
+                warning() << "Not started with --shardsvr, but a shardIdentity document was "
+                             "provided through --overrideShardIdentity: "
+                          << serverGlobalParams.overrideShardIdentity;
+            }
+            return Status::OK();
+        }
+    }
+    // In sharded *non*-readOnly mode, error if --overrideShardIdentity is provided. Use the
+    // shardIdentity document on disk if one exists, but it is okay if no shardIdentity document is
+    // provided at all (sharding awareness will be initialized when a shardIdentity document is
+    // inserted).
+    else {
+        if (!serverGlobalParams.overrideShardIdentity.isEmpty()) {
+            return {ErrorCodes::InvalidOptions,
+                    str::stream() << "--overrideShardIdentity is only allowed in sharded "
+                                     "queryableBackupMode. If not in queryableBackupMode, edit the "
+                                     "shardIdentity document by starting the server *without* "
+                                     "--shardsvr, manually updating the shardIdentity document in "
+                                     "the "
+                                  << NamespaceString::kConfigCollectionNamespace.toString()
+                                  << " collection, and restarting the server with --shardsvr."};
+        }
+
+        // Load the shardIdentity document from disk.
+        invariant(!txn->lockState()->isLocked());
+        BSONObj shardIdentityBSON;
+        try {
+            AutoGetCollection autoColl(txn, NamespaceString::kConfigCollectionNamespace, MODE_IS);
+            Helpers::findOne(txn,
+                             autoColl.getCollection(),
+                             BSON("_id" << ShardIdentityType::IdName),
+                             shardIdentityBSON);
+        } catch (const DBException& ex) {
+            return ex.toStatus();
+        }
+
+        if (serverGlobalParams.clusterRole == ClusterRole::ShardServer) {
+            if (shardIdentityBSON.isEmpty()) {
+                warning() << "Started with --shardsvr, but no shardIdentity document was found on "
+                             "disk in "
+                          << NamespaceString::kConfigCollectionNamespace;
+                return Status::OK();
+            }
+            auto swShardIdentity = ShardIdentityType::fromBSON(shardIdentityBSON);
+            if (!swShardIdentity.isOK()) {
+                return swShardIdentity.getStatus();
+            }
+            auto status = initializeFromShardIdentity(txn, swShardIdentity.getValue());
+            if (!status.isOK()) {
+                return status;
+            }
+            return reloadShardRegistryUntilSuccess(txn);
+        } else {
+            // Warn if a shardIdentity document is found on disk but *not* started with --shardsvr.
+            if (!shardIdentityBSON.isEmpty()) {
+                warning() << "Not started with --shardsvr, but a shardIdentity document was found "
+                             "on disk in "
+                          << NamespaceString::kConfigCollectionNamespace << ": "
+                          << shardIdentityBSON;
+            }
+            return Status::OK();
+        }
+    }
+}
+
 StatusWith<ChunkVersion> ShardingState::_refreshMetadata(
     OperationContext* txn, const NamespaceString& nss, const CollectionMetadata* metadataForDiff) {
     invariant(!txn->lockState()->isLocked());
diff --git a/src/mongo/db/s/sharding_state.h b/src/mongo/db/s/sharding_state.h
index a27b2841eda..209d8754a34 100644
--- a/src/mongo/db/s/sharding_state.h
+++ b/src/mongo/db/s/sharding_state.h
@@ -118,25 +118,10 @@ public:
     void initializeFromConfigConnString(OperationContext* txn, const std::string& configSvr);
 
     /**
-     * Initializes the sharding state of this server from the shard identity document from local
-     * storage.
-     *
-     * Note that this will also try to connect to the config servers and will block until it
-     * succeeds.
-     */
-    Status initializeFromShardIdentity(OperationContext* txn);
-
-    /**
      * Initializes the sharding state of this server from the shard identity document argument.
-     * This is the more genaralized form of the initializeFromShardIdentity(OperationContext*)
-     * method that can accept the shard identity from any source. Note that shardIdentity must
-     * be valid.
-     *
-     * Returns ErrorCodes::ExceededTimeLimit if deadline has passed.
      */
     Status initializeFromShardIdentity(OperationContext* txn,
-                                       const ShardIdentityType& shardIdentity,
-                                       Date_t deadline);
+                                       const ShardIdentityType& shardIdentity);
 
     /**
      * Shuts down sharding machinery on the shard.
@@ -266,6 +251,8 @@ public:
      */
     void setScheduleCleanupFunctionForTest(RangeDeleterCleanupNotificationFunc fn);
 
+    Status initializeShardingAwarenessIfNeeded(OperationContext* txn);
+
 private:
     friend class ScopedRegisterMigration;
 
diff --git a/src/mongo/db/s/sharding_state_test.cpp b/src/mongo/db/s/sharding_state_test.cpp
index 7320dd06c94..43ed8b5c8c7 100644
--- a/src/mongo/db/s/sharding_state_test.cpp
+++ b/src/mongo/db/s/sharding_state_test.cpp
@@ -173,7 +173,7 @@ TEST_F(ShardingStateTest, ValidShardIdentitySucceeds) {
     shardIdentity.setShardName("a");
     shardIdentity.setClusterId(OID::gen());
 
-    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity, Date_t::max()));
+    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity));
     ASSERT_TRUE(shardingState()->enabled());
     ASSERT_EQ("a", shardingState()->getShardName());
     ASSERT_EQ("config/a:1,b:2", shardingState()->getConfigServer(txn()).toString());
@@ -192,8 +192,7 @@ TEST_F(ShardingStateTest, InitWhilePreviouslyInErrorStateWillStayInErrorState) {
         });
 
     {
-        auto status =
-            shardingState()->initializeFromShardIdentity(txn(), shardIdentity, Date_t::max());
+        auto status = shardingState()->initializeFromShardIdentity(txn(), shardIdentity);
         ASSERT_EQ(ErrorCodes::ShutdownInProgress, status);
     }
 
@@ -205,8 +204,7 @@ TEST_F(ShardingStateTest, InitWhilePreviouslyInErrorStateWillStayInErrorState) {
         });
 
     {
-        auto status =
-            shardingState()->initializeFromShardIdentity(txn(), shardIdentity, Date_t::max());
+        auto status = shardingState()->initializeFromShardIdentity(txn(), shardIdentity);
         ASSERT_EQ(ErrorCodes::ManualInterventionRequired, status);
     }
 
@@ -221,7 +219,7 @@ TEST_F(ShardingStateTest, InitializeAgainWithMatchingShardIdentitySucceeds) {
     shardIdentity.setShardName("a");
     shardIdentity.setClusterId(clusterID);
 
-    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity, Date_t::max()));
+    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity));
 
     ShardIdentityType shardIdentity2;
     shardIdentity2.setConfigsvrConnString(
@@ -234,7 +232,7 @@ TEST_F(ShardingStateTest, InitializeAgainWithMatchingShardIdentitySucceeds) {
             return Status{ErrorCodes::InternalError, "should not reach here"};
         });
 
-    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity2, Date_t::max()));
+    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity2));
 
     ASSERT_TRUE(shardingState()->enabled());
     ASSERT_EQ("a", shardingState()->getShardName());
@@ -249,7 +247,7 @@ TEST_F(ShardingStateTest, InitializeAgainWithSameReplSetNameSucceeds) {
     shardIdentity.setShardName("a");
     shardIdentity.setClusterId(clusterID);
 
-    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity, Date_t::max()));
+    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity));
 
     ShardIdentityType shardIdentity2;
     shardIdentity2.setConfigsvrConnString(
@@ -262,7 +260,7 @@ TEST_F(ShardingStateTest, InitializeAgainWithSameReplSetNameSucceeds) {
             return Status{ErrorCodes::InternalError, "should not reach here"};
         });
 
-    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity2, Date_t::max()));
+    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity2));
 
     ASSERT_TRUE(shardingState()->enabled());
     ASSERT_EQ("a", shardingState()->getShardName());
@@ -277,7 +275,7 @@ TEST_F(ShardingStateTest, InitializeAgainWithDifferentReplSetNameFails) {
     shardIdentity.setShardName("a");
     shardIdentity.setClusterId(clusterID);
 
-    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity, Date_t::max()));
+    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity));
 
     ShardIdentityType shardIdentity2;
     shardIdentity2.setConfigsvrConnString(
@@ -290,8 +288,7 @@ TEST_F(ShardingStateTest, InitializeAgainWithDifferentReplSetNameFails) {
             return Status{ErrorCodes::InternalError, "should not reach here"};
         });
 
-    auto status =
-        shardingState()->initializeFromShardIdentity(txn(), shardIdentity2, Date_t::max());
+    auto status = shardingState()->initializeFromShardIdentity(txn(), shardIdentity2);
     ASSERT_EQ(ErrorCodes::InconsistentShardIdentity, status);
 
     ASSERT_TRUE(shardingState()->enabled());
@@ -307,7 +304,7 @@ TEST_F(ShardingStateTest, InitializeAgainWithDifferentShardNameFails) {
     shardIdentity.setShardName("a");
     shardIdentity.setClusterId(clusterID);
 
-    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity, Date_t::max()));
+    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity));
 
     ShardIdentityType shardIdentity2;
     shardIdentity2.setConfigsvrConnString(
@@ -320,8 +317,7 @@ TEST_F(ShardingStateTest, InitializeAgainWithDifferentShardNameFails) {
             return Status{ErrorCodes::InternalError, "should not reach here"};
         });
 
-    auto status =
-        shardingState()->initializeFromShardIdentity(txn(), shardIdentity2, Date_t::max());
+    auto status = shardingState()->initializeFromShardIdentity(txn(), shardIdentity2);
     ASSERT_EQ(ErrorCodes::InconsistentShardIdentity, status);
 
     ASSERT_TRUE(shardingState()->enabled());
@@ -336,7 +332,7 @@ TEST_F(ShardingStateTest, InitializeAgainWithDifferentClusterIdFails) {
     shardIdentity.setShardName("a");
     shardIdentity.setClusterId(OID::gen());
 
-    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity, Date_t::max()));
+    ASSERT_OK(shardingState()->initializeFromShardIdentity(txn(), shardIdentity));
 
     ShardIdentityType shardIdentity2;
     shardIdentity2.setConfigsvrConnString(
@@ -349,8 +345,7 @@ TEST_F(ShardingStateTest, InitializeAgainWithDifferentClusterIdFails) {
             return Status{ErrorCodes::InternalError, "should not reach here"};
         });
 
-    auto status =
-        shardingState()->initializeFromShardIdentity(txn(), shardIdentity2, Date_t::max());
+    auto status = shardingState()->initializeFromShardIdentity(txn(), shardIdentity2);
     ASSERT_EQ(ErrorCodes::InconsistentShardIdentity, status);
 
     ASSERT_TRUE(shardingState()->enabled());