jihuayu commented on code in PR #3541:
URL: https://github.com/apache/kvrocks/pull/3541#discussion_r3689269865
##########
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:
cc @git-hulk @PragmaTwice
Do you think this is a good design? My concern is that, with this approach,
all the affected keys would need to be propagated from the storage layer up to
the command layer, and every storage-layer method would need an additional
field for them.
Would it be better to emit the event immediately after the deletion is
performed in the storage layer, rather than propagating the information all the
way up to the command layer?
--
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]
