feat(ws): performance improvements #4142
Conversation
p-fernandez
force-pushed
the
feat-ws-performance-improvements
branch
from
21deab7 to
4466e50
Compare
There was a problem hiding this comment.
do we need this one if you did changes here?
https://github.com/novuhq/novu/pull/4126/files#r1319969641
p-fernandez
force-pushed
the
feat-ws-performance-improvements
branch
from
31eb912 to
9d42f8d
Compare
A change from |
| if (command.event === WebSocketEventEnum.UNSEEN) { | ||
| await this.sendUnseenCountChange(command); | ||
| } | ||
|
|
||
| if (command.event === WebSocketEventEnum.UNREAD) { | ||
| await this.sendUnreadCountChange(command); | ||
| } |
There was a problem hiding this comment.
Is this still needed or it's for backwards compatible version of a worker? Since if we now only send the RECEIVED those will never run?
There was a problem hiding this comment.
I mentioned it here: #4152 (comment)
Headless and Notification Center still use those events. @LetItRock confirmed.
There was a problem hiding this comment.
They used to listen to them, but the only producer of those is the worker from my memory. After your refactoring they will still get them because of line https://github.com/novuhq/novu/pull/4142/files#diff-69a5fd44e678889585214e635e81e988c3a95a541e36d30df6de70d7e345bcd3R47
So those lines I'm seeing here should only executed if the worker still sends the old event names. If that makes sense
There was a problem hiding this comment.
Here they are used as events are being sent:
There was a problem hiding this comment.
Very good Catch! Totally forgot about it 💪
What change does this PR introduce?
Integrates two tasks to improve the performance of the WebSockets.
Why was this change needed?
Performance improvements identified while migrating MemoryDB.
Other information (Screenshots)