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

dynamic subscription mode breaks remote socket.emit() #524

Closed
MartinKolarik opened this issue Feb 13, 2024 · 3 comments · Fixed by #525
Closed

dynamic subscription mode breaks remote socket.emit() #524

MartinKolarik opened this issue Feb 13, 2024 · 3 comments · Fixed by #525

Comments

@MartinKolarik
Copy link
Contributor

MartinKolarik commented Feb 13, 2024

When using the sharded adapter with subscriptionMode: "dynamic", messages sent to the socket's private room via socket.emit() get lost without any warning or error.

It happens because in this check, such messages look just like any other single-room broadcasts, so useDynamicChannel ends up being true but private rooms are excluded here so there are no subscribers.

I can see a couple of possible solutions here, but none is ideal:

  1. Attempt to identify the channel is private during emit, and set useDynamicChannel to false in such case. I made an attempt on this in fix(sharded): private room broadcast in dynamic mode #525 but it seems somewhat tricky.
  2. Clearly document that socket.emit() won't work in this mode.
  3. Remove the exception for private rooms and subscribe to them as well.
  4. Combination of 2 and 3 - add an option for subscribing to private rooms and document socket.emit() won't work without it.
@MartinKolarik
Copy link
Contributor Author

MartinKolarik commented Feb 19, 2024

Upon further consideration, I think, regardless of this issue, it makes sense to have an option for creating separate channels for private rooms.

In our scenario, we have ~1k clients connected to ~10 servers, all communication is 1:1, and the connections stay open for a long time. Using separate channels significantly lowers the overall redis bandwidth, and the subscription/unsubscription impact shouldn't be significant. We could create our own "public" channel for each client, but using the existing ones seems like the cleanest option.

I implemented this, along with the previous fix, in #526. We're already running this in production, and it reduced the overall redis load by 50 - 60 % in our setup.

@darrachequesne
Copy link
Member

Hi! I could indeed reproduce the issue, thanks for reporting this.

This happens when calling io.to(someSocketId).emit("hello"); (which is also what happens with a RemoteSocket, under the hood). Classic socket.emit()'s are not affected though.

@MartinKolarik
Copy link
Contributor Author

Classic socket.emit()'s are not affected though.

By this, you mean local emits? If yes, that's right, it only affects emits on remote sockets.

@MartinKolarik MartinKolarik changed the title dynamic subscription mode breaks socket.emit() dynamic subscription mode breaks remote socket.emit() Feb 23, 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 a pull request may close this issue.

2 participants