MB-20822: Erase all diverged branch entries correctly in failover table 81/67481/6
authorManu Dhundi <manu@couchbase.com>
Wed, 14 Sep 2016 19:12:29 +0000 (12:12 -0700)
committerManu Dhundi <manu@couchbase.com>
Fri, 30 Sep 2016 00:44:22 +0000 (00:44 +0000)
When we add an entry in failover table we must erase all the other
entries with higher seqno because they are from a diverged branch.

This commit fixes a bug in the loop that was erasing diverged entires.
In a std::list when we erase an entry, the function returns the
iterator pointing to next element and hence we must be careful not to
double increment it.

Change-Id: I9275fba8057f26e2853a8d7d359f1d01f107f2be
Reviewed-on: http://review.couchbase.org/67481
Reviewed-by: Dave Rigby <daver@couchbase.com>
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
src/failover-table.cc
src/failover-table.h
tests/module_tests/failover_table_test.cc

index 44b257e..2ba252a 100644 (file)
@@ -55,12 +55,9 @@ void FailoverTable::createEntry(uint64_t high_seqno) {
     LockHolder lh(lock);
     // Our failover table represents only *our* branch of history.
     // We must remove branches we've diverged from.
-    table_t::iterator itr = table.begin();
-    for (; itr != table.end(); ++itr) {
-        if (itr->by_seqno > high_seqno) {
-            itr = table.erase(itr);
-        }
-    }
+    table.remove_if([high_seqno](const failover_entry_t& e) {
+                        return (e.by_seqno > high_seqno);
+                    });
 
     failover_entry_t entry;
     entry.vb_uuid = (provider.next() >> 16);
@@ -343,6 +340,11 @@ void FailoverTable::replaceFailoverLog(uint8_t* bytes, uint32_t length) {
     cacheTableJSON();
 }
 
+size_t FailoverTable::getNumEntries() const
+{
+    return table.size();
+}
+
 void FailoverTable::adjustSnapshotRange(uint64_t start_seqno,
                                         uint64_t &snap_start_seqno,
                                         uint64_t &snap_end_seqno)
index 7777b5a..85a1de3 100644 (file)
@@ -147,6 +147,14 @@ class FailoverTable {
 
     void replaceFailoverLog(uint8_t* bytes, uint32_t length);
 
+    /**
+     * Returns total number of entries in the failover table. These entries
+     * represent a branch
+     *
+     * @return total number of entries
+     */
+    size_t getNumEntries() const;
+
  private:
 
     bool loadFromJSON(cJSON *json);
index 16e5dbb..6352e21 100644 (file)
@@ -247,6 +247,45 @@ static void test_pop_5_failover_log() {
     cb_assert(rollback_seqno == 0);
 }
 
+static void test_add_entry() {
+    /* Capacity of max 10 entries */
+    const int max_entries = 10;
+    FailoverTable table(max_entries);
+
+    /* Add 4 entries with increasing order of seqno */
+    const int low_seqno = 100, step = 100;
+    for (int i = 0; i < (max_entries/2); ++i) {
+        table.createEntry(low_seqno + i * step);
+    }
+
+    /* We must have all the entries we added + one default entry with seqno == 0
+       that was added when we created failover table */
+    cb_assert((max_entries/2 + 1) == table.getNumEntries());
+
+    /* Add an entry such that low_seqno < seqno < low_seqno + step.
+       Now the table must have only 3 entries: 0, low_seqno, seqno */
+    table.createEntry(low_seqno + step - 1);
+    cb_assert(3 == table.getNumEntries());
+}
+
+static void test_max_capacity() {
+    /* Capacity of max 5 entries */
+    const int max_entries = 5;
+    FailoverTable table(max_entries);
+
+    const int low_seqno = 100, step = 100;
+    for (int i = 0; i < max_entries + 2; ++i) {
+        table.createEntry(low_seqno + i * step);
+    }
+    const int max_seqno = low_seqno + (max_entries + 1) * step;
+
+    /* Expect to have only max num of entries */
+    cb_assert(max_entries == table.getNumEntries());
+
+    /* The table must remove entries in FIFO */
+    cb_assert(table.getLatestEntry().by_seqno == max_seqno);
+}
+
 int main(int argc, char **argv) {
     (void)argc;
     (void)argv;
@@ -256,5 +295,7 @@ int main(int argc, char **argv) {
     test_pop_5_failover_log();
     test_5_failover_largeseqno_log();
     test_edgetests_failover_log();
+    test_add_entry();
+    test_max_capacity();
     return 0;
 }