MB-20852 [9/N]: Explicilty handle all queue_op uses 61/68161/8
authorDave Rigby <daver@couchbase.com>
Thu, 29 Sep 2016 14:05:14 +0000 (15:05 +0100)
committerDave Rigby <daver@couchbase.com>
Mon, 31 Oct 2016 16:18:20 +0000 (16:18 +0000)
Remove instances of switch statements where we don't check all
enumerations in queue_op, and instead handle them all. Makes it easier
/ safer to add any new queue_op types (and ensure they are handled
everywhere).

Change-Id: I9406c6d327d572268afa4d1830d690ee8ec153a1
Reviewed-on: http://review.couchbase.org/68161
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
src/checkpoint.cc
src/item.h
src/tapconnection.cc
src/tapconnection.h

index 71fdba8..a30553b 100644 (file)
@@ -236,8 +236,7 @@ size_t Checkpoint::mergePrevCheckpoint(Checkpoint *pPrevCheckpoint) {
 
     for (; rit != pPrevCheckpoint->rend(); ++rit) {
         const std::string &key = (*rit)->getKey();
-        if ((*rit)->getOperation() != queue_op::del &&
-            (*rit)->getOperation() != queue_op::set) {
+        if ((*rit)->isCheckPointMetaItem()) {
             continue;
         }
         checkpoint_index::iterator it = keyIndex.find(key);
index 704a215..db87574 100644 (file)
@@ -660,27 +660,33 @@ public:
      * Should this item be persisted?
      */
     bool shouldPersist() const {
-        return (op == queue_op::set) ||
-               (op == queue_op::del);
+        return !isCheckPointMetaItem();
     }
 
     /*
      * Should this item be replicated (e.g. by DCP)
      */
     bool shouldReplicate() const {
-        return (op == queue_op::set) ||
-               (op == queue_op::del);
+        return !isCheckPointMetaItem();
     }
 
     void setOperation(queue_op o) {
         op = o;
     }
 
-    bool isCheckPointMetaItem(void) const {
-        if ((queue_op::set == op) || (queue_op::del == op)) {
-            return false;
+    bool isCheckPointMetaItem() const {
+        switch (op) {
+            case queue_op::set:
+            case queue_op::del:
+                return false;
+            case queue_op::flush:
+            case queue_op::empty:
+            case queue_op::checkpoint_start:
+            case queue_op::checkpoint_end:
+                return true;
         }
-        return true;
+        // Silence GCC warning
+        return false;
     }
 
     void setNRUValue(uint8_t nru_value) {
index 8da702a..f57ee91 100644 (file)
@@ -1248,6 +1248,9 @@ queued_item TapProducer::nextFgFetched_UNLOCKED(bool &shouldPause) {
                 }
                 addEvent_UNLOCKED(qi);
                 break;
+            case queue_op::flush:
+                // ignored
+                break;
             case queue_op::checkpoint_start:
                 {
                     it->second.currentCheckpointId = qi->getRevSeqno();
@@ -1282,8 +1285,6 @@ queued_item TapProducer::nextFgFetched_UNLOCKED(bool &shouldPause) {
                     ++open_checkpoint_count;
                 }
                 break;
-            default:
-                break;
             }
         }
 
index 0d3e543..8842775 100644 (file)
@@ -150,14 +150,15 @@ public:
         case queue_op::flush:
             event_ = TAP_FLUSH;
             break;
+        case queue_op::empty:
+            // Ignored
+            break;
         case queue_op::checkpoint_start:
             event_ = TAP_CHECKPOINT_START;
             break;
         case queue_op::checkpoint_end:
             event_ = TAP_CHECKPOINT_END;
             break;
-        default:
-            break;
         }
     }