CBL-4412: Enhance checkpoint resolution algorithm when local and remo…

…te checkpoint are mismatched.

LiteCore/Support/SequenceSet.hh

@@ -15,6 +15,7 @@
 #include <map>
 #include <string>
 #include <utility>
+#include <algorithm>
 #include "betterassert.hh"
 
 namespace litecore {
@@ -138,6 +139,35 @@ namespace litecore {
             }
         }
 
+        /** Creates an intersection with another sequence set to ensure that all missing
+            sequences get accounted for */
+        static SequenceSet intersection(const SequenceSet& s1, const SequenceSet& s2) {
+            SequenceSet retVal;
+            auto        i1 = s1.begin();
+            auto        i2 = s2.begin();
+
+            // Move through each of the sets and check if the current two are overlapping.
+            // If so, add the overlapping range and then check below which set(s) to advance.
+            while ( i1 != s1.end() && i2 != s2.end() ) {
+                auto start = std::max(i1->first, i2->first);
+                auto end   = std::min(i1->second, i2->second);
+                if ( start < end ) { retVal.add(start, end); }
+
+                // Need to evaluate this first before we advance anything
+                // If the first end value is less than the second, advance only the first iterator
+                // If the first end value is greater than the second, advance only the second iterator
+                // If equal, advance both
+                auto advanceFirst  = i1->second <= i2->second;
+                auto advanceSecond = i1->second >= i2->second;
+
+                if ( advanceFirst ) { ++i1; }
+
+                if ( advanceSecond ) { ++i2; }
+            }
+
+            return retVal;
+        }
+
         /** Iteration is over pair<sequence,sequence> values, where the first sequence is the
             start of a consecutive range, and the second sequence is the end of the range
             (one past the last sequence in the range.) */

Replicator/Checkpoint.cc

@@ -105,19 +105,42 @@ namespace litecore::repl {
     }
 
     bool Checkpoint::validateWith(const Checkpoint& remoteSequences) {
+        // If _completed or _remote changes in any way because of this method, this method must
+        // return false.  Currently the only way this remains true is if neither of the below if
+        // blocks are entered, or if we ignore the fact that the only difference in the local
+        // and remote checkpoint documents are that the INTEGRAL (i.e. NOT a backfill checkpoint)
+        // remote sequence on the local side is older than on the remote side and all we need
+        // to do is ignore the remote checkpoint and use the local checkpoint as-is.
         bool match = true;
+
         if ( _completed != remoteSequences._completed ) {
-            LogTo(SyncLog, "Local sequence mismatch: I had completed: %s, remote had %s",
+            LogTo(SyncLog, "Local sequence mismatch: I had completed: %s, remote had %s.",
                   _completed.to_string().c_str(), remoteSequences._completed.to_string().c_str());
-            resetLocal();
-            match = false;
+            LogTo(SyncLog, "Rolling back to a failsafe, some redundant changes may be proposed...");
+            _completed = SequenceSet::intersection(_completed, remoteSequences._completed);
+            match      = false;
         }
         if ( _remote && _remote != remoteSequences._remote ) {
             LogTo(SyncLog, "Remote sequence mismatch: I had '%s', remote had '%s'", _remote.toJSONString().c_str(),
                   remoteSequences._remote.toJSONString().c_str());
-            _remote = {};
-            match   = false;
+            if ( _remote.isInt() && remoteSequences._remote.isInt() ) {
+                if ( _remote.intValue() > remoteSequences._remote.intValue() ) {
+                    LogTo(SyncLog, "Rolling back to earlier remote sequence from server, some redundant changes may be "
+                                   "proposed...");
+                    _remote = remoteSequences._remote;
+                    match   = false;
+                } else {
+                    LogTo(SyncLog, "Ignoring remote sequence on server since client side is older, some redundant "
+                                   "changes may be proposed...");
+                }
+            } else {
+                Warn("Non-numeric remote sequence detected, resetting replication back to start.  Redundant changes "
+                     "will be proposed...");
+                _remote = {};
+                match   = false;
+            }
         }
+
         return match;
     }
 

Replicator/tests/ReplicatorAPITest.cc

@@ -22,6 +22,7 @@
 #include "c4Internal.hh"
 #include "fleece/Fleece.hh"
 #include "c4ReplicatorImpl.hh"
+#include "SequenceSet.hh"
 
 using namespace fleece;
 using namespace std;
@@ -1084,3 +1085,52 @@ TEST_CASE_METHOD(ReplicatorAPITest, "Large 64-bit values in max retry should not
     ReplicatorAPITestRemoteReplicator replicator(db, parameters);
     CHECK(replicator.maxRetryCount() == UINT_MAX);  // 32-bit capped
 }
+
+TEST_CASE("Sequence set merging", "[SequenceSet]") {
+    litecore::SequenceSet    s1, s2;
+    std::function<bool(int)> expectation;
+    int                      max;
+    SECTION("Equal sets") {
+        s1.add(1, 12);
+        s2.add(1, 12);
+        max         = 12;
+        expectation = ([](int i) { return true; });
+    }
+
+    SECTION("Non equal sets") {
+        s1.add(1, 6);
+        s1.add(7);
+        s1.add(9);
+        s1.add(11);
+
+        s2.add(1, 11);
+        max         = 12;
+        expectation = ([](int i) { return !(i == 6 || i == 8 || i == 10 || i == 11); });
+    }
+
+    SECTION("Non equal sets reversed") {
+        s2.add(1, 6);
+        s2.add(7);
+        s2.add(9);
+        s2.add(11);
+
+        s1.add(1, 11);
+        max         = 12;
+        expectation = ([](int i) { return !(i == 6 || i == 8 || i == 10 || i == 11); });
+    }
+
+    SECTION("Alternating") {
+        s1.add(1);
+        s1.add(3);
+        s1.add(5);
+
+        s2.add(2);
+        s2.add(4);
+        s2.add(6);
+        max         = 7;
+        expectation = ([](int i) { return false; });
+    }
+
+    litecore::SequenceSet intersection = litecore::SequenceSet::intersection(s1, s2);
+    for ( auto i = 1; i < 11; i++ ) { CHECK(intersection.contains(i) == expectation(i)); }
+}