MB-42780: Expand tests and improve comments

+ Verify that Disk->Disk snapshot always creates new checkpoint
  at replica (regardless of MARKER_FLAG_CHK being sent or not)

Change-Id: Ic0a1741aff10d57c0ec7be2929bba136b37f9e5a
Reviewed-on: http://review.couchbase.org/c/kv_engine/+/140643
Well-Formed: Build Bot <[email protected]>
Tested-by: Build Bot <[email protected]>
Reviewed-by: Dave Rigby <[email protected]>

Author: paolococchi

engines/ep/src/dcp/passive_stream.cc

@@ -996,11 +996,12 @@ void PassiveStream::processMarker(SnapshotMarker* marker) {
                 // MB-42780: In general we cannot merge multiple snapshots into
                 // the same checkpoint. The only exception is for when replica
                 // receives multiple Memory checkpoints in a row.
+                // Since 6.5.0 the Active behaves correctly with regard to that
+                // (ie, the Active always sets the MARKER_FLAG_CHK in a snapshot
+                // transition tha involves Disk snapshots), but older Producers
+                // may still miss the MARKER_FLAG_CHK.
                 if (prevSnapType == Snapshot::Memory &&
                     cur_snapshot_type == Snapshot::Memory) {
-                    // If we are reconnecting then we need to update the snap
-                    // end and potentially the checkpoint type as We do not send
-                    // the CHK snapshot marker flag for disk snapshots.
                     ckptMgr.updateCurrentSnapshot(cur_snapshot_end.load(),
                                                   visibleSeq,
                                                   checkpointType);

engines/ep/tests/module_tests/dcp_stream_test.cc

@@ -2646,24 +2646,34 @@ TEST_P(SingleThreadedPassiveStreamTest, ReplicaNeverMergesDiskSnapshot) {
                         CheckpointType::Disk /*expectedOpenCkptType*/);
     }
 
-    // Memory snap but previous snap is Disk -> no merge
+    // From Disk to Disk + we miss the MARKER_FLAG_CHK, still not merged
     {
         SCOPED_TRACE("");
         receiveSnapshot(5 /*snapStart*/,
                         5 /*snapEnd*/,
-                        dcp_marker_flag_t::MARKER_FLAG_MEMORY,
+                        dcp_marker_flag_t::MARKER_FLAG_DISK,
                         4 /*expectedNumCheckpoint*/,
-                        CheckpointType::Memory /*expectedOpenCkptType*/);
+                        CheckpointType::Disk /*expectedOpenCkptType*/);
     }
 
+    // Memory snap but previous snap is Disk -> no merge
     {
         SCOPED_TRACE("");
         receiveSnapshot(6 /*snapStart*/,
                         6 /*snapEnd*/,
-                        dcp_marker_flag_t::MARKER_FLAG_MEMORY | MARKER_FLAG_CHK,
+                        dcp_marker_flag_t::MARKER_FLAG_MEMORY,
                         5 /*expectedNumCheckpoint*/,
                         CheckpointType::Memory /*expectedOpenCkptType*/);
     }
+
+    {
+        SCOPED_TRACE("");
+        receiveSnapshot(7 /*snapStart*/,
+                        7 /*snapEnd*/,
+                        dcp_marker_flag_t::MARKER_FLAG_MEMORY | MARKER_FLAG_CHK,
+                        6 /*expectedNumCheckpoint*/,
+                        CheckpointType::Memory /*expectedOpenCkptType*/);
+    }
 }
 
 TEST_P(SingleThreadedActiveStreamTest,