MB-58961: Remove AS::isCurrentSnapshotCompleted()

Used only at ActiveStream::snapshot(), can be replaced with a 1-liner.

Change-Id: Ief86d2b1dfdcc228fc355ca467345057883ecf78
Reviewed-on: https://review.couchbase.org/c/kv_engine/+/199179
Tested-by: Paolo Cocchi <[email protected]>
Reviewed-by: Vesko Karaganev <[email protected]>
Well-Formed: Restriction Checker
Reviewed-by: Trond Norbye <[email protected]>

Author: paolococchi

engines/ep/src/dcp/active_stream.cc

@@ -1474,8 +1474,26 @@ void ActiveStream::snapshot(const OutstandingItemsResult& meta,
 
     lastReadSeqno.store(newLastReadSeqno);
 
+    // Note: ActiveStream is in a complete snapshot
+    // - Always, on active vbuckets
+    // - If we have sent up to the last seqno in the last marker range, for
+    //   non-active vbuckets
+    //
+    // @todo MB-58961:
+    // 1. Shouldn't it be a weak-inequality here (ie, <=) ?
+    // 2. Shouldn't we use lastSentSeqno in place of lastReadSeqno here?
+    // At the time of writing I'm pushing a non-logic change, so defer the above
+    const auto isReplicaSnapshotComplete =
+            lastSentSnapEndSeqno.load(std::memory_order_relaxed) <
+            lastReadSeqno;
+
+    // Note: Here we consider "!replica" rather than "active", but I believe
+    // that streaming from "pending|dead" is just illegal, so we should change
+    // this.
+
     const auto vb = engine->getVBucket(vb_);
-    if (vb && isCurrentSnapshotCompleted(*vb)) {
+    if (vb && (vb->getState() != vbucket_state_replica ||
+               isReplicaSnapshotComplete)) {
         // Get OptionalSeqnos which for the items list types should have values
         auto seqnoStart = items.front()->getBySeqno();
         auto seqnoEnd = items.back()->getBySeqno();
@@ -2303,17 +2321,6 @@ uint64_t ActiveStream::getLastSentSeqno() const {
     return lastSentSeqno.load();
 }
 
-bool ActiveStream::isCurrentSnapshotCompleted(const VBucket& vb) const {
-    if (vb.getState() != vbucket_state_replica) {
-        return true;
-    }
-    // @todo MB-58961:
-    // 1. Shouldn't it be a weak-inequality here (ie, <=) ?
-    // 2. Shouldn't we use lastSentSeqno in place of lastReadSeqno here?
-    // At the time of writing I'm pushing a non-logic change, so defer the above
-    return lastSentSnapEndSeqno.load(std::memory_order_relaxed) < lastReadSeqno;
-}
-
 bool ActiveStream::dropCheckpointCursor_UNLOCKED() {
     VBucketPtr vbucket = engine->getVBucket(vb_);
     if (!vbucket) {

engines/ep/src/dcp/active_stream.h

@@ -711,17 +711,6 @@ class ActiveStream : public Stream,
      */
     void scheduleBackfill_UNLOCKED(DcpProducer& producer, bool reschedule);
 
-    /**
-     * ActiveStream is in a complete snapshot:
-     * - Always, on active vbuckets
-     * - If we have sent up to the last seqno in the last marker range, for
-     *   non-active vbuckets
-     *
-     * @param vb Reference to the underlying vbucket
-     * @return Whether the stream is in a complete snapshot
-     */
-    bool isCurrentSnapshotCompleted(const VBucket& vb) const;
-
     /**
      * Drop the cursor registered with the checkpoint manager. Used during
      * cursor dropping. Upon failure to drop the cursor, puts stream to