SERVER-59867 Split horizon mappings in ReplSetConfig/MemberConfig should be serialized deterministically.

(cherry picked from commit 8675c70fd480557fd1c5c9b3ce9e731b1101f93f)

4 files changed, 74 insertions, 5 deletions

diff --git a/src/mongo/db/repl/repl_set_config.cpp b/src/mongo/db/repl/repl_set_config.cpp
index fc65ba021fa..f895c669848 100644
--- a/src/mongo/db/repl/repl_set_config.cpp
+++ b/src/mongo/db/repl/repl_set_config.cpp
@@ -933,6 +933,7 @@ BSONObj ReplSetConfig::toBSON() const {
                                   durationCount<Milliseconds>(_catchUpTakeoverDelay));
 
 
+    std::vector<std::pair<StringData, const ReplSetTagPattern*>> sortedModes;
     BSONObjBuilder gleModes(settingsBuilder.subobjStart(kGetLastErrorModesFieldName));
     for (StringMap<ReplSetTagPattern>::const_iterator mode = _customWriteConcernModes.begin();
          mode != _customWriteConcernModes.end();
@@ -941,9 +942,13 @@ BSONObj ReplSetConfig::toBSON() const {
             // Filter out internal modes
             continue;
         }
-        BSONObjBuilder modeBuilder(gleModes.subobjStart(mode->first));
-        for (ReplSetTagPattern::ConstraintIterator itr = mode->second.constraintsBegin();
-             itr != mode->second.constraintsEnd();
+        sortedModes.emplace_back(mode->first, &mode->second);
+    }
+    std::sort(sortedModes.begin(), sortedModes.end());
+    for (const auto& mode : sortedModes) {
+        BSONObjBuilder modeBuilder(gleModes.subobjStart(mode.first));
+        for (ReplSetTagPattern::ConstraintIterator itr = mode.second->constraintsBegin();
+             itr != mode.second->constraintsEnd();
              itr++) {
             modeBuilder.append(_tagConfig.getTagKey(ReplSetTag(itr->getKeyIndex(), 0)),
                                itr->getMinCount());
diff --git a/src/mongo/db/repl/repl_set_config_test.cpp b/src/mongo/db/repl/repl_set_config_test.cpp
index 6e76d6d880f..3fe40a2f72f 100644
--- a/src/mongo/db/repl/repl_set_config_test.cpp
+++ b/src/mongo/db/repl/repl_set_config_test.cpp
@@ -1228,6 +1228,49 @@ TEST(ReplSetConfig, toBSONRoundTripAbilityInvalid) {
     ASSERT_TRUE(configA == configB);
 }
 
+TEST(ReplSetConfig, toBSONRoundTripAbilityWithWriteConcernModesOrdered) {
+    // Objects which are the same should serialize identically, even if stored internally as maps.
+    ReplSetConfig configA;
+    ReplSetConfig configB;
+    ASSERT_OK(configA.initialize(
+        BSON("_id"
+             << "rs0"
+             << "version" << 1 << "protocolVersion" << 1 << "members"
+             << BSON_ARRAY(BSON("_id" << 0 << "host"
+                                      << "localhost:12345"
+                                      << "tags"
+                                      << BSON("tag1"
+                                              << "1"
+                                              << "tag2"
+                                              << "2"
+                                              << "tag3"
+                                              << "3"
+                                              << "tag4"
+                                              << "4"
+                                              << "tag5"
+                                              << "5"
+                                              << "tag6"
+                                              << "6"
+                                              << "tag7"
+                                              << "7"
+                                              << "tag8"
+                                              << "8"
+                                              << "tag9"
+                                              << "9")))
+             << "settings"
+             << BSON("heartbeatIntervalMillis"
+                     << 5000 << "heartbeatTimeoutSecs" << 20 << "replicaSetId" << OID::gen()
+                     << "getLastErrorModes"
+                     << BSON("wc2" << BSON("tag1" << 1 << "tag2" << 2) << "wc4" << BSON("tag3" << 3)
+                                   << "wc3" << BSON("tag5" << 5 << "tag4" << 4) << "wc1"
+                                   << BSON("tag6" << 6 << "tag8" << 8 << "tag7" << 7))))));
+    auto bsonA = configA.toBSON();
+    ASSERT_OK(configB.initialize(bsonA));
+    ASSERT_TRUE(configA == configB);
+    auto bsonB = configB.toBSON();
+    ASSERT_BSONOBJ_EQ(bsonA, bsonB);
+}
+
 TEST(ReplSetConfig, CheckIfWriteConcernCanBeSatisfied) {
     ReplSetConfig configA;
     ASSERT_OK(configA.initialize(BSON(
diff --git a/src/mongo/db/repl/split_horizon.cpp b/src/mongo/db/repl/split_horizon.cpp
index 82911488b5a..85ade87da84 100644
--- a/src/mongo/db/repl/split_horizon.cpp
+++ b/src/mongo/db/repl/split_horizon.cpp
@@ -214,12 +214,18 @@ void SplitHorizon::toBSON(BSONObjBuilder& configBuilder) const {
     if (_forwardMapping.size() == 1)
         return;
 
-    BSONObjBuilder horizonsBson(configBuilder.subobjStart("horizons"));
+    // StringMaps are iterated in arbitrary order, so we sort before returning the horizons.
+    std::vector<std::pair<StringData, std::string>> sortedHorizons;
     for (const auto& horizon : _forwardMapping) {
         // The "__default" horizon should never be emitted in the horizon table.
         if (horizon.first == SplitHorizon::kDefaultHorizon)
             continue;
-        horizonsBson.append(horizon.first, horizon.second.toString());
+        sortedHorizons.emplace_back(horizon.first, horizon.second.toString());
+    }
+    std::sort(sortedHorizons.begin(), sortedHorizons.end());
+    BSONObjBuilder horizonsBson(configBuilder.subobjStart("horizons"));
+    for (const auto& horizon : sortedHorizons) {
+        horizonsBson.append(horizon.first, horizon.second);
     }
 }
 
diff --git a/src/mongo/db/repl/split_horizon_test.cpp b/src/mongo/db/repl/split_horizon_test.cpp
index 95b2df2ad36..37e5515b04b 100644
--- a/src/mongo/db/repl/split_horizon_test.cpp
+++ b/src/mongo/db/repl/split_horizon_test.cpp
@@ -443,6 +443,12 @@ TEST(SplitHorizonTesting, BSONRoundTrip) {
     const Input tests[] = {
         {{{"horizon1", "horizon1.example.com:42"}}},
         {{{"horizon1", "horizon1.example.com:42"}, {"horizon2", "horizon2.example.com:42"}}},
+        {{{"horizon1", "horizon1.example.com:42"}, {"horizon2", "horizon2.example.com:42"}}},
+        {{{"q_horizon", "horizonq.example.com:42"},
+          {"b_horizon1", "horizonb.example.com:42"},
+          {"z_horizon1", "horizonz.example.com:42"},
+          {"horizon1", "horizon1.example.com:42"},
+          {"horizon2", "horizon2.example.com:42"}}},
     };
     for (const auto& input : tests) {
         const auto testNumber = &input - tests;
@@ -457,10 +463,19 @@ TEST(SplitHorizonTesting, BSONRoundTrip) {
 
         const SplitHorizon witness(HostAndPort(defaultHostAndPort), bson["horizons"]);
 
+        const BSONObj witnessBson = [&] {
+            BSONObjBuilder outputBuilder;
+            witness.toBSON(outputBuilder);
+            return outputBuilder.obj();
+        }();
+
         ASSERT_TRUE(horizon.getForwardMappings() == witness.getForwardMappings())
             << "Test #" << testNumber << " Failed on bson round trip with forward map";
         ASSERT_TRUE(horizon.getReverseHostMappings() == witness.getReverseHostMappings())
             << "Test #" << testNumber << " Failed on bson round trip with reverse map";
+        ASSERT_EQ(0, bson.woCompare(witnessBson))
+            << "Test #" << testNumber << " Failed on bson round trip with toBSON. BSONObj " << bson
+            << " != " << witnessBson;
     }
 }
 }  // namespace