feat(ws): simplify events for web sockets queue for performance #4152
Conversation
NV-2824 WebSockets: Simplify the job events created when sending an InApp notification
What?Send one single event adding one single job to the WebSockets queue instead of 3 as we do right now. Swift the logic around in ExternalServicesRoute: to check if the user is online and if not not to emit any of the current 3 events for an InApp notification sent. If user is online we can send the 3 events: Unseen, Unread and Received. Why? (Context)Right now we are sending 3 events when sending an InApp message: This seems non performant as it triples the usage of the resources for the Definition of Done
|
| @@ -247,7 +247,7 @@ export class SendMessageInApp extends SendMessageBase { | |||
| ); | |||
|
|
|||
| await this.webSocketsQueueService.bullMqService.add( | |||
| `sendMessage-received-${message._id}`, | |||
| message._id, | |||
There was a problem hiding this comment.
We will use the messageId as the jobId now we will have an unique event sent, therefore one unique job created.
|
|
||
| constructor(private wsGateway: WSGateway, private messageRepository: MessageRepository) { | ||
| this.bullMqService = new BullMqService(); |
There was a problem hiding this comment.
This wasn't used. The dependency with BullMQ is attached in the WebSocketWorker in this same app.
| if (command.event === WebSocketEventEnum.UNSEEN) { | ||
| await this.sendUnseenCountChange(command); | ||
| } | ||
|
|
||
| return; | ||
| if (command.event === WebSocketEventEnum.UNREAD) { | ||
| await this.sendUnreadCountChange(command); |
There was a problem hiding this comment.
Headless sends this events to request the count changes, so keeping them.
Though would appreciate a second opinion if they are needed and we should keep it.
There was a problem hiding this comment.
The headless and notification-center actually are listening for these events. The events are triggered when the messages are marked as read/seen through the API or the notification-center.
| if (command.event === WebSocketEventEnum.UNSEEN) { | ||
| await this.sendUnseenCountChange(command); | ||
| } | ||
|
|
||
| return; | ||
| if (command.event === WebSocketEventEnum.UNREAD) { | ||
| await this.sendUnreadCountChange(command); |
There was a problem hiding this comment.
The headless and notification-center actually are listening for these events. The events are triggered when the messages are marked as read/seen through the API or the notification-center.
p-fernandez
deleted the
nv-2824-websockets-simplify-the-job-events-created-when-sending-an
branch
What change does this PR introduce?
From the Worker we will only send one event instead of 3 and we will process it executing the functions that before were triggered by 3 different events, as they are sent at the same time.
We also only process the event and trigger the messages to the WebSocket if the user is online as if not it is a waste of resources.
Why was this change needed?
It is a performance improvement identified during the MemoryDB migration.
Other information (Screenshots)