SERVER-48411 Increase maximum allowed replica set memberId on disk from 255 to INT_MAX.

(cherry picked from commit 52db264df93fe1f5f75647136793bd025c0393a8)

7 files changed, 131 insertions, 9 deletions

diff --git a/src/mongo/db/repl/member_config.h b/src/mongo/db/repl/member_config.h
index d6bb75622f3..39fbc30750b 100644
--- a/src/mongo/db/repl/member_config.h
+++ b/src/mongo/db/repl/member_config.h
@@ -76,6 +76,14 @@ public:
     static const std::string kConfigVoterTagName;
 
     /**
+     * Inline `kMaxUserMemberId` to allow others use
+     * the constant without linking to `member_config.cpp`.
+     *
+     * Maximum allowed member id for new nodes added by user.
+     */
+    inline static const int kMaxUserMemberId = 255;
+
+    /**
      * Construct a MemberConfig from the contents of "mcfg".
      *
      * If "mcfg" describes any tags, builds ReplSetTags for this
diff --git a/src/mongo/db/repl/member_config_test.cpp b/src/mongo/db/repl/member_config_test.cpp
index 04037567886..cafa6d85693 100644
--- a/src/mongo/db/repl/member_config_test.cpp
+++ b/src/mongo/db/repl/member_config_test.cpp
@@ -78,19 +78,26 @@ TEST(MemberConfig, ParseFailsWithMissingIdField) {
                        40414);
 }
 
-TEST(MemberConfig, ParseFailsWithIdOutOfRange) {
+TEST(MemberConfig, ParseMemberConfigId) {
     ReplSetTagConfig tagConfig;
     {
+        // Fail case
         ASSERT_THROWS(MemberConfig(BSON("_id" << -1 << "host"
                                               << "localhost:12345"),
                                    &tagConfig),
                       ExceptionFor<ErrorCodes::BadValue>);
     }
     {
-        ASSERT_THROWS(MemberConfig(BSON("_id" << 256 << "host"
-                                              << "localhost:12345"),
-                                   &tagConfig),
-                      ExceptionFor<ErrorCodes::BadValue>);
+        // Pass cases
+        MemberConfig(BSON("_id" << 0 << "host"
+                                << "localhost:12345"),
+                     &tagConfig);
+        MemberConfig(BSON("_id" << MemberConfig::kMaxUserMemberId << "host"
+                                << "localhost:12345"),
+                     &tagConfig);
+        MemberConfig(BSON("_id" << (MemberConfig::kMaxUserMemberId + 1) << "host"
+                                << "localhost:12345"),
+                     &tagConfig);
     }
 }
 
diff --git a/src/mongo/db/repl/member_id.h b/src/mongo/db/repl/member_id.h
index e6e937c81dc..10d2053c2d9 100644
--- a/src/mongo/db/repl/member_id.h
+++ b/src/mongo/db/repl/member_id.h
@@ -46,9 +46,10 @@ public:
     MemberId() : _id(kUninitializedMemberId) {}
 
     explicit MemberId(int id) {
-        if (id < 0 || id > 255) {
+        if (id < 0) {
             uasserted(ErrorCodes::BadValue,
-                      str::stream() << "_id field value of " << id << " is out of range.");
+                      str::stream()
+                          << "_id field value of " << id << " can't be a negative number");
         }
         _id = id;
     }
diff --git a/src/mongo/db/repl/repl_set_config.cpp b/src/mongo/db/repl/repl_set_config.cpp
index a0df57d6315..c768cedd634 100644
--- a/src/mongo/db/repl/repl_set_config.cpp
+++ b/src/mongo/db/repl/repl_set_config.cpp
@@ -454,7 +454,7 @@ int ReplSetConfig::findMemberIndexByHostAndPort(const HostAndPort& hap) const {
     return -1;
 }
 
-int ReplSetConfig::findMemberIndexByConfigId(long long configId) const {
+int ReplSetConfig::findMemberIndexByConfigId(int configId) const {
     int x = 0;
     for (const auto& member : getMembers()) {
         if (member.getId() == MemberId(configId)) {
diff --git a/src/mongo/db/repl/repl_set_config.h b/src/mongo/db/repl/repl_set_config.h
index fe69f96d5d4..665b3dbd03e 100644
--- a/src/mongo/db/repl/repl_set_config.h
+++ b/src/mongo/db/repl/repl_set_config.h
@@ -323,7 +323,7 @@ public:
      * Returns a MemberConfig index position corresponding to the member with the given
      * _id in the config, or -1 if there is no member with that address.
      */
-    int findMemberIndexByConfigId(long long configId) const;
+    int findMemberIndexByConfigId(int configId) const;
 
     /**
      * Gets the default write concern for the replica set described by this configuration.
diff --git a/src/mongo/db/repl/repl_set_config_checks.cpp b/src/mongo/db/repl/repl_set_config_checks.cpp
index b768317f041..df7049f9d25 100644
--- a/src/mongo/db/repl/repl_set_config_checks.cpp
+++ b/src/mongo/db/repl/repl_set_config_checks.cpp
@@ -142,6 +142,35 @@ Status validateSingleNodeChange(const ReplSetConfig& oldConfig, const ReplSetCon
 }
 
 /**
+ * Returns ErrorCodes::BadValue if the user tries to add new nodes with member id greater than
+ * MemberConfig::kMaxUserMemberId(255).
+ *
+ * validateMemberId() will be called only for replSetReconfig and replSetInitiate commands.
+ *
+ * TODO SERVER-48345: Remove this method on MongoDB v4.6 as we allow users to add new nodes
+ * with member id > MemberConfig::kMaxUserMemberId(255).
+ */
+Status validateMemberId(const ReplSetConfig& newConfig,
+                        const ReplSetConfig& oldConfig = ReplSetConfig()) {
+    for (ReplSetConfig::MemberIterator mNew = newConfig.membersBegin();
+         mNew != newConfig.membersEnd();
+         ++mNew) {
+        auto mNewConfigId = mNew->getId().getData();
+        // Existing members are allowed to have member id greater than kMaxUserMemberId. And,
+        // it can happen only if this node was previously running on MongoDB version greater
+        // than v4.4.
+        if (oldConfig.findMemberIndexByConfigId(mNewConfigId) == -1 &&
+            mNewConfigId > MemberConfig::kMaxUserMemberId) {
+            return Status(ErrorCodes::BadValue,
+                          str::stream()
+                              << "Replica set configuration contains new members with "
+                              << "member id greater than " << MemberConfig::kMaxUserMemberId);
+        }
+    }
+    return Status::OK();
+}
+
+/**
  * Compares two initialized and validated replica set configurations, and checks to
  * see if "newConfig" is a legal successor configuration to "oldConfig".
  *
@@ -331,6 +360,11 @@ StatusWith<int> validateConfigForInitiate(ReplicationCoordinatorExternalState* e
         return StatusWith<int>(status);
     }
 
+    status = validateMemberId(newConfig);
+    if (!status.isOK()) {
+        return status;
+    }
+
     status = newConfig.checkIfWriteConcernCanBeSatisfied(newConfig.getDefaultWriteConcern());
     if (!status.isOK()) {
         return status.withContext(
@@ -381,6 +415,11 @@ Status validateConfigForReconfig(const ReplSetConfig& oldConfig,
         return status;
     }
 
+    status = validateMemberId(newConfig, oldConfig);
+    if (!status.isOK()) {
+        return status;
+    }
+
     // For non-force reconfigs, verify that the reconfig only adds or removes a single node.
     // This ensures that all quorums of the new config overlap with all quorums of the old
     // config.
diff --git a/src/mongo/db/repl/repl_set_config_checks_test.cpp b/src/mongo/db/repl/repl_set_config_checks_test.cpp
index 39dd07b1eb4..0b6f573cc18 100644
--- a/src/mongo/db/repl/repl_set_config_checks_test.cpp
+++ b/src/mongo/db/repl/repl_set_config_checks_test.cpp
@@ -83,6 +83,24 @@ TEST_F(ServiceContextTest, ValidateConfigForInitiate_TermIsAlwaysInitialTerm) {
     ASSERT_EQUALS(config.getConfigTerm(), OpTime::kInitialTerm);
 }
 
+TEST_F(ServiceContextTest, ValidateConfigForInitiate_memberId) {
+    ReplicationCoordinatorExternalStateMock rses;
+    rses.addSelf(HostAndPort("h1"));
+
+    // Config with Member id > 255.
+    OID newReplSetId = OID::gen();
+    auto invalidConfig = ReplSetConfig::parseForInitiate(
+        BSON("_id"
+             << "rs0"
+             << "version" << 1 << "protocolVersion" << 1 << "members"
+             << BSON_ARRAY(BSON("_id" << (MemberConfig::kMaxUserMemberId + 1) << "host"
+                                      << "h1"))),
+        newReplSetId);
+    ASSERT_EQUALS(
+        ErrorCodes::BadValue,
+        validateConfigForInitiate(&rses, invalidConfig, getGlobalServiceContext()).getStatus());
+}
+
 TEST_F(ServiceContextTest, ValidateConfigForInitiate_MustFindSelf) {
     ReplSetConfig config;
     OID newReplSetId = OID::gen();
@@ -310,6 +328,55 @@ TEST_F(ServiceContextTest, ValidateConfigForReconfig_NewConfigVersionNumberMustB
                   validateConfigForReconfig(newConfig, oldConfig, false));
 }
 
+TEST_F(ServiceContextTest, ValidateConfigForReconfig_memberId) {
+    ReplicationCoordinatorExternalStateMock externalState;
+    externalState.addSelf(HostAndPort("h1"));
+
+    ReplSetConfig oldConfig;
+    ReplSetConfig newConfig;
+
+    // Case 1: Add a new node with member id > 255.
+    oldConfig = ReplSetConfig::parse(BSON("_id"
+                                          << "rs0"
+                                          << "version" << 1 << "protocolVersion" << 1 << "members"
+                                          << BSON_ARRAY(BSON("_id" << 1 << "host"
+                                                                   << "h1"))));
+    newConfig = ReplSetConfig::parse(
+        BSON("_id"
+             << "rs0"
+             << "version" << 2 << "protocolVersion" << 1 << "members"
+             << BSON_ARRAY(BSON("_id" << 1 << "host"
+                                      << "h1")
+                           << BSON("_id" << (MemberConfig::kMaxUserMemberId + 1) << "host"
+                                         << "h2"))));
+    ASSERT_OK(oldConfig.validate());
+    ASSERT_OK(newConfig.validate());
+    ASSERT_EQUALS(ErrorCodes::BadValue, validateConfigForReconfig(oldConfig, newConfig, false));
+
+    // Case 2: Change the member config setting for the existing member with member id > 255.
+    oldConfig = ReplSetConfig::parse(
+        BSON("_id"
+             << "rs0"
+             << "version" << 1 << "protocolVersion" << 1 << "members"
+             << BSON_ARRAY(BSON("_id" << 1 << "host"
+                                      << "h1")
+                           << BSON("_id" << (MemberConfig::kMaxUserMemberId + 1) << "host"
+                                         << "h2"))));
+    newConfig = ReplSetConfig::parse(
+        BSON("_id"
+             << "rs0"
+             << "version" << 2 << "protocolVersion" << 1 << "members"
+             << BSON_ARRAY(BSON("_id" << 1 << "host"
+                                      << "h1")
+                           << BSON("_id" << (MemberConfig::kMaxUserMemberId + 1) << "host"
+                                         << "h2"
+                                         << "priority" << 0))));
+    ASSERT_OK(oldConfig.validate());
+    ASSERT_OK(newConfig.validate());
+    ASSERT_OK(validateConfigForReconfig(oldConfig, newConfig, false));
+}
+
+
 TEST_F(ServiceContextTest, ValidateConfigForReconfig_NewConfigMustNotChangeSetName) {
     ReplicationCoordinatorExternalStateMock externalState;
     externalState.addSelf(HostAndPort("h1"));