MB-16181: Build SystemEvent keys with the collections separator 36/78136/12
authorJim Walker <jim@couchbase.com>
Thu, 23 Mar 2017 16:55:32 +0000 (16:55 +0000)
committerDave Rigby <daver@couchbase.com>
Tue, 23 May 2017 17:58:11 +0000 (17:58 +0000)
The keys were fixed as $collections::<event> but are now changed
so that the :: is the collections separator.

This allows code to split the event key if they wish using the
same code they would split document keys.

Change-Id: I48575d295f8c058a79cf208fe3c9d3a9b3c9ed15
Reviewed-on: http://review.couchbase.org/78136
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
src/collections/collections_types.h
src/collections/vbucket_manifest.cc
src/systemevent.cc
src/systemevent.h
tests/module_tests/collections/evp_store_collections_test.cc
tests/module_tests/systemevent_test.cc

index 3eaab17..9fd4f62 100644 (file)
@@ -30,9 +30,10 @@ static cb::const_char_buffer DefaultCollectionIdentifier(
 const char* const DefaultSeparator = "::";
 
 // SystemEvent keys
-const char* const CreateEventKey = "$collections::create:";
-const char* const DeleteEventKey = "$collections::delete:";
-const char* const SeparatorChangedKey = "$collections::separator:";
+const char* const SystemEventPrefix = "$collections";
+const char* const CreateEventKey = "create";
+const char* const DeleteEventKey = "delete";
+const char* const SeparatorChangedKey = "separator";
 
 // Couchstore private file name for manifest data
 const char CouchstoreManifest[] = "_local/collections_manifest";
index b4b862b..3d43280 100644 (file)
@@ -255,7 +255,7 @@ void Collections::VB::Manifest::changeSeparator(
     } else {
         LOG(EXTENSION_LOG_NOTICE,
             "collections: vb:%" PRIu16
-            " changing collection separator from:%s, to:%*.s",
+            " changing collection separator from:%s, to:%.*s",
             vb.getId(),
             separator.c_str(),
             int(newSeparator.size()),
@@ -333,7 +333,8 @@ std::unique_ptr<Item> Collections::VB::Manifest::createSystemEvent(
 
     auto item = SystemEventFactory::make(
             se,
-            cb::to_string(collection) + std::to_string(revision),
+            separator,
+            cb::to_string(collection) + separator + std::to_string(revision),
             getSerialisedDataSize(collection),
             seqno);
 
@@ -354,6 +355,7 @@ std::unique_ptr<Item> Collections::VB::Manifest::createSeparatorChangedEvent(
     // We serialise the state of this object  into the Item's value.
     auto item =
             SystemEventFactory::make(SystemEvent::CollectionsSeparatorChanged,
+                                     separator,
                                      {/*empty string*/},
                                      getSerialisedDataSize(),
                                      seqno);
index 332b529..a166d45 100644 (file)
 #include "dcp/response.h"
 #include "kvstore.h"
 
-std::unique_ptr<Item> SystemEventFactory::make(SystemEvent se,
-                                               const std::string& keyExtra,
-                                               size_t itemSize,
-                                               OptionalSeqno seqno) {
-    std::string key;
-    switch (se) {
-    case SystemEvent::CreateCollection: {
-        // CreateCollection SystemEvent results in:
-        // 1) A special marker document representing the creation.
-        // 2) An update to the persisted collection manifest.
-        key = Collections::CreateEventKey + keyExtra;
-        break;
-    }
-    case SystemEvent::BeginDeleteCollection: {
-        // BeginDeleteCollection SystemEvent results in:
-        // 1) An update to the persisted collection manifest.
-        // 2) Trigger DCP to tell clients the collection is being deleted.
-        key = Collections::DeleteEventKey + keyExtra;
-        break;
-    }
-    case SystemEvent::DeleteCollectionHard: {
-        // DeleteCollectionHard SystemEvent results in:
-        // 1. An update to the persisted collection manifest removing an entry.
-        // 2. A deletion of the SystemEvent::CreateCollection document.
-        // Note: uses CreateEventKey because we are deleting the create item
-        key = Collections::CreateEventKey + keyExtra;
-    }
-    case SystemEvent::DeleteCollectionSoft: {
-        // DeleteCollectionHard SystemEvent results in:
-        // 1. An update to the persisted collection manifest (updating the end
-        // seqno).
-        // 2. A deletion of the SystemEvent::CreateCollection document.
-        // Note: uses CreateEventKey because we are deleting the create item
-        key = Collections::CreateEventKey + keyExtra;
-    }
-    case SystemEvent::CollectionsSeparatorChanged: {
-        // CollectionSeparatorChanged SystemEvent results in:
-        // An update to the persisted collection manifest (updating the
-        // "separator" field) and a document is persisted.
-        // Note: the key of this document is fixed so only 1 document exists
-        // regardless of the changes made.
-        key = Collections::SeparatorChangedKey;
-        break;
-    }
-    }
+std::unique_ptr<Item> SystemEventFactory::make(
+        SystemEvent se,
+        const std::string& collectionsSeparator,
+        const std::string& keyExtra,
+        size_t itemSize,
+        OptionalSeqno seqno) {
+    std::string key = makeKey(se, collectionsSeparator, keyExtra);
 
     auto item = std::make_unique<Item>(DocKey(key, DocNamespace::System),
                                        uint32_t(se) /*flags*/,
@@ -80,6 +42,32 @@ std::unique_ptr<Item> SystemEventFactory::make(SystemEvent se,
     return item;
 }
 
+// Build a key using the separator so we can split it if needed
+std::string SystemEventFactory::makeKey(SystemEvent se,
+                                        const std::string& collectionsSeparator,
+                                        const std::string& keyExtra) {
+    std::string key = Collections::SystemEventPrefix;
+    switch (se) {
+    case SystemEvent::CreateCollection:
+    case SystemEvent::DeleteCollectionSoft:
+    case SystemEvent::DeleteCollectionHard: {
+        // These all operate on the same key
+        key += collectionsSeparator + Collections::CreateEventKey +
+               collectionsSeparator + keyExtra;
+        break;
+    }
+    case SystemEvent::BeginDeleteCollection: {
+        key += collectionsSeparator + Collections::DeleteEventKey + keyExtra;
+        break;
+    }
+    case SystemEvent::CollectionsSeparatorChanged: {
+        key += collectionsSeparator + Collections::SeparatorChangedKey;
+        break;
+    }
+    }
+    return key;
+}
+
 ProcessStatus SystemEventFlush::process(const queued_item& item) {
     if (item->getOperation() != queue_op::system_event) {
         return ProcessStatus::Continue;
index 05adcea..631e499 100644 (file)
@@ -110,9 +110,15 @@ public:
      *        the seqno value set as its bySeqno.
      */
     static std::unique_ptr<Item> make(SystemEvent se,
+                                      const std::string& collectionsSeparator,
                                       const std::string& keyExtra,
                                       size_t itemSize,
                                       OptionalSeqno seqno);
+
+private:
+    static std::string makeKey(SystemEvent se,
+                               const std::string& collectionsSeparator,
+                               const std::string& keyExtra);
 };
 
 enum class ProcessStatus { Skip, Continue };
index a652769..f640021 100644 (file)
@@ -861,16 +861,19 @@ TEST_F(CollectionsDcpTest, test_dcp_separator_many) {
     // The replica should have the :: separator
     EXPECT_EQ("::", replica->lockCollections().getSeparator());
 
-    // Now step the producer to transfer the separator
-    EXPECT_EQ(ENGINE_WANT_MORE, producer->step(producers.get()));
+    std::array<std::string, 3> expectedData = {{"@@", ":", ","}};
+    for (auto expected : expectedData) {
+        // Now step the producer to transfer the separator
+        EXPECT_EQ(ENGINE_WANT_MORE, producer->step(producers.get()));
 
-    // The producer should now have the new separator
-    EXPECT_EQ(",", producer->getCurrentSeparatorForStream(vbid));
+        // The producer should now have the new separator
+        EXPECT_EQ(expected, producer->getCurrentSeparatorForStream(vbid));
 
-    // The replica should now have the new separator
-    EXPECT_EQ(",", replica->lockCollections().getSeparator());
+        // The replica should now have the new separator
+        EXPECT_EQ(expected, replica->lockCollections().getSeparator());
+    }
 
-    // Now step the producer to transfer the collection
+    // Now step the producer to transfer the create "meat"
     EXPECT_EQ(ENGINE_WANT_MORE, producer->step(producers.get()));
 
     // Collection should now be live on the replica with the final separator
index 5de8839..6a5d3c5 100644 (file)
@@ -21,6 +21,7 @@
 
 TEST(SystemEventTest, make) {
     auto value = SystemEventFactory::make(SystemEvent::CreateCollection,
+                                          "::",
                                           "SUFFIX",
                                           0,
                                           OptionalSeqno(/*none*/));
@@ -31,6 +32,7 @@ TEST(SystemEventTest, make) {
     EXPECT_EQ(-1, value->getBySeqno());
 
     value = SystemEventFactory::make(SystemEvent::CreateCollection,
+                                     "::",
                                      "SUFFIX",
                                      100,
                                      OptionalSeqno(/*none*/));
@@ -41,6 +43,7 @@ TEST(SystemEventTest, make) {
 
     value = SystemEventFactory::make(SystemEvent::BeginDeleteCollection,
                                      "SUFFIX",
+                                     "::",
                                      100,
                                      OptionalSeqno(/*none*/));
     EXPECT_EQ(queue_op::system_event, value->getOperation());
@@ -50,6 +53,7 @@ TEST(SystemEventTest, make) {
 
     value = SystemEventFactory::make(SystemEvent::BeginDeleteCollection,
                                      "SUFFIX",
+                                     "::",
                                      100,
                                      OptionalSeqno(122));
     EXPECT_EQ(queue_op::system_event, value->getOperation());