[GitHub] [incubator-kvrocks] caipengbo commented on a diff in pull request #902: feat: impl FilterBlobByKey for SubKeyFilter

[GitBox <gi...@apache.org>]

caipengbo commented on code in PR #902:
URL: https://github.com/apache/incubator-kvrocks/pull/902#discussion_r975959389


##########
src/compact_filter.cc:
##########
@@ -117,4 +124,27 @@ bool SubKeyFilter::Filter(int level,
              << ", result: " << (result ? "deleted" : "reserved");
   return result;
 }
+
+rocksdb::CompactionFilter::Decision SubKeyFilter::FilterBlobByKey(int level,
+          const Slice& key, std::string* new_value, std::string* skip_until) const {
+  InternalKey ikey(key, stor_->IsSlotIdEncoded());
+  Metadata metadata(kRedisNone, false);
+  if (GetMetadata(ikey, &metadata)) {
+    return rocksdb::CompactionFilter::Decision::kRemove;
+  }
+  // bitmap will be checked in Filter
+  if (metadata.Type() == kRedisBitmap) {
+    return rocksdb::CompactionFilter::Decision::kUndetermined;
+  }
+
+  bool result = IsMetadataExpired(ikey, metadata);
+  DLOG(INFO) << "[compact_filter/subkey] "
+            << "namespace: " << ikey.GetNamespace().ToString()
+            << ", metadata key: " << ikey.GetKey().ToString()
+            << ", subkey: " << ikey.GetSubKey().ToString()
+            << ", verison: " << ikey.GetVersion()
+            << ", result: " << (result ? "deleted" : "reserved");

Review Comment:
   I think this log content should be distinguished with the log in Filter (line 119), although the location of the log can be determined by the line of the file.



##########
src/compact_filter.cc:
##########
@@ -108,7 +110,12 @@ bool SubKeyFilter::Filter(int level,
                                   std::string *new_value,
                                   bool *modified) const {
   InternalKey ikey(key, stor_->IsSlotIdEncoded());
-  bool result = IsKeyExpired(ikey, value);
+  Metadata metadata(kRedisNone, false);
+  if (GetMetadata(ikey, &metadata)) {
+    return true;
+  }

Review Comment:
   I don't think the name `GetMetadata` is clear, so I'm confused here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org