feat(keyspace): add keyspace notification support for replicas - #3607
feat(keyspace): add keyspace notification support for replicas#3607Aetherance wants to merge 1 commit into
Conversation
|
cc @jihuayu @git-hulk @PragmaTwice, if you have time. |
|
The CI failed, but it looks like a flaky test. |
| std::vector<rocksdb::Status> getRawValues(engine::Context &ctx, const std::vector<Slice> &keys, | ||
| std::vector<std::string> *raw_values); | ||
| rocksdb::Status updateRawValue(engine::Context &ctx, const std::string &ns_key, const std::string &raw_value); | ||
| rocksdb::Status updateRawValue(engine::Context &ctx, const std::string &ns_key, const std::string &raw_value, |
There was a problem hiding this comment.
Hi @Aetherance.
We already declare the keyspace event in the existing code. Why do we need to maintain another separate set of event-related logic here?
Could we make each operation declare the event only once, and let a unified batch / commit path generate the LogData required for replication and handle the local notification after the write is committed successfully? This would avoid maintaining the semantics of the same event separately in the local notification and replication paths.
There was a problem hiding this comment.
This is because we need to reuse the existing PutLogData logic in updateRawValue. Since updateRawValue is shared by multiple write operations, the lower layer cannot determine which command was executed unless the specific event type is passed down.
There was a problem hiding this comment.
I think it is reasonable to keep this logic here because this LogData may also be inspected by code paths other than the keyspace notification mechanism. Therefore, it should not be merged into keyspace-event-specific logic.
There was a problem hiding this comment.
Additionally, this API:
rocksdb::Status String::updateRawValue(
engine::Context &ctx,
const std::string &ns_key,
const std::string &raw_value)is used only by string operations, and all string operation requires only a single change, so this will not result in widespread API changes. For commands such as DEL, only one line of code needs to be changed.
In #3541, we introduced keyspace notifications for the primary node. This PR extends that support to replicas.
We previously discussed the replica-side implementation in Discussion #3533 We hope that discussion provides additional context and helps reviewers better understand the changes introduced in this PR.
Background:
Kvrocks uses WAL-based replication. Unlike Redis, Kvrocks does not forward the commands executed on the primary to replicas verbatim. Instead, a replica receives a RocksDB
WriteBatchand applies it locally. However, to correctly support keyspace notifications on replicas, the replica must know which events occurred, and this information is currently unavailable to it.Therefore, this PR propagates the necessary information through
WriteBatchLogData. Emitting a keyspace notification on a replica generally requires four pieces of information: the event name, event type, namespace, and user key.The existing
LogDatamechanism already carries the event type. However, because the event name cannot be derived from the event type alone, the replica still needs three additional pieces of information: the event name, namespace, and user key.This PR does not store all three pieces of information in
LogData. The event name can be derived by introducing a more specific command identifier. For example,kRedisCmdSetmaps to"set", whilekRedisCmdDelmaps to"del". The namespace and user key are already encoded in the internal keys of subsequent Metadata CF operations, so storing them again would be redundant.On the replica side, records in the
WriteBatchare processed in their original order. The current command context is first obtained fromLogData. The namespace and user key are then extracted from the subsequentPutCForDeleteCFoperation. These pieces of information are finally combined into a complete keyspace event.Because the
LogDatarecord and its corresponding data mutation reside in the sameWriteBatch, this association preserves both operation ordering and atomicity. It also correctly handles transaction batches containing multiple commands.With this approach, the PR avoids redundantly storing the event name, namespace, or user key in the WAL. The only significant addition is the specific command identifier. The notification categories, as well as whether an event is published to the keyspace channel, the keyevent channel, or both, are determined by the replica’s own
notify-keyspace-eventsconfiguration.Assisted by GPT-5.6.