MB-23535 [Ephemeral]: Return 0 for OBSERVE_SEQNO::persisted_seqno 15/77115/5
authorDave Rigby <daver@couchbase.com>
Thu, 20 Apr 2017 10:54:35 +0000 (11:54 +0100)
committerDave Rigby <daver@couchbase.com>
Thu, 20 Apr 2017 20:51:26 +0000 (20:51 +0000)
Ephemeral buckets (which have no persistence) should not return a
last_persisted_seqno in the OBSERVE_SEQNO response - that would be
inaccurate (and misleading) to applications which are attempting to
verify that an item has been persisted to disk. Instead zero should be
returned.

However, *wihin* the cluster we still need to report the highest
sequence number a vBucket is up to for things like DCP takeover, and
currently both of these use-cases use the same method -
getPersistenceSeqno().

To support both use-cases, add a new getPublicPersistenceSeqno()
method which is used when reporting persistence to clients. For EP
buckets this is identical to the original getPersistenceSeqno()
method, but for Ephemeral it always returns zero.

Update OBSERVE_SEQNO tests as appropriate so they check for the
correct sequence number based on the bucket type.

Change-Id: I2db32bd0747e45a749bf964d2152bdfe79d1e3d2
Reviewed-on: http://review.couchbase.org/77115
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
src/ep_engine.cc
src/ep_vb.h
src/ephemeral_vb.h
src/vbucket.h
tests/ep_testsuite.cc

index eb252c4..30f92a6 100644 (file)
@@ -4157,7 +4157,8 @@ void EventuallyPersistentEngine::addSeqnoVbStats(const void *cookie,
         add_casted_stat(buffer, vb->getHighSeqno(), add_stat, cookie);
         checked_snprintf(buffer, sizeof(buffer), "vb_%d:last_persisted_seqno",
                          vb->getId());
-        add_casted_stat(buffer, vb->getPersistenceSeqno(), add_stat, cookie);
+        add_casted_stat(
+                buffer, vb->getPublicPersistenceSeqno(), add_stat, cookie);
         checked_snprintf(buffer, sizeof(buffer), "vb_%d:uuid", vb->getId());
         add_casted_stat(buffer, entry.vb_uuid, add_stat, cookie);
         checked_snprintf(buffer, sizeof(buffer), "vb_%d:purge_seqno",
@@ -4544,7 +4545,7 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::observe_seqno(
        }
 
        format_type = 1;
-       last_persisted_seqno = htonll(vb->getPersistenceSeqno());
+       last_persisted_seqno = htonll(vb->getPublicPersistenceSeqno());
        current_seqno = htonll(vb->getHighSeqno());
        latest_uuid = htonll(entry.vb_uuid);
        vb_id = htons(vb_id);
@@ -4560,7 +4561,7 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::observe_seqno(
        result.write((char*) &failover_highseqno, sizeof(uint64_t));
     } else {
         format_type = 0;
-        last_persisted_seqno = htonll(vb->getPersistenceSeqno());
+        last_persisted_seqno = htonll(vb->getPublicPersistenceSeqno());
         current_seqno = htonll(vb->getHighSeqno());
         vb_id   =  htons(vb_id);
         vb_uuid =  htonll(vb_uuid);
@@ -4882,7 +4883,7 @@ EventuallyPersistentEngine::handleSeqnoCmds(const void *cookie,
                         "for vb:%" PRIu16 ", Persisted seqno %" PRIu64
                         " > requested seqno %" PRIu64,
                         vbucket,
-                        vb->getPersistenceSeqno(),
+                        persisted_seqno,
                         seqno);
                     break;
                 }
index 7a42cc4..81a9a84 100644 (file)
@@ -100,6 +100,11 @@ public:
         return persistenceSeqno.load();
     }
 
+    uint64_t getPublicPersistenceSeqno() const override {
+        // For EPVBuckets this is the same as the PersistenceSeqno.
+        return getPersistenceSeqno();
+    }
+
     void queueBackfillItem(queued_item& qi,
                            const GenerateBySeqno generateBySeqno) override;
 
index d4d2586..c28733d 100644 (file)
@@ -120,14 +120,17 @@ public:
     void dump() const override;
 
     uint64_t getPersistenceSeqno() const override {
-        /* We do not have persistence in an ephemeral vb. hence we return
-           the last seen seqno (highSeqno) as the persisted seqno.
-           This is needed because higher layers like ns_server have long
-           considered persisted seqno as last seen seqno for certain operations
-           like vb takeover */
+        /* Technically we do not have persistence in an ephemeral vb, however
+         * logically the "persistence" seqno is used internally as the
+         * value to use for replication / takeover. Hence we return
+           the last seen seqno (highSeqno) as the persisted seqno. */
         return static_cast<uint64_t>(getHighSeqno());
     }
 
+    uint64_t getPublicPersistenceSeqno() const override {
+        return 0;
+    }
+
     void queueBackfillItem(queued_item& qi,
                            const GenerateBySeqno generateBySeqno) override;
 
index c755e7c..074c27b 100644 (file)
@@ -311,6 +311,26 @@ public:
     // Returns the last persisted sequence number for the VBucket
     virtual uint64_t getPersistenceSeqno() const = 0;
 
+    /**
+     * Returns the sequence number to expose publically as the highest
+     * persisted seqno. Note this is may differ from getPersistenceSeqno,
+     * depending on the Bucket type.
+     *
+     * Historical note: This is the same as PersistenceSeqno for EP buckets,
+     * and hence before Spock wasn't a separate function; however for Ephemeral
+     * buckets we need to distinguish between what sequence number we report
+     * to external clients for Observe/persistTo, and what sequence number we
+     * report to internal DCP / ns_server for takeover:
+     *  a) Clients need 0 for the Ephemeral "persisted to" seqno (as
+     *     there isn't any Persistence and we can't claim something is on-disk
+     *     when it is not).
+     *  b) ns_server / replication needs a non-zero, "logically-persisted" seqno
+     *     from the replica to know that a vBucket is ready for takeover.
+     * As such, getPublicPersistenceSeqno() is used for (a), and
+     * getPersistenceSeqno() is used for (b).
+     */
+    virtual uint64_t getPublicPersistenceSeqno() const = 0;
+
     void setPersistenceSeqno(uint64_t seqno) {
         persistenceSeqno.store(seqno);
     }
index b1ebe2e..ea63d7d 100644 (file)
@@ -83,12 +83,18 @@ public:
     int               extra;
 };
 
-
-static void check_observe_seqno(bool failover, uint8_t format_type, uint16_t vb_id,
-                                uint64_t vb_uuid, uint64_t last_persisted_seqno,
-                                uint64_t current_seqno, uint64_t failover_vbuuid = 0,
+enum class BucketType { EP, Ephemeral };
+
+static void check_observe_seqno(bool failover,
+                                BucketType bucket_type,
+                                uint8_t format_type,
+                                uint16_t vb_id,
+                                uint64_t vb_uuid,
+                                uint64_t last_persisted_seqno,
+                                uint64_t current_seqno,
+                                uint64_t failover_vbuuid = 0,
                                 uint64_t failover_seqno = 0) {
-    uint8_t  recv_format_type;
+    uint8_t recv_format_type;
     uint16_t recv_vb_id;
     uint64_t recv_vb_uuid;
     uint64_t recv_last_persisted_seqno;
@@ -103,8 +109,23 @@ static void check_observe_seqno(bool failover, uint8_t format_type, uint16_t vb_
     memcpy(&recv_vb_uuid, last_body.data() + 3, sizeof(uint64_t));
     checkeq(vb_uuid, ntohll(recv_vb_uuid), "Wrong vbucket uuid in result");
     memcpy(&recv_last_persisted_seqno, last_body.data() + 11, sizeof(uint64_t));
-    checkeq(last_persisted_seqno, ntohll(recv_last_persisted_seqno),
-            "Wrong persisted seqno in result");
+
+    switch (bucket_type) {
+    case BucketType::EP:
+        // Should get the "real" persisted seqno:
+        checkeq(last_persisted_seqno,
+                ntohll(recv_last_persisted_seqno),
+                "Wrong persisted seqno in result (EP)");
+        break;
+    case BucketType::Ephemeral:
+        // For ephemeral, this should always be zero, as there is no
+        // persistence.
+        checkeq(uint64_t(0),
+                ntohll(recv_last_persisted_seqno),
+                "Wrong persisted seqno in result (Ephemeral)");
+        break;
+    }
+
     memcpy(&recv_current_seqno, last_body.data() + 19, sizeof(uint64_t));
     checkeq(current_seqno, ntohll(recv_current_seqno), "Wrong current seqno in result");
 
@@ -4184,9 +4205,14 @@ static enum test_result test_observe_seqno_basic_tests(ENGINE_HANDLE *h,
     uint64_t high_seqno = get_int_stat(h, h1, "vb_1:high_seqno", "vbucket-seqno");
     observe_seqno(h, h1, 1, vb_uuid);
 
-    checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS, last_status.load(), "Expected success");
+    checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS,
+            last_status.load(),
+            "Expected success");
 
-    check_observe_seqno(false, 0, 1, vb_uuid, high_seqno, high_seqno);
+    const auto bucket_type =
+            isPersistentBucket(h, h1) ? BucketType::EP : BucketType::Ephemeral;
+    check_observe_seqno(
+            false, bucket_type, 0, 1, vb_uuid, high_seqno, high_seqno);
 
     //Add some mutations and verify the output
     int num_items = 10;
@@ -4216,7 +4242,8 @@ static enum test_result test_observe_seqno_basic_tests(ENGINE_HANDLE *h,
 
     observe_seqno(h, h1, 1, vb_uuid);
 
-    check_observe_seqno(false, 0, 1, vb_uuid, total_persisted, high_seqno);
+    check_observe_seqno(
+            false, bucket_type, 0, 1, vb_uuid, total_persisted, high_seqno);
     //Stop persistence. Add more mutations and check observe result
     stop_persistence(h, h1);
 
@@ -4238,7 +4265,8 @@ static enum test_result test_observe_seqno_basic_tests(ENGINE_HANDLE *h,
         /* if bucket is not persistent then total_persisted == high_seqno */
         total_persisted = high_seqno;
     }
-    check_observe_seqno(false, 0, 1, vb_uuid, total_persisted, high_seqno);
+    check_observe_seqno(
+            false, bucket_type, 0, 1, vb_uuid, total_persisted, high_seqno);
     start_persistence(h, h1);
     wait_for_flusher_to_settle(h, h1);
 
@@ -4250,7 +4278,8 @@ static enum test_result test_observe_seqno_basic_tests(ENGINE_HANDLE *h,
 
     observe_seqno(h, h1, 1, vb_uuid);
 
-    check_observe_seqno(false, 0, 1, vb_uuid, total_persisted, high_seqno);
+    check_observe_seqno(
+            false, bucket_type, 0, 1, vb_uuid, total_persisted, high_seqno);
     return SUCCESS;
 }
 
@@ -4287,8 +4316,17 @@ static enum test_result test_observe_seqno_failover(ENGINE_HANDLE *h,
 
     observe_seqno(h, h1, 0, vb_uuid);
 
-    check_observe_seqno(true, 1, 0, new_vb_uuid, high_seqno, high_seqno,
-                        vb_uuid, high_seqno);
+    const auto bucket_type =
+            isPersistentBucket(h, h1) ? BucketType::EP : BucketType::Ephemeral;
+    check_observe_seqno(true,
+                        bucket_type,
+                        1,
+                        0,
+                        new_vb_uuid,
+                        high_seqno,
+                        high_seqno,
+                        vb_uuid,
+                        high_seqno);
 
     return SUCCESS;
 }