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

Add disablePatternSubscriptions option #493

Closed

Conversation

stevebaum23
Copy link
Contributor

Turning on this option will disable the use of redis pattern subscriptions for receiving messages. All messages will be received through regular subscriptions.

Added new test scripts for running against all the redis clients with the option enabled.

@stevebaum23
Copy link
Contributor Author

To add some context to this, I opened issue #480 and worked on a solution to that. The solution was to clean up unused namespaces so that the number of pattern subscriptions doesn't continually rise. That solution has worked pretty well for us. At our peak times we have around 20k namespaces and are publishing around 25 messages per second to redis (via socket messages). That consumes around 12% of the redis engine CPU. Redis publish latency is averaging 12ms.

We still have room to scale with the current solution, but there will be some combination of # of namespaces and # of publishes per second that will bring latency to an unacceptable point. That is the purpose of this new option. Since pattern subscriptions are the main reason for the increased latency, I attempted to remove them and use standard subscriptions (which are fast).

It looks like pattern subscriptions are mainly used to reduce some CPU load when sending a message to a single room. The message handler can look at the channel name and if it includes a room, the namespace can check if it has the room before trying to decode the message. In our usage of sockets, we are not using rooms (we are using namespaces to segregate users), so this optimization doesn't help us.

Anyway, sorry for the long explanation, but I wanted to give a reasoning for this change. It's more of a proactive move. 😄

@stevebaum23 stevebaum23 force-pushed the swb/disable-pattern-subs-option branch from 4097451 to 96166ab Compare April 5, 2023 20:58
Turning on this option will disable the use of redis pattern subscriptions
for receiving messages. All messages will be received through regular
subscriptions.

Added new test scripts for running against all the redis clients with the
option enabled.
@stevebaum23 stevebaum23 force-pushed the swb/disable-pattern-subs-option branch from 96166ab to d401b9d Compare April 11, 2023 15:29
@darrachequesne
Copy link
Member

Hi @stevebaum23 , I've seen your PR, thanks a lot for your work on this 👍

I didn't merge it yet because I think we could maybe implement it differently, in order to address #492 and #486. I'm thinking about a new subscriptionMode option, whose value could be:

  • "wildcard": the current behavior, with the pattern
  • "static": what you suggest in this PR
  • "per-room": a new subscription is created for each public room

Do you think that makes sense?

@stevebaum23
Copy link
Contributor Author

That sounds fine @darrachequesne. Do what you want with this PR. 😄 It was a fairly quick attempt at not using pattern subscriptions. Thanks for looking into it!

darrachequesne added a commit that referenced this pull request May 2, 2023
The subscriptionMode option allows to configure how many Redis Pub/Sub
channels are used:

- "static": 2 channels per namespace

Useful when used with dynamic namespaces.

- "dynamic": (2 + 1 per public room) channels per namespace

The default value, useful when some rooms have a low number of clients
(so only a few Socket.IO servers are notified).

Related:

- #491
- #492
- #493
@darrachequesne
Copy link
Member

Superseded by d3388bf, released in version 8.2.0.

Thanks!

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.

2 participants