Skip to content

Improve SignalR Redis Cluster section #28310

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

Merged
merged 8 commits into from
Feb 9, 2023
Merged

Conversation

VilleKylmamaa
Copy link
Contributor

@VilleKylmamaa VilleKylmamaa commented Feb 5, 2023

SignalR Redis backplane documentation informs about the trade-off that increasing the number of nodes in a Redis Cluster decreases the message throughput of the backplane (source video, and source slides).

Also mention that in the SignalR app, all the Redis nodes should be included in the Redis connection string or added as EndPoints in the ConfigurationOptions object.

Reference issue: dotnet/aspnetcore#46408

Copy link
Collaborator

@guardrex guardrex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @VilleKylmamaa ... I'll perform an edit pass after technical review. Done!

@guardrex guardrex self-assigned this Feb 5, 2023
Removed Redis Sentinel suggestion because of uncertainty whether it is supported. You can trigger Sentinel mode in StackExchange.Redis by adding a service name to the connection string (https://stackexchange.github.io/StackExchange.Redis/Configuration), but I am unsure if it works with SignalR.
@VilleKylmamaa VilleKylmamaa changed the title Improve SignalR Redis backplane high availability section Improve SignalR Redis Cluster section Feb 9, 2023
@VilleKylmamaa
Copy link
Contributor Author

VilleKylmamaa commented Feb 9, 2023

I removed the Redis Sentinel suggestion because of uncertainty whether it is supported out of the box. You can trigger Sentinel mode in StackExchange.Redis by adding a service name to the connection string (https://stackexchange.github.io/StackExchange.Redis/Configuration), but I am unsure if it works with SignalR. I failed to make it work properly in quick testing. It should be confirmed to work if it is to be added in the documentation.

@guardrex
Copy link
Collaborator

guardrex commented Feb 9, 2023

@VilleKylmamaa ... See if my updates broke any of the meaning.

@VilleKylmamaa
Copy link
Contributor Author

@guardrex The edits are good 👍

@guardrex
Copy link
Collaborator

guardrex commented Feb 9, 2023

Thanks @VilleKylmamaa ... We'll stand-by for a bit until @BrennanConroy is free to take a look.

@guardrex
Copy link
Collaborator

guardrex commented Feb 9, 2023

Let's see how this looks ...

There's a tradeoff between the number of nodes in the cluster and the throughput of the backplane. Increasing the number of nodes increases the availability of the cluster but decreases the throughput because messages must be transmitted to all of the nodes in the cluster.

Copy link
Collaborator

@guardrex guardrex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

We made it! Thanks @VilleKylmamaa! 🚀

Thanks, @BrennanConroy! 🎸

@guardrex guardrex merged commit 1e17378 into dotnet:main Feb 9, 2023
Donciavas pushed a commit to Donciavas/AspNetCore.Docs that referenced this pull request Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants