SERVER-50881 Avoid array modifications in $v:1 idempotency tests

5 files changed, 41 insertions, 35 deletions

diff --git a/src/mongo/db/repl/idempotency_test.cpp b/src/mongo/db/repl/idempotency_test.cpp
index 9435ec730c6..aad94b99636 100644
--- a/src/mongo/db/repl/idempotency_test.cpp
+++ b/src/mongo/db/repl/idempotency_test.cpp
@@ -66,7 +66,8 @@ protected:
 
     std::string getStatesString(const std::vector<CollectionState>& state1,
                                 const std::vector<CollectionState>& state2,
-                                const std::vector<OplogEntry>& ops) override;
+                                const std::vector<OplogEntry>& state1Ops,
+                                const std::vector<OplogEntry>& state2Ops) override;
 
     Status resetState() override;
 
@@ -146,16 +147,17 @@ std::vector<OplogEntry> RandomizedIdempotencyTest::createUpdateSequence(
 
 std::string RandomizedIdempotencyTest::getStatesString(const std::vector<CollectionState>& state1,
                                                        const std::vector<CollectionState>& state2,
-                                                       const std::vector<OplogEntry>& ops) {
+                                                       const std::vector<OplogEntry>& state1Ops,
+                                                       const std::vector<OplogEntry>& state2Ops) {
     LOGV2(21157,
           "{IdempotencyTest_getStatesString_state1_state2_ops}",
           "IdempotencyTest_getStatesString_state1_state2_ops"_attr =
-              IdempotencyTest::getStatesString(state1, state2, ops));
+              IdempotencyTest::getStatesString(state1, state2, state1Ops, state2Ops));
     StringBuilder sb;
     sb << "Ran update ops: ";
     sb << "[ ";
     bool firstIter = true;
-    for (const auto& op : ops) {
+    for (const auto& op : state2Ops) {
         if (!firstIter) {
             sb << ", ";
         } else {
@@ -166,9 +168,13 @@ std::string RandomizedIdempotencyTest::getStatesString(const std::vector<Collect
     sb << " ]\n";
 
     ASSERT_OK(resetState());
+    ASSERT_OK(runOpsInitialSync(state1Ops));
+    sb << "Document at the end of state1: " << getDoc() << "\n";
 
-    sb << "Start: " << getDoc() << "\n";
-    for (const auto& op : ops) {
+    ASSERT_OK(resetState());
+
+    sb << "Start document for state2: " << getDoc() << "\n";
+    for (const auto& op : state2Ops) {
         ASSERT_OK(runOpInitialSync(op));
         sb << "Apply: " << op.getObject() << "\n  ==> " << getDoc() << "\n";
     }
@@ -193,8 +199,8 @@ void RandomizedIdempotencyTest::runIdempotencyTestCase() {
         ReplicationCoordinator::get(_opCtx.get())->setFollowerMode(MemberState::RS_RECOVERING));
 
     std::set<StringData> fields{"a", "b"};
-    size_t depth = 1;
-    size_t length = 1;
+    size_t depth = 2;
+    const size_t lengthOfNumericComponent = 1;
 
     // Eliminate modification of array elements, because they cause theoretically valid sequences
     // that cause idempotency issues.
@@ -205,15 +211,19 @@ void RandomizedIdempotencyTest::runIdempotencyTestCase() {
     this->seed = SecureRandom().nextInt64();
     PseudoRandom seedGenerator(this->seed);
     RandomizedScalarGenerator scalarGenerator{PseudoRandom(seedGenerator.nextInt64())};
-    UpdateSequenceGenerator updateGenerator(
-        {fields, depth, length, kScalarProbability, kDocProbability, kArrProbability},
-        PseudoRandom{seedGenerator.nextInt64()},
-        &scalarGenerator);
+    UpdateSequenceGenerator updateGenerator({fields,
+                                             depth,
+                                             lengthOfNumericComponent,
+                                             kScalarProbability,
+                                             kDocProbability,
+                                             kArrProbability},
+                                            PseudoRandom{seedGenerator.nextInt64()},
+                                            &scalarGenerator);
 
     const bool kSkipDocs = kDocProbability == 0.0;
     const bool kSkipArrs = kArrProbability == 0.0;
-    DocumentStructureEnumerator enumerator({fields, depth, length, kSkipDocs, kSkipArrs},
-                                           &scalarGenerator);
+    DocumentStructureEnumerator enumerator(
+        {fields, depth, lengthOfNumericComponent, kSkipDocs, kSkipArrs}, &scalarGenerator);
 
     const size_t kUpdateSequenceLength = 5;
     // For the sake of keeping the speed of iteration sane and feasible.
@@ -246,17 +256,10 @@ void RandomizedIdempotencyTest::runUpdateV2IdempotencyTestCase(double v2Probabil
     // idempotency if the entries are applied on an input document '{a: []}'. These entries should
     // not have been generated in practice if the starting document is '{a: []}', but the current
     // 'UpdateSequenceGenerator' is not smart enough to figure that out.
-    const double kScalarProbability = 0.375;
-    const double kDocProbability = 0.375;
-    const double kArrProbability = 0;
+    const size_t lengthOfNumericComponent = 0;
 
     std::set<StringData> fields{"f00", "f10", "f01", "f11", "f02", "f20"};
-    UpdateSequenceGenerator updateV1Generator({fields,
-                                               2 /* depth */,
-                                               2 /* length */,
-                                               kScalarProbability,
-                                               kDocProbability,
-                                               kArrProbability},
+    UpdateSequenceGenerator updateV1Generator({fields, 2 /* depth */, lengthOfNumericComponent},
                                               PseudoRandom(seedGenerator.nextInt64()),
                                               &scalarGenerator);
 
diff --git a/src/mongo/db/repl/idempotency_test_fixture.cpp b/src/mongo/db/repl/idempotency_test_fixture.cpp
index f721e567688..7207fc8aaaf 100644
--- a/src/mongo/db/repl/idempotency_test_fixture.cpp
+++ b/src/mongo/db/repl/idempotency_test_fixture.cpp
@@ -211,7 +211,7 @@ void IdempotencyTest::testOpsAreIdempotent(std::vector<OplogEntry> ops, Sequence
 
         auto state2 = validateAllCollections();
         if (state1 != state2) {
-            FAIL(getStatesString(state1, state2, fullSequence));
+            FAIL(getStatesString(state1, state2, ops, fullSequence));
         }
     }
 }
@@ -434,7 +434,8 @@ CollectionState IdempotencyTest::validate(const NamespaceString& nss) {
 
 std::string IdempotencyTest::getStatesString(const std::vector<CollectionState>& state1,
                                              const std::vector<CollectionState>& state2,
-                                             const std::vector<OplogEntry>& ops) {
+                                             const std::vector<OplogEntry>& state1Ops,
+                                             const std::vector<OplogEntry>& state2Ops) {
     StringBuilder sb;
     sb << "The states:\n";
     for (const auto& s : state1) {
@@ -446,7 +447,7 @@ std::string IdempotencyTest::getStatesString(const std::vector<CollectionState>&
     }
     sb << "found after applying the operations a second time, therefore breaking idempotency.\n";
     sb << "Applied ops:\n";
-    for (const auto& op : ops) {
+    for (const auto& op : state2Ops) {
         sb << op.toString() << "\n";
     }
     return sb.str();
diff --git a/src/mongo/db/repl/idempotency_test_fixture.h b/src/mongo/db/repl/idempotency_test_fixture.h
index 654e9c87ddd..1432e35f1f3 100644
--- a/src/mongo/db/repl/idempotency_test_fixture.h
+++ b/src/mongo/db/repl/idempotency_test_fixture.h
@@ -155,7 +155,8 @@ protected:
     std::string computeDataHash(const CollectionPtr& collection);
     virtual std::string getStatesString(const std::vector<CollectionState>& state1,
                                         const std::vector<CollectionState>& state2,
-                                        const std::vector<OplogEntry>& ops);
+                                        const std::vector<OplogEntry>& state1Ops,
+                                        const std::vector<OplogEntry>& state2Ops);
     /**
      * Validate data and indexes. Return the MD5 hash of the documents ordered by _id.
      */
diff --git a/src/mongo/db/repl/idempotency_update_sequence.cpp b/src/mongo/db/repl/idempotency_update_sequence.cpp
index 8394fbd74b7..3963e940276 100644
--- a/src/mongo/db/repl/idempotency_update_sequence.cpp
+++ b/src/mongo/db/repl/idempotency_update_sequence.cpp
@@ -43,13 +43,13 @@ namespace mongo {
 
 UpdateSequenceGeneratorConfig::UpdateSequenceGeneratorConfig(std::set<StringData> fields_,
                                                              std::size_t depth_,
-                                                             std::size_t length_,
+                                                             std::size_t lengthOfNumericComponent_,
                                                              double scalarProbability_,
                                                              double docProbability_,
                                                              double arrProbability_)
     : fields(std::move(fields_)),
       depth(depth_),
-      length(length_),
+      lengthOfNumericComponent(lengthOfNumericComponent_),
       scalarProbability(scalarProbability_),
       docProbability(docProbability_),
       arrProbability(arrProbability_) {}
@@ -79,7 +79,7 @@ void UpdateSequenceGenerator::_generatePaths(const UpdateSequenceGeneratorConfig
     }
 
     if (!path.empty()) {
-        for (std::size_t i = 0; i < config.length; i++) {
+        for (std::size_t i = 0; i < config.lengthOfNumericComponent; i++) {
             FieldRef arrPathRef(path);
             arrPathRef.appendPart(std::to_string(i));
             auto arrPath = arrPathRef.dottedField().toString();
@@ -101,7 +101,7 @@ void UpdateSequenceGenerator::_generatePaths(const UpdateSequenceGeneratorConfig
         _paths.push_back(docPath);
         UpdateSequenceGeneratorConfig remainingConfig = {remainingFields,
                                                          config.depth,
-                                                         config.length,
+                                                         config.lengthOfNumericComponent,
                                                          config.scalarProbability,
                                                          config.docProbability,
                                                          config.arrProbability};
@@ -261,8 +261,9 @@ DocumentStructureEnumerator UpdateSequenceGenerator::_getValidEnumeratorForPath(
         remainingDepth -= 1;
     }
 
-    DocumentStructureEnumerator enumerator({remainingFields, remainingDepth, this->_config.length},
-                                           this->_scalarGenerator);
+    DocumentStructureEnumerator enumerator(
+        {remainingFields, remainingDepth, this->_config.lengthOfNumericComponent},
+        this->_scalarGenerator);
     return enumerator;
 }
 
diff --git a/src/mongo/db/repl/idempotency_update_sequence.h b/src/mongo/db/repl/idempotency_update_sequence.h
index d6453eec7e4..2e98d5221f8 100644
--- a/src/mongo/db/repl/idempotency_update_sequence.h
+++ b/src/mongo/db/repl/idempotency_update_sequence.h
@@ -48,14 +48,14 @@ class BSONObjBuilder;
 struct UpdateSequenceGeneratorConfig {
     UpdateSequenceGeneratorConfig(std::set<StringData> fields_,
                                   std::size_t depth_,
-                                  std::size_t length_,
+                                  std::size_t lengthOfNumericComponent_,
                                   double scalarProbability_ = 0.250,
                                   double docProbability_ = 0.250,
                                   double arrProbability_ = 0.250);
 
     const std::set<StringData> fields = {};
     const std::size_t depth = 0;
-    const std::size_t length = 0;
+    const std::size_t lengthOfNumericComponent = 0;
     const double scalarProbability = 0.250;
     const double docProbability = 0.250;
     const double arrProbability = 0.250;