MB-20852 [18/N]: Remove now dead VBucket persist Tasks 51/69151/9
authorDave Rigby <daver@couchbase.com>
Mon, 24 Oct 2016 11:46:01 +0000 (12:46 +0100)
committerDave Rigby <daver@couchbase.com>
Tue, 1 Nov 2016 09:45:35 +0000 (09:45 +0000)
Delete all tasks (apart from Flusher) which used to persist VBucket
state to disk - the Flusher Task now handles all VBucket state
persistence.

Also remove statistics which are no longer valid / relevent -
snapshotVbucketHisto / persistVBStateHisto - we no longer snapshot
separately from flushing.

Change-Id: Ia1b3f30f0f792a8ca739d377443f909138dcaa86
Reviewed-on: http://review.couchbase.org/69151
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
docs/stats.org
src/ep.cc
src/ep.h
src/ep_engine.cc
src/kvshard.cc
src/kvshard.h
src/stats.h
src/tasks.cc
src/tasks.def.h
src/tasks.h
tests/ep_testsuite.cc

index cb1eda5..f2503a0 100644 (file)
@@ -351,7 +351,6 @@ For introductory information on stats within Couchbase, start with the
 | ep_diskqueue_fill        | Total enqueued items on disk queue             |
 | ep_diskqueue_drain       | Total drained items on disk queue              |
 | ep_diskqueue_pending     | Total bytes of pending writes                  |
-| ep_vb_snapshot_total     | Number of VB state snapshots persisted to disk |
 | ep_persist_vbstate_total | Total VB persist state to disk                 |
 | ep_meta_data_memory      | Total memory used by meta data                 |
 | ep_meta_data_disk        | Total disk used by meta data                   |
@@ -845,7 +844,6 @@ form to describe when time was spent doing various things:
 | disk_del              | waiting for disk to delete an item             |
 | disk_vb_del           | waiting for disk to delete a vbucket           |
 | disk_commit           | waiting for a commit after a batch of updates  |
-| disk_vbstate_snapshot | Time spent persisting vbucket state changes    |
 | item_alloc_sizes      | Item allocation size counters (in bytes)       |
 
 The following histograms are available from "scheduler" and "runtimes"
index 19ccecd..bd71b57 100644 (file)
--- a/src/ep.cc
+++ b/src/ep.cc
@@ -302,10 +302,6 @@ EventuallyPersistentStore::EventuallyPersistentStore(
 
     size_t num_vbs = config.getMaxVbuckets();
     vb_mutexes = new Mutex[num_vbs];
-    schedule_vbstate_persist = new AtomicValue<bool>[num_vbs];
-    for (size_t i = 0; i < num_vbs; ++i) {
-        schedule_vbstate_persist[i] = false;
-    }
 
     stats.memOverhead = sizeof(EventuallyPersistentStore);
 
@@ -477,7 +473,6 @@ EventuallyPersistentStore::~EventuallyPersistentStore() {
                                             stats.forceShutdown);
 
     delete [] vb_mutexes;
-    delete [] schedule_vbstate_persist;
     delete [] stats.schedulingHisto;
     delete [] stats.taskRuntimeHisto;
     delete warmupTask;
@@ -1133,121 +1128,6 @@ class KVStatsCallback : public Callback<kvstats_ctx> {
         EventuallyPersistentStore *epstore;
 };
 
-void EventuallyPersistentStore::snapshotVBuckets(VBSnapshotTask::Priority prio,
-                                                 uint16_t shardId) {
-
-    class VBucketStateVisitor : public VBucketVisitor {
-    public:
-        VBucketStateVisitor(VBucketMap &vb_map, uint16_t sid)
-            : vbuckets(vb_map), shardId(sid) { }
-        bool visitBucket(RCPtr<VBucket> &vb) {
-            if (vbuckets.getShardByVbId(vb->getId())->getId() == shardId) {
-                states.insert(std::make_pair(vb->getId(),
-                                             vb->getVBucketState()));
-            }
-            return false;
-        }
-
-        void visit(StoredValue*) {
-            throw std::logic_error("VBucketStateVisitor:visit: Should never be called");
-        }
-
-        std::map<uint16_t, vbucket_state> states;
-
-    private:
-        VBucketMap &vbuckets;
-        uint16_t shardId;
-    };
-
-    KVShard *shard = vbMap.shards[shardId];
-    if (prio == VBSnapshotTask::Priority::LOW) {
-        shard->setLowPriorityVbSnapshotFlag(false);
-    } else {
-        shard->setHighPriorityVbSnapshotFlag(false);
-    }
-
-    KVStatsCallback kvcb(this);
-    VBucketStateVisitor v(vbMap, shard->getId());
-    visit(v);
-    hrtime_t start = gethrtime();
-
-    bool success = true;
-    vbucket_map_t::reverse_iterator iter = v.states.rbegin();
-    for (; iter != v.states.rend(); ++iter) {
-        LockHolder lh(vb_mutexes[iter->first], true /*tryLock*/);
-        if (!lh.islocked()) {
-            continue;
-        }
-        KVStore *rwUnderlying = getRWUnderlying(iter->first);
-        if (!rwUnderlying->snapshotVBucket(iter->first, iter->second,
-                    &kvcb)) {
-            LOG(EXTENSION_LOG_WARNING,
-                    "VBucket snapshot task failed!!! Rescheduling");
-            success = false;
-            break;
-        }
-
-        if (prio == VBSnapshotTask::Priority::HIGH) {
-            if (vbMap.setBucketCreation(iter->first, false)) {
-                LOG(EXTENSION_LOG_INFO, "VBucket %d created", iter->first);
-            }
-        }
-    }
-
-    if (!success) {
-        scheduleVBSnapshot(prio);
-    } else {
-        stats.snapshotVbucketHisto.add((gethrtime() - start) / 1000);
-    }
-}
-
-bool EventuallyPersistentStore::persistVBState(uint16_t vbid) {
-    schedule_vbstate_persist[vbid] = false;
-
-    RCPtr<VBucket> vb = getVBucket(vbid);
-    if (!vb) {
-        LOG(EXTENSION_LOG_WARNING,
-            "VBucket %d not exist!!! vb_state persistence task failed!!!", vbid);
-        return false;
-    }
-
-    bool inverse = false;
-    LockHolder lh(vb_mutexes[vbid], true /*tryLock*/);
-    if (!lh.islocked()) {
-        if (schedule_vbstate_persist[vbid].compare_exchange_strong(inverse,
-                                                                   true)) {
-            return true;
-        } else {
-            return false;
-        }
-    }
-
-    const hrtime_t start = gethrtime();
-
-    KVStatsCallback kvcb(this);
-
-    const vbucket_state vb_state(vb->getVBucketState());
-
-    KVStore *rwUnderlying = getRWUnderlying(vbid);
-    if (rwUnderlying->snapshotVBucket(vbid, vb_state, &kvcb)) {
-        stats.persistVBStateHisto.add((gethrtime() - start) / 1000);
-        if (vbMap.setBucketCreation(vbid, false)) {
-            LOG(EXTENSION_LOG_INFO, "VBucket %d created", vbid);
-        }
-    } else {
-        LOG(EXTENSION_LOG_WARNING,
-            "VBucket %d: vb_state persistence task failed!!! Rescheduling", vbid);
-
-        if (schedule_vbstate_persist[vbid].compare_exchange_strong(inverse,
-                                                                   true)) {
-            return true;
-        } else {
-            return false;
-        }
-    }
-    return false;
-}
-
 ENGINE_ERROR_CODE EventuallyPersistentStore::setVBucketState(uint16_t vbid,
                                                            vbucket_state_t to,
                                                            bool transfer,
@@ -1337,31 +1217,6 @@ ENGINE_ERROR_CODE EventuallyPersistentStore::setVBucketState(uint16_t vbid,
     return ENGINE_SUCCESS;
 }
 
-bool EventuallyPersistentStore::scheduleVBSnapshot(VBSnapshotTask::Priority prio) {
-    KVShard *shard = NULL;
-    if (prio == VBSnapshotTask::Priority::HIGH) {
-        for (size_t i = 0; i < vbMap.shards.size(); ++i) {
-            shard = vbMap.shards[i];
-            if (shard->setHighPriorityVbSnapshotFlag(true)) {
-                ExTask task = new VBSnapshotTaskHigh(&engine, i, true);
-                ExecutorPool::get()->schedule(task, WRITER_TASK_IDX);
-            }
-        }
-    } else {
-        for (size_t i = 0; i < vbMap.shards.size(); ++i) {
-            shard = vbMap.shards[i];
-            if (shard->setLowPriorityVbSnapshotFlag(true)) {
-                ExTask task = new VBSnapshotTaskLow(&engine, i, true);
-                ExecutorPool::get()->schedule(task, WRITER_TASK_IDX);
-            }
-        }
-    }
-    if (stats.isShutdown) {
-        return false;
-    }
-    return true;
-}
-
 void EventuallyPersistentStore::scheduleVBStatePersist() {
     for (auto vbid : vbMap.getBuckets()) {
         scheduleVBStatePersist(vbid);
index cb941de..c45faa6 100644 (file)
--- a/src/ep.h
+++ b/src/ep.h
@@ -517,8 +517,6 @@ public:
         return vbMap.getPersistenceSeqno(vb);
     }
 
-    void snapshotVBuckets(VBSnapshotTask::Priority prio, uint16_t shardId);
-
     /* transfer should be set to true *only* if this vbucket is becoming master
      * as the result of the previous master cleanly handing off control. */
     ENGINE_ERROR_CODE setVBucketState(uint16_t vbid, vbucket_state_t state,
@@ -687,11 +685,6 @@ public:
     }
 
     /**
-     * schedule a vb_state snapshot task for all the shards.
-     */
-    bool scheduleVBSnapshot(VBSnapshotTask::Priority prio);
-
-    /**
      * Schedule a vbstate persistence operation for all vbuckets.
      */
     void scheduleVBStatePersist();
@@ -1031,7 +1024,6 @@ protected:
     /* Array of mutexes for each vbucket
      * Used by flush operations: flushVB, deleteVB, compactVB, snapshotVB */
     Mutex                          *vb_mutexes;
-    AtomicValue<bool>              *schedule_vbstate_persist;
     std::vector<MutationLog*>       accessLog;
 
     AtomicValue<size_t> bgFetchQueue;
index c1c4442..366344d 100644 (file)
@@ -3407,9 +3407,6 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::doEngineStats(const void *cookie,
                     deadCountVisitor.getFileSize(),
                     add_stat, cookie);
 
-    add_casted_stat("ep_vb_snapshot_total",
-                    epstats.snapshotVbucketHisto.total(), add_stat, cookie);
-
     add_casted_stat("ep_persist_vbstate_total",
                     epstats.totalPersistVBState, add_stat, cookie);
 
@@ -4468,10 +4465,6 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::doTimingStats(const void *cookie,
     add_casted_stat("disk_del", stats.diskDelHisto, add_stat, cookie);
     add_casted_stat("disk_vb_del", stats.diskVBDelHisto, add_stat, cookie);
     add_casted_stat("disk_commit", stats.diskCommitHisto, add_stat, cookie);
-    add_casted_stat("disk_vbstate_snapshot", stats.snapshotVbucketHisto,
-                    add_stat, cookie);
-    add_casted_stat("disk_persist_vbstate", stats.persistVBStateHisto,
-                    add_stat, cookie);
 
     add_casted_stat("item_alloc_sizes", stats.itemAllocSizeHisto,
                     add_stat, cookie);
index b97e7f0..fb2e138 100644 (file)
@@ -25,8 +25,7 @@
 #include "kvshard.h"
 
 KVShard::KVShard(uint16_t id, EventuallyPersistentStore &store) :
-    shardId(id), highPrioritySnapshot(false),
-    lowPrioritySnapshot(false),
+    shardId(id),
     kvConfig(store.getEPEngine().getConfiguration(), shardId),
     highPriorityCount(0)
 {
@@ -134,16 +133,6 @@ std::vector<VBucket::id_type> KVShard::getVBuckets() {
     return rv;
 }
 
-bool KVShard::setHighPriorityVbSnapshotFlag(bool highPriority) {
-    bool inverse = !highPriority;
-    return highPrioritySnapshot.compare_exchange_strong(inverse, highPriority);
-}
-
-bool KVShard::setLowPriorityVbSnapshotFlag(bool lowPriority) {
-    bool inverse = !lowPriority;
-    return lowPrioritySnapshot.compare_exchange_strong(inverse, lowPriority);
-}
-
 void NotifyFlusherCB::callback(uint16_t &vb) {
     if (shard->getBucket(vb)) {
         shard->notifyFlusher();
index 459bd5b..9b77c77 100644 (file)
@@ -79,57 +79,6 @@ public:
     std::vector<VBucket::id_type> getVBuckets();
     size_t getMaxNumVbuckets() { return maxVbuckets; }
 
-    /**
-     * Set the flag to coordinate the scheduled high priority vbucket
-     * snapshot and new snapshot requests with the high priority. The
-     * flag is "true" if a snapshot task with the high priority is
-     * currently scheduled, otherwise "false".  If (1) the flag is
-     * currently "false" and (2) a new snapshot request invokes this
-     * method by passing "true" parameter, this will set the flag to
-     * "true" and return "true" to indicate that the new request can
-     * be scheduled now. Otherwise, return "false" to prevent
-     * duplciate snapshot tasks from being scheduled.  When the
-     * snapshot task is running and about to writing to disk, it will
-     * invoke this method to reset the flag by passing "false"
-     * parameter.
-     *
-     * @param highPrioritySnapshot bool flag for coordination between
-     *                             the scheduled snapshot task and new
-     *                             snapshot requests.
-     * @return "true" if a flag's value was changed. Otherwise "false".
-     */
-    bool setHighPriorityVbSnapshotFlag(bool highPrioritySnapshot);
-    bool getHighPriorityVbSnapshotFlag(void) {
-        return highPrioritySnapshot;
-    }
-
-    /**
-     * Set the flag to coordinate the scheduled low priority vbucket
-     * snapshot and new snapshot requests with the low priority. The
-     * flag is "true" if a snapshot task with the low priority is
-     * currently scheduled, otherwise "false".  If (1) the flag is
-     * currently "false" and (2) a new snapshot request invokes this
-     * method by passing "true" parameter, this will set the flag to
-     * "true" and return "true" to indicate that the new request can
-     * be scheduled now. Otherwise, return "false" to prevent
-     * duplciate snapshot tasks from being scheduled.  When the
-     * snapshot task is running and about to writing to disk, it will
-     * invoke this method to reset the flag by passing "false"
-     * parameter.
-     *
-     * @param lowPrioritySnapshot bool flag for coordination between
-     *                             the scheduled low priority snapshot
-     *                             task and new snapshot requests with
-     *                             low priority.
-     *
-     * @return "true" if a flag's value was changed. Otherwise
-     *                "false".
-     */
-    bool setLowPriorityVbSnapshotFlag(bool lowPrioritySnapshot);
-    bool getLowPriorityVbSnapshotFlag(void) {
-        return lowPrioritySnapshot;
-    }
-
 private:
     std::vector<RCPtr<VBucket>> vbuckets;
 
@@ -142,9 +91,6 @@ private:
     size_t maxVbuckets;
     uint16_t shardId;
 
-    AtomicValue<bool> highPrioritySnapshot;
-    AtomicValue<bool> lowPrioritySnapshot;
-
     KVStoreConfig kvConfig;
 
 public:
index f91bb75..66834e5 100644 (file)
@@ -552,12 +552,6 @@ public:
     //! Histogram of disk commits
     Histogram<hrtime_t> diskCommitHisto;
 
-    //! Histogram of setting vbucket state
-    Histogram<hrtime_t> snapshotVbucketHisto;
-
-    //! Histogram of persisting vbucket state
-    Histogram<hrtime_t> persistVBStateHisto;
-
     //! Histogram of mutation log compactor
     Histogram<hrtime_t> mlogCompactorHisto;
 
@@ -643,8 +637,6 @@ public:
         diskDelHisto.reset();
         diskVBDelHisto.reset();
         diskCommitHisto.reset();
-        snapshotVbucketHisto.reset();
-        persistVBStateHisto.reset();
         itemAllocSizeHisto.reset();
         dirtyAgeHisto.reset();
         mlogCompactorHisto.reset();
index a29ed1b..c08d549 100644 (file)
@@ -27,7 +27,6 @@
 
 #include <type_traits>
 
-static const double VBSTATE_SNAPSHOT_FREQ(300.0);
 static const double WORKLOAD_MONITOR_FREQ(5.0);
 
 GlobalTask::GlobalTask(Taskable& t,
@@ -79,10 +78,6 @@ static_assert(TaskPriority::MultiBGFetcherTask < TaskPriority::BGFetchCallback,
 static_assert(TaskPriority::BGFetchCallback == TaskPriority::VBDeleteTask,
               "BGFetchCallback not equal VBDeleteTask");
 
-static_assert(TaskPriority::VBStatePersistTaskHigh <
-              TaskPriority::VKeyStatBGFetchTask,
-              "VBStatePersistTaskHigh not less than VKeyStatBGFetchTask");
-
 static_assert(TaskPriority::VKeyStatBGFetchTask < TaskPriority::FlusherTask,
               "VKeyStatBGFetchTask not less than FlusherTask");
 
@@ -138,28 +133,6 @@ bool FlusherTask::run() {
     return flusher->step(this);
 }
 
-bool VBSnapshotTask::run() {
-    engine->getEpStore()->snapshotVBuckets(priority, shardID);
-    return false;
-}
-
-DaemonVBSnapshotTask::DaemonVBSnapshotTask(EventuallyPersistentEngine *e,
-                                           bool completeBeforeShutdown)
-    : GlobalTask(e, TaskId::DaemonVBSnapshotTask,
-                 VBSTATE_SNAPSHOT_FREQ, completeBeforeShutdown) {
-    desc = "Snapshotting vbucket states";
-}
-
-bool DaemonVBSnapshotTask::run() {
-    bool ret = engine->getEpStore()->scheduleVBSnapshot(VBSnapshotTask::Priority::LOW);
-    snooze(VBSTATE_SNAPSHOT_FREQ);
-    return ret;
-}
-
-bool VBStatePersistTask::run() {
-    return engine->getEpStore()->persistVBState(vbid);
-}
-
 bool VBDeleteTask::run() {
     return !engine->getEpStore()->completeVBucketDeletion(vbucketId, cookie);
 }
index fa4ef48..a7612f2 100644 (file)
@@ -56,13 +56,8 @@ TASK(BackfillVisitorTask, 8)
 TASK(VBDeleteTask, 1)
 TASK(RollbackTask, 1)
 TASK(CompactVBucketTask, 2)
-TASK(VBStatePersistTaskHigh, 2)
-TASK(VBSnapshotTaskHigh, 2)
 TASK(FlushAllTask, 3)
 TASK(FlusherTask, 5)
-TASK(VBStatePersistTaskLow, 9)
-TASK(VBSnapshotTaskLow, 9)
-TASK(DaemonVBSnapshotTask, 9)
 TASK(StatSnap, 9)
 
 // Non-IO tasks
index 558eb28..f26489e 100644 (file)
@@ -223,127 +223,6 @@ private:
 };
 
 /**
- * A task for persisting VBucket state changes to disk and creating new
- * VBucket database files.
- * sid (shard ID) passed on to GlobalTask indicates that task needs to be
- *     serialized with other tasks that require serialization on its shard
- */
-class VBSnapshotTask : public GlobalTask {
-public:
-    enum class Priority {
-        HIGH,
-        LOW
-    };
-
-    bool run();
-
-    std::string getDescription() {
-        return "Snapshotting vbucket states for the shard: " + std::to_string(shardID);
-    }
-
-protected:
-    VBSnapshotTask(EventuallyPersistentEngine *e,
-                   TaskId id,
-                   uint16_t sID = 0,
-                   bool completeBeforeShutdown = true)
-        : GlobalTask(e, id, 0, completeBeforeShutdown),
-          priority(Priority::LOW),
-          shardID(sID) {}
-    Priority priority;
-
-private:
-    uint16_t shardID;
-};
-
-class VBSnapshotTaskHigh : public VBSnapshotTask {
-public:
-    VBSnapshotTaskHigh(EventuallyPersistentEngine *e,
-                       uint16_t sID = 0,
-                       bool completeBeforeShutdown = true)
-        : VBSnapshotTask(e, TaskId::VBSnapshotTaskHigh,
-                         sID, completeBeforeShutdown) {
-            priority = Priority::HIGH;
-        }
-};
-
-class VBSnapshotTaskLow : public VBSnapshotTask {
-public:
-    VBSnapshotTaskLow(EventuallyPersistentEngine *e,
-                      uint16_t sID = 0,
-                      bool completeBeforeShutdown = true)
-        : VBSnapshotTask(e, TaskId::VBSnapshotTaskLow,
-                         sID, completeBeforeShutdown) {
-            priority = Priority::LOW;
-        }
-};
-
-/**
- * A daemon task for persisting VBucket state changes to disk periodically.
- */
-class DaemonVBSnapshotTask : public GlobalTask {
-public:
-    DaemonVBSnapshotTask(EventuallyPersistentEngine *e,
-                         bool completeBeforeShutdown = true);
-
-    bool run();
-
-    std::string getDescription() {
-        return desc;
-    }
-
-private:
-    std::string desc;
-};
-
-/**
- * A task for persisting a VBucket state to disk and creating a vbucket
- * database file if necessary.
- */
-class VBStatePersistTask : public GlobalTask {
-public:
-    enum class Priority {
-        HIGH,
-        LOW
-    };
-
-    bool run();
-
-    std::string getDescription() {
-        return "Persisting a vbucket state for vbucket: " + std::to_string(vbid);
-    }
-
-protected:
-    VBStatePersistTask(EventuallyPersistentEngine *e,
-                       TaskId taskId,
-                       uint16_t vbucket,
-                       bool completeBeforeShutdown = true)
-        : GlobalTask(e, taskId, 0, completeBeforeShutdown),
-          vbid(vbucket) {
-    }
-
-private:
-    uint16_t vbid;
-};
-
-class VBStatePersistTaskHigh : public VBStatePersistTask {
-public:
-    VBStatePersistTaskHigh(EventuallyPersistentEngine *e,
-                           uint16_t vbucket,
-                           bool completeBeforeShutdown = true)
-        : VBStatePersistTask(e, TaskId::VBStatePersistTaskHigh,
-                             vbucket, completeBeforeShutdown) {}
-};
-
-class VBStatePersistTaskLow : public VBStatePersistTask {
-public:
-    VBStatePersistTaskLow(EventuallyPersistentEngine *e,
-                          uint16_t vbucket,
-                          bool completeBeforeShutdown = true)
-        : VBStatePersistTask(e, TaskId::VBStatePersistTaskLow,
-                             vbucket, completeBeforeShutdown) {}
-};
-
-/**
  * A task for deleting VBucket files from disk and cleaning up any outstanding
  * writes for that VBucket file.
  * sid (shard ID) passed on to GlobalTask indicates that task needs to be
index f75e78e..069890c 100644 (file)
@@ -6496,7 +6496,6 @@ static enum test_result test_mb19687_fixed(ENGINE_HANDLE* h,
                 "ep_uuid",
                 "ep_value_size",
                 "ep_vb0",
-                "ep_vb_snapshot_total",
                 "ep_vb_total",
                 "ep_vbucket_del",
                 "ep_vbucket_del_fail",