Aetherance commented on PR #3541:
URL: https://github.com/apache/kvrocks/pull/3541#issuecomment-4881523873
@PragmaTwice @git-hulk Thanks for the careful review.
When I started implementing keyspace notifications, I also considered using
a single common trigger point. However, while working on SET/DEL, I found that
the generic command execution entry does not have enough semantic information
to generate correct notifications.
For example, a conditional SET may return OK without actually writing the
key, such as SET ... NX when the key already exists. DEL also needs to know the
exact keys that were actually removed; cases like DEL k k k must not emit
duplicate
notifications. These details are only known after the concrete command/type
logic evaluates the command semantics, and cannot be derived reliably from the
command name, arguments, Status, or reply at the generic execution entry.
I agree the current implementation is not scalable enough. I’ll revisit the
design and try to centralize the shared parts better. I also think it may not
be realistic to add more keyspace notification coverage without touching
individual command/type implementations at all, because some event decisions
still need to be made where the Redis semantics are fully known.
--
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]
