WIP: keyspace notifications including cluster support #2995
+1,829
−307
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Keyspace notifications are poorly supported; here we extend this
keyspaceandkeyeventmessages, which have non-trivial semantics21: new
KeyNotificationAPIreadonly struct KeyNotificationthat wraps a channel and value, and exposes.Database,.Type,.GetKey(), etc - and the raw.Channeland.Valuefor unknown message types; overall usage is shown inKeyNotificationTests.cs.enum KeyNotificationTypefor the expected values (KeyNotification.Type, elseKeyNotificationType.Unknown)ChannelMessagetype gains a newTryParseKeyNotificationAPI, "do we recognize this as aKeyNotification?"The
ChannelMessageandKeyNotificationtypes are very similar; I considered just adding the new members toChannelMessage, but IMOKeyNotificationdeserves calling out into a dedicated type.Note that
keyspaceandkeyeventmessages have different semantics for where the type and key are located;KeyNotificationhides that detail, providing a single API that works for both scenarios. The individual values are parsed lazily in the accessors - they are not pre-computed; so if nobody accesses.Database: it is never parsed from the channel, etc. Internally, the[FastHash]approach is used for parsing.Type, since there is not a direct map between the enum names and the raw expected values (the enums follow .NET conventions; the raw values have a mixture of underscores, hyphens, etc - plus different capitalization); this is handled in the internalKeyNotificationTypeFastHash, with extensive unit testing.2: new API for subscribing to keyspace/keyevent channels
Proposed API:
public class RedisChannel { + // single exact key, database is required, always uses SUBSCRIBE + // example "SUBSCRIBE __keyspace@42__:mykey` + // note that this scenario can use single-node key-like routing + public static RedisChannel KeySpace(RedisKey key, int database) + + // key pattern, database optional, interpreted as "any" when omitted, always uses PSUBSCRIBE + // example: "PSUBSCRIBE __keyspace@42__:customer/*" + // example: "PSUBSCRIBE __keyspace@*__:customer/*" + public static RedisChannel KeySpacePattern(RedisKey pattern, int? database = null) + + // type-based, database optional, interpreted as "any" when omitted; uses PSUBSCRIBE when database omitted + // example: "SUBSCRIBE __keyevent@42__:del" + // example: "PSUBSCRIBE __keyevent@*__:del" + public static RedisChannel KeyEvent(KeyNotificationType type, int? database = null); }Additionally, note that the
WithKeyRouting()method intentionally throws if used with any of the channels that would use multi-node semantics, as key-routing and multi-node are fundamentally incompatible.No explicit fully-open API (i.e. all-keys keyspace or all-type keyevent) is provided; this is intentional; we should discourage this for performance/impact reasons, but this is covered indirectly via
KeyPattern("*", null), which allows the usage without making it a too-obvious default.When
dbisnull, it is interpreted as "any database". We intentionally do not provide simplified access to the default database as a substituted value - this is because we know that active:active is on the horizon, and that gets into very ugly overlap with pub/sub if different config groups have different default databases. If we want to support this scenario (for example, so that negative values are interpreted as the default database), we should probably enforce, in active:active, that all config groups have the same effective default database.3: internal changes to support cluster routing
(incomplete) The key challenge here is that the events are routed to all primaries; this contrasts with the current scenarios:
var channel = RedisChannel.Literal("abc");- single-node, arbitrary routing, usesSUBSCRIBEandPUBLISHvar channel = RedisChannel.Pattern("abc*");- single-node, arbitrary routing, usesPSUBSCRIBEandPUBLISH.WithKeyRouting();- same, but routed according to channel as a keyvar channel = RedisChannel.Sharded("abc");- single node, routed according to channel as a key, usesSSUBSCRIBEandSPUBLISHThis will demand a new model for the initial subscription, and subscription maintenance over both reconnects and topology changes, where the semantics are
SUBSCRIBEorPSUBSCRIBE(whichever is more appropriate), but multi-node routing. We should not support or allow any publish API, since these events are server-originated.