Added support for bulk join and leave #873
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We have a large number of rooms a socket client can subscribe to, and when a client connects to our server, we pull all the rooms they were previously subscribed to from our backend and issue a join request for all the rooms, this also ends up issuing a publish on pubsub (redis in our case) for JOIN / LEAVE events This is causing high CPU spikes Added a bulk JOIN / LEAVE API to allow the above use case to be handled in a more efficient manner for us. This will allow client to join a large number of rooms at the same time without causing a lot of resource strain Bump version Fix - Changed join to leave in leaveRooms Fix - Using Sets instead of List, as it makes more sense semantically here Cleaned up imports/formatting Update serialVersionUID
|
Thanks for contribution! |
ksahu750
pushed a commit
to ksahu750/netty-socketio
that referenced
this pull request
Aug 2, 2023
We were on a rather old version of this library, so ran into mrniko#668, saw the issue was open so tried a fix, which worked for us Checked the later version and found that this was fixed in commit 452870d, but the issue with the current approach is it issues leave event one by one for all the joined rooms, which puts a lot of stress on pubsub store if there are a lot of joined rooms. Due to the above issue we had high CPU spikes, and had added this feature mrniko#873, a little while back. Thinking of leveraging that here to send leave event for all the joined rooms at once, to reduce the stress on pubsub store. Few possible solutions, that I can think of 1. Do a bulk leave beforehand, as joinedRooms set gets modified on leaving room The obvious problem here that I do see is we send pubsub event before actually leaving the rooms on this node. 2. Copy joinedRooms and do bulk leave after leaving Has a memory overhead for copying the joinedRooms set 3. Have a separate event altogether like LEAVE_ALL (essentially same as DISCONNECT) 4. Currently, the default DISCONNECT subscription we have in BaseStoreFactory is just a debug log, we can handle it there itself, but only sessionId is propagated there, namespace is not, we can either iterate all namespaces, or pass joined namespaces of the client in that packet. Would like to know your opinion, or if there are better suggestions.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have a large number of rooms a socket client can subscribe to, and when a client connects to our server, we pull all the rooms they were previously subscribed to from our backend and issue a join request for all the rooms, this also ends up issuing a publish on pubsub (async wrapper over RedissonPubSubStore in our case) for JOIN / LEAVE events
This is causing high CPU spikes
Added a bulk JOIN / LEAVE API to allow the above use case to be handled in a more efficient manner for us. This will allow client to join a large number of rooms at the same time without causing a lot of resource strain