Skip to content

feat(keyspace): add keyspace notification support for replicas - #3607

Open
Aetherance wants to merge 1 commit into
apache:unstablefrom
Aetherance:feat/keyspace-events-replica
Open

feat(keyspace): add keyspace notification support for replicas#3607
Aetherance wants to merge 1 commit into
apache:unstablefrom
Aetherance:feat/keyspace-events-replica

Conversation

@Aetherance

Copy link
Copy Markdown
Contributor

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 WriteBatch and 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 WriteBatch LogData. Emitting a keyspace notification on a replica generally requires four pieces of information: the event name, event type, namespace, and user key.

The existing LogData mechanism 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, kRedisCmdSet maps to "set", while kRedisCmdDel maps 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 WriteBatch are processed in their original order. The current command context is first obtained from LogData. The namespace and user key are then extracted from the subsequent PutCF or DeleteCF operation. These pieces of information are finally combined into a complete keyspace event.

Because the LogData record and its corresponding data mutation reside in the same WriteBatch, 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-events configuration.

Assisted by GPT-5.6.

@Aetherance

Copy link
Copy Markdown
Contributor Author

cc @jihuayu @git-hulk @PragmaTwice, if you have time.

@Aetherance

Copy link
Copy Markdown
Contributor Author

The CI failed, but it looks like a flaky test.

Comment thread src/types/redis_string.h
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,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Aetherance Aetherance Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants