From a53bf6764dbf098a33c2147b00afca2b39547125 Mon Sep 17 00:00:00 2001 From: Kunal Sahu Date: Wed, 2 Aug 2023 15:22:16 +0530 Subject: [PATCH 1/2] Send Bulk leave event instead of multiple leave events on disconnect We were on a rather old version of this library, so ran into https://github.com/mrniko/netty-socketio/issues/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 452870d6, 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 https://github.com/mrniko/netty-socketio/pull/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. --- .../java/com/corundumstudio/socketio/namespace/Namespace.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/corundumstudio/socketio/namespace/Namespace.java b/src/main/java/com/corundumstudio/socketio/namespace/Namespace.java index f54610a1..00512e96 100644 --- a/src/main/java/com/corundumstudio/socketio/namespace/Namespace.java +++ b/src/main/java/com/corundumstudio/socketio/namespace/Namespace.java @@ -176,9 +176,9 @@ public void onDisconnect(SocketIOClient client) { allClients.remove(client.getSessionId()); // client must leave all rooms and publish the leave msg one by one on disconnect. + storeFactory.pubSubStore().publish(PubSubType.BULK_LEAVE, new BulkJoinLeaveMessage(client.getSessionId(), joinedRooms, getName())); for (String joinedRoom : joinedRooms) { leave(roomClients, joinedRoom, client.getSessionId()); - storeFactory.pubSubStore().publish(PubSubType.LEAVE, new JoinLeaveMessage(client.getSessionId(), joinedRoom, getName())); } clientRooms.remove(client.getSessionId()); From 6aa74dc354d07b07e914ffecc82153ebba424c1b Mon Sep 17 00:00:00 2001 From: Kunal Sahu Date: Tue, 26 Sep 2023 12:48:03 +0530 Subject: [PATCH 2/2] Copy joined rooms and send pubsub event after performing clean up locally --- .../java/com/corundumstudio/socketio/namespace/Namespace.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/corundumstudio/socketio/namespace/Namespace.java b/src/main/java/com/corundumstudio/socketio/namespace/Namespace.java index 00512e96..86cbf3ee 100644 --- a/src/main/java/com/corundumstudio/socketio/namespace/Namespace.java +++ b/src/main/java/com/corundumstudio/socketio/namespace/Namespace.java @@ -174,13 +174,14 @@ public void addDisconnectListener(DisconnectListener listener) { public void onDisconnect(SocketIOClient client) { Set joinedRooms = client.getAllRooms(); allClients.remove(client.getSessionId()); + final Set roomsToLeave = new HashSet<>(joinedRooms); // client must leave all rooms and publish the leave msg one by one on disconnect. - storeFactory.pubSubStore().publish(PubSubType.BULK_LEAVE, new BulkJoinLeaveMessage(client.getSessionId(), joinedRooms, getName())); for (String joinedRoom : joinedRooms) { leave(roomClients, joinedRoom, client.getSessionId()); } clientRooms.remove(client.getSessionId()); + storeFactory.pubSubStore().publish(PubSubType.BULK_LEAVE, new BulkJoinLeaveMessage(client.getSessionId(), roomsToLeave, getName())); try { for (DisconnectListener listener : disconnectListeners) {