-
Notifications
You must be signed in to change notification settings - Fork 1.6k
keyspace notifications including cluster support #2995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
e9a8023 to
1a571c0
Compare
- KeyNotification wraps channel+value, exposes friendly parsed members (db, type, etc) - KeyNotificationType is new enum for known values - add TryParseKeyNotification help to ChannelMessage (and use explicit fields)
1a571c0 to
21ddfa9
Compare
|
@philon-msft as a progress update: "making progress here". Locally, successfully observing both event and key events, in both standalone and cluster scenarios. CI is less happy, and still some connection management bits to polish, but: fundamentally we can see the notifications. I've also validated callback-style vs queue-style, and validated "alt-lookup" usage on .NET 9+ (for efficient handling in cache-server scenarios). |
|
@philon-msft if you get a sec, can you eyeball the updated doc (linked above)? in particular, paying attention to the new section on keyspace and channel isolation - sound workable? |
- EnsureSubscribedToServer[Async] can now *remove* subscriptions in the multi-node case
…s not routed to the channel, and can use primary or replica, so Ping is not a reliable test *unless* we demand master
|
I think this is functionally complete now, just some trailing CI-fighting. Primary ref should be docs linked in top post. |
- prevent concurrency problem on TextWriterOutputHelper
Note that related to this change we also do:
Overall usage is described in the new/proposed docs entry
the docs above are probably the most useful way of getting an idea for the feature at the consumer level
Additional: this PR fixes a pre-existing bug where "key-routed" channel commands (
SSUBSCRIBE, andSUBSCRIBEwith opt-in key-like routing), when combined withChannelPrefix, were sent to the wrong node; the hash-slot calculation did not account forChannelPrefix. The impact of this was minor, hence it not being previously reported:SSUBSCRIBE, it was silently corrected via a-MOVED, at the expense of an extra hopSSUBSCRIBEwith opt-in key-like routing, it had no real consequence in most cases (the channels were still distributed, just: distributed subtly differently), although it does now mean that if multiple prefix-isolated clients each use a single channel: those channels are now balanced rather than colocatedThe existing tests were enhanced to force and validate this behaviour.
Keyspace notifications are poorly supported; here we extend this
keyspaceandkeyeventmessages, which have non-trivial semantics1: new
KeyNotificationAPIreadonly struct KeyNotificationthat wraps a channel and value, and exposes.Database,.Type,.GetKey(), etc - and access to the raw channel and payload for 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?"Additionally, note that it is expected that cache-invalidation will be a key scenario; as such, this type plays well with alt-lookup APIs in .NET 9+, with integration tests that demonstrate using the span-based API (rather than sub-string etc).
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.This APIs is discussed in more detail in the documentation linked above.
2: new API for subscribing to keyspace/keyevent channels
The API changes for this feature consist of:
RedisChannelinstancesKeyNotificationAPI that is used when consuming events (via the existing pub/sub consumer API), providing access to the component parts of the messagesThese APIs are discussed in more detail in the documentation linked above.
3: internal changes to support cluster routing
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, usesSSUBSCRIBEandSPUBLISHTo implement this, the
Subscriptiontype that underpins how subscriptions are stored is overhauled; instead of a singlesealed class Subscription, we move to anabstract class Subscription(fortunately the type isinternal), with concrete subclasses, using polymorphism to respond appropriately to subscribe/unsubscribe operations4: Overlap with channel and keyspace isolation
The nature of the keyspace / keyevent channels is that they are determined by the server; as such, they are incompatible with the
ChannelPrefixfeature (configured at the muxer/options level) . Consequently, keyspace / keyevent channels ignore theChannelPrefixfeature, and are processed directly. Explicitly:Keyspace isolation is configered at the
IDatabaselevel; since the database and pub/sub APIs are independent, there is no way for the key notification events to know about and account for any keyspace isolation that may be applied by arbitrary consumers. As such, it is left for the caller to account for keyspace isolation, when either constructing channels or accessing the keys from individual events.These decisions are listed the documentation linked above.
5: Subscription management during connection lifetime
In the existing code, there are 2 main reset paths:
OnFullyEstablished(when a connection fails and reconnects)ReconfigureAsync(when there is a topology change)Both of these end up calling into
Subscription.EnsureSubscriptions, which would connect as needed.We add changes:
EnsureSubscribedToServernow, in the multi-node case, has the ability to remove a subscription if it should no longer be connected; this basically covers nodes switching between primary/replica