MB-45670: Revert "MB-45505: VB::Filter 'uid' clean-up"

This reverts commit 7efc1df. Reason for revert: Reverting patch as this has broken multiple DCP clients when sending stream requests with just a collection manifest uid (MB-45670 & MB-45673). Change-Id: Id64c932ea14af9c4408252659b5f23e6fc068cc2 Reviewed-on: http://review.couchbase.org/c/kv_engine/+/151130 Reviewed-by: Dave Rigby <[email protected]> Reviewed-by: Daniel Owen <[email protected]> Tested-by: Richard de Mellow <[email protected]>
couchbase · Apr 15, 2021 · 8723f19 · 8723f19
1 parent bfd3953
commit 8723f19
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 4 deletions.
diff --git a/engines/ep/src/collections/vbucket_filter.cc b/engines/ep/src/collections/vbucket_filter.cc
@@ -113,6 +113,14 @@ std::pair<cb::engine_errc, uint64_t> Filter::constructFromJson(
         }
     }
 
+    const auto uidObject = json.find(UidKey);
+    // Check if a uid is specified and parse it
+    if (uidObject != json.end()) {
+        auto jsonUid = cb::getJsonObject(
+                json, UidKey, UidType, "Filter::constructFromJson");
+        uid = makeUid(jsonUid.get<std::string>());
+    }
+
     const auto scopesObject = json.find(ScopeKey);
     const auto collectionsObject = json.find(CollectionsKey);
     if (scopesObject != json.end()) {
@@ -154,10 +162,10 @@ std::pair<cb::engine_errc, uint64_t> Filter::constructFromJson(
         }
     }
 
-    // The input JSON must of contained at least a sid, scope, or
+    // The input JSON must of contained at least a sid, uid, scope, or
     // collections key
-    if (collectionsObject == json.end() && scopesObject == json.end() &&
-        streamIdObject == json.end()) {
+    if (uidObject == json.end() && collectionsObject == json.end() &&
+        scopesObject == json.end() && streamIdObject == json.end()) {
         throw cb::engine_error(
                 cb::engine_errc::invalid_arguments,
                 "Filter::constructFromJson no sid, uid, scope or "
@@ -333,16 +341,20 @@ bool Filter::processScopeEvent(const Item& item) {
     }
 
     // scope filter we check if event matches our scope
+    // passthrough we check if the event manifest-id is greater than the clients
     if (scopeID || passthrough) {
         ScopeID sid = 0;
+        ManifestUid manifestUid{0};
 
         if (!item.isDeleted()) {
             auto dcpData = VB::Manifest::getCreateScopeEventData(
                     {item.getData(), item.getNBytes()});
+            manifestUid = dcpData.manifestUid;
             sid = dcpData.metaData.sid;
         } else {
             auto dcpData = VB::Manifest::getDropScopeEventData(
                     {item.getData(), item.getNBytes()});
+            manifestUid = dcpData.manifestUid;
             sid = dcpData.sid;
 
             if (sid == scopeID) {
@@ -411,6 +423,10 @@ void Filter::addStats(const AddStatFn& add_stat,
             add_casted_stat(buffer, scopeIsDropped, add_stat, c);
         }
 
+        checked_snprintf(
+                buffer, bsize, "%s:filter_%d_uid", prefix.c_str(), vb.get());
+        add_casted_stat(buffer, getUid(), add_stat, c);
+
         checked_snprintf(
                 buffer, bsize, "%s:filter_%d_sid", prefix.c_str(), vb.get());
         add_casted_stat(buffer, streamId.to_string(), add_stat, c);
@@ -428,6 +444,13 @@ void Filter::addStats(const AddStatFn& add_stat,
     }
 }
 
+std::string Filter::getUid() const {
+    if (uid) {
+        return std::to_string(*uid);
+    }
+    return "none";
+}
+
 cb::engine_errc Filter::checkPrivileges(
         gsl::not_null<const void*> cookie,
         const EventuallyPersistentEngine& engine) {
@@ -490,6 +513,7 @@ void Filter::dump() const {
 // To use the keys in json::find, they need to be statically allocated
 const char* Filter::CollectionsKey = "collections";
 const char* Filter::ScopeKey = "scope";
+const char* Filter::UidKey = "uid";
 const char* Filter::StreamIdKey = "sid";
 
 std::ostream& operator<<(std::ostream& os, const Filter& filter) {
@@ -505,6 +529,9 @@ std::ostream& operator<<(std::ostream& os, const Filter& filter) {
         os << ", lastCheckedPrivilegeRevision: "
            << filter.lastCheckedPrivilegeRevision.value();
     }
+    if (filter.uid) {
+        os << ", uid:" << *filter.uid;
+    }
 
     os << ", sid:" << filter.streamId;
     os << ", filter.size:" << filter.filter.size() << std::endl;

diff --git a/engines/ep/src/collections/vbucket_filter.h b/engines/ep/src/collections/vbucket_filter.h
@@ -180,6 +180,8 @@ class Filter {
         return filter.begin()->first;
     }
 
+    std::string getUid() const;
+
     cb::mcbp::DcpStreamId getStreamId() const {
         return streamId;
     }
@@ -290,6 +292,7 @@ class Filter {
 
     Container filter;
 
+    std::optional<Collections::ManifestUid> uid;
     std::optional<ScopeID> scopeID;
     // use an optional so we don't use any special values to mean unset
     std::optional<uint32_t> lastCheckedPrivilegeRevision;
@@ -304,6 +307,7 @@ class Filter {
     // keys and types used in JSON parsing
     static const char* CollectionsKey;
     static const char* ScopeKey;
+    static const char* UidKey;
     static const char* StreamIdKey;
 };
 

diff --git a/engines/ep/src/dcp/active_stream.cc b/engines/ep/src/dcp/active_stream.cc
@@ -108,12 +108,13 @@ ActiveStream::ActiveStream(EventuallyPersistentEngine* e,
 
     log(spdlog::level::info,
         "{} Creating {}stream with start seqno {} and end seqno {}; "
-        "requested end seqno was {}, collections-manifest filter:{} {}",
+        "requested end seqno was {}, collections-manifest uid:{}, filter:{} {}",
         logPrefix,
         type,
         st_seqno,
         end_seqno_,
         en_seqno,
+        filter.getUid(),
         filter.size(),
         sid);