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

feat(instances): message queue #207

Merged
merged 10 commits into from
Oct 18, 2021
Merged

feat(instances): message queue #207

merged 10 commits into from
Oct 18, 2021

Conversation

samuelmasse
Copy link
Contributor

@samuelmasse samuelmasse commented Oct 15, 2021

This PR implements a sort of message queue mechanism for sending messages to external channels. It works by having a QueuedMessage that has a next property which is another QueuedMessage. When a message is done being sent, it will check its next property is set and send the next message right away.

Tested and working on telegram

Closes DEV-1914

@linear
Copy link

linear bot commented Oct 15, 2021

DEV-1914 Move typing indicators delay into queues

We don't want the waiting that is done in the backend for typing indicators to be blocking for anything else. To fix this we'll stop awaiting these delays and to prevent messages from all being sent at once we'll put them in queues that are scoped to a channel and a conversation


private async enqueueMessage(message: QueuedMessage) {
const tail = this.messageQueueTail[message.message.conversationId]
this.messageQueueTail[message.message.conversationId] = message
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be a queue, shouldn't the queue be an array ? Feels like if there's a couple of messages sent quickly, they would be overwritten ? You can check memory-queue.ts on the main repository, it's the queue system we use for events, it's effective and battle-tested

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing gets overwritten. The queue is a linked list. We only keep a reference to the tail of the queue, and if we want to add an item to the queue we set the next item of the tail to this new item and the new item becomes the tail of the queue. It works because the await is placed at the correct place, and node isn't multi-threaded.

}
} catch (e) {
this.logger.error(e, 'Failed to send message to instance')
this.logger.error(e, 'Failed to run message queue')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is an error with the messaging queue, let's say in the middle of it, won't it just fail in loop every time we receive new messages for this queue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The try catch here is more to safeguard the whole server crashing since this function is not awaited. I'm not sure how a queue could persistently failed unless there's something terribly wrong with the LinkedQueue class. Otherwise there is already a try catch inside the loop itself that allows continuing in the case one of the message can't be sent.

@samuelmasse samuelmasse merged commit 335cf3c into master Oct 18, 2021
@samuelmasse samuelmasse deleted the sm-message-queue branch October 18, 2021 21:49
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 this pull request may close these issues.

None yet

3 participants