jihuayu commented on code in PR #3541:
URL: https://github.com/apache/kvrocks/pull/3541#discussion_r3689384634
##########
src/commands/cmd_key.cc:
##########
@@ -372,9 +374,17 @@ class CommandDel : public Commander {
uint64_t cnt = 0;
redis::Database redis(srv->storage, conn->GetNamespace());
- auto s = redis.MDel(ctx, keys, &cnt);
+ const bool notify_del = GetAttributes()->name == "del" &&
conn->IsKeyspaceEventEnabled(kNotifyGeneric);
+ std::vector<rocksdb::Slice> deleted_keys;
+ if (notify_del) deleted_keys.reserve(keys.size());
+
+ auto s = redis.MDel(ctx, keys, &cnt, notify_del ? &deleted_keys : nullptr);
if (!s.ok()) return {Status::RedisExecErr, s.ToString()};
+ for (const auto &key : deleted_keys) {
+ conn->AddKeyspaceEvent(kNotifyGeneric, "del",
std::string_view(key.data(), key.size()));
+ }
+
Review Comment:
What I mean is, could we emit the event here instead of propagating it up to
the command layer?
https://github.com/apache/kvrocks/pull/3541/changes#diff-df4860055937f1bb78946c49a63a6c4120812a1ec6229a5ded3aa3536220c253R210
Please don’t rush to make changes. I’d like to hear what others think first.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
