Skip to content
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

[Epic] SignalR Redis improvements #27583

Open
BrennanConroy opened this issue Nov 6, 2020 · 10 comments
Open

[Epic] SignalR Redis improvements #27583

BrennanConroy opened this issue Nov 6, 2020 · 10 comments
Labels
area-signalr Includes: SignalR clients and servers cost: L Will take from 5 - 10 days to complete enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-major This label is used by an internal tool
Milestone

Comments

@BrennanConroy
Copy link
Member

BrennanConroy commented Nov 6, 2020

@BrennanConroy BrennanConroy added the area-signalr Includes: SignalR clients and servers label Nov 6, 2020
@BrennanConroy BrennanConroy added this to the Next sprint planning milestone Nov 6, 2020
@ghost
Copy link

ghost commented Nov 6, 2020

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@BrennanConroy BrennanConroy added affected-few This issue impacts only small number of customers enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-major This label is used by an internal tool labels Nov 6, 2020 — with ASP.NET Core Issue Ranking
@BrennanConroy BrennanConroy added the Epic Groups multiple user stories. Can be grouped under a theme. label Nov 23, 2020
@ghost
Copy link

ghost commented Jul 29, 2021

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@rafikiassumani-msft rafikiassumani-msft added the cost: L Will take from 5 - 10 days to complete label Nov 8, 2021
@rafikiassumani-msft rafikiassumani-msft removed the affected-few This issue impacts only small number of customers label Jan 14, 2022
@rafikiassumani-msft rafikiassumani-msft changed the title SignalR Redis improvements [Epic] SignalR Redis improvements Jan 25, 2022
@sikri-eic
Copy link

Figure out if Clustering works or what it would take to make it work

Our experience shows that Redis clustering (with Azure Redis Cache) does not work (of course, it's possible that we configured something wrong). So, we had to move our SignalR app to a non-clustered Redis cache apart from all the others using the Redis cluster (which is not a great way to scale and is not cost-effective). So, I'm very interested in learning what your findings (and solution) on this will be. Subscribing...

@ghost
Copy link

ghost commented Oct 11, 2022

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@BrennanConroy
Copy link
Member Author

Tested Redis Clustering with Azure Redis Cache. No additional configuration or code changes were needed on the server, just the connection string like usual.

Tested with 2 servers on separate shards, one client on each server.
Tested that broadcast to all clients works, and that sending to a specific connection works (this hits the "group" subscription code path).
Also, forced 1 shard (primary and replica) to restart, server automatically reconnected and maintained it's subscriptions, tested via broadcast and send to specific client again.

@sikri-eic
Copy link

@BrennanConroy Thank you for testing this. We could not get Redis clustering to work on Azure Cache for Redis. I cannot remember what sort of errors/problems we got.

No additional configuration or code changes were needed on the server, just the connection string like usual.

This is interesting. Our connection string is in the following format as of now: <url>:<port>,password=<password>,ssl=True,abortConnect=False

I wonder if this has anything to do with the problems we have experienced.

Anyway, I will retest this as soon as I can.

@BrennanConroy
Copy link
Member Author

Our connection string is in the following format as of now: <url>:<port>,password=<password>,ssl=True,abortConnect=False

That's a normal connection string.

@sikri-eic
Copy link

That's what I thought too... Will test as soon as I can. Thanks.

@tomap
Copy link

tomap commented Nov 20, 2022

Hello,

My understanding was that the commands used for redis are publish and subscribe which are not spread across nodes of a redis cluster.
So I'm not sure to understand how the current implementation of backplane on redis is compatible with redis clusters.

Redis 7 introduced ssubscribe and spublish which are compatible with redis clusters. (Sharded)

But maybe there is something I did not understand in either redis clusters or SignalR backplane?

@BrennanConroy
Copy link
Member Author

I think this is the key paragraph:

Sharded Pub/Sub helps to scale the usage of Pub/Sub in cluster mode. It restricts the propagation of message to be within the shard of a cluster. Hence, the amount of data passing through the cluster bus is limited in comparison to global Pub/Sub where each message propagates to each node in the cluster. This allows users to horizontally scale the Pub/Sub usage by adding more shards.

So these new Redis commands help performance, but that doesn't mean Sharding will not work with SignalR today.

@adityamandaleeka adityamandaleeka removed the Epic Groups multiple user stories. Can be grouped under a theme. label Jan 5, 2024
@BrennanConroy BrennanConroy modified the milestones: .NET 8 Planning, Backlog Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-signalr Includes: SignalR clients and servers cost: L Will take from 5 - 10 days to complete enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-major This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests

7 participants