Pub/Sub: default to 3.0, fix PING, fix server selection in cluster, and cleanup #1958
Conversation
…nd cleanup In prep for changes to how we handle subscriptions internally, this does several things: - Upgrades default Redis server assumption to 3.x - Routes PING on Subscription keepalives over the subscription bridge appropriately - Fixes cluster sharding from default(RedisKey) to shared logic for RedisChannel as well (both in byte[] form) - General code cleanup in the area (getting a lot of DEBUG/VERBOSE noise into isolated files)
| public Version DefaultVersion | ||
| { | ||
| get => defaultVersion ?? (IsAzureEndpoint() ? RedisFeatures.v4_0_0 : RedisFeatures.v2_8_0); | ||
| get => defaultVersion ?? (IsAzureEndpoint() ? RedisFeatures.v4_0_0 : RedisFeatures.v3_0_0); |
There was a problem hiding this comment.
So that we go with PING off the bat
| @@ -0,0 +1,50 @@ | |||
| using System; | |||
There was a problem hiding this comment.
These are method moves - no functional changes
| @@ -0,0 +1,59 @@ | |||
| using System; | |||
There was a problem hiding this comment.
These are method moves - no functional changes
| { | ||
| if (condition) OnTraceWithoutContext(message, category); | ||
| } | ||
|
|
There was a problem hiding this comment.
Moved to another partial
| if (server != null && message != null) | ||
| { | ||
| var bs = server.GetBridgeStatus(message.Command); | ||
| var bs = server.GetBridgeStatus(message.IsForSubscriptionBridge ? ConnectionType.Subscription: ConnectionType.Interactive); |
There was a problem hiding this comment.
Turns out we were also getting the wrong error stats, for the command we sent to the wrong place - fixing that.
|
Note: pub/sub failing tests are because subscriptions don't do awesome things, and we're not masking that - the tests are correct, overall implementation and our |
|
@mgravell ^ this is the cheese move (tests appropriately failing) |
| /// </summary> | ||
| /// <param name="channel">The <see cref="RedisChannel"/> to determine a slot ID for.</param> | ||
| public int HashSlot(in RedisChannel channel) | ||
| => ServerType == ServerType.Standalone || channel.IsNull ? NoSlot : GetClusterSlot((byte[])channel); |
There was a problem hiding this comment.
TODO Marc: consider this in my "don't use byte[] for GetClusterSlot rework"
mgravell
left a comment
mgravell
left a comment
There was a problem hiding this comment.
looks good; some side thoughts
In prep for changes to how we handle subscriptions internally, this does several things: