[services] Tunnelbroker - Adding delivering of undelivered messages from the database to the messages stream
ClosedPublic
Actions

Authored by • max on Nov 6 2022, 6:08 AM.

Details

Reviewers

• jon
varun
marcin
tomek

Commits

rCOMM18ef8636ed87: [services] Tunnelbroker - Adding delivering of undelivered messages from the…

Summary

This diff adds delivery of the undelivered messages from the database to the client when connecting to the bidirectional stream.
When the client is offline the messages to deliver are stored in the database. Right after the client is online and connected to the messages bidirectional stream we will deliver messages from the database to the client as messagesToDeliver.

The client will process messages and send back processed messages IDs to the Tunnelbroker as processedMessages only after that, we will delete messages from the database.

Removing of processed messages implemented in D5584.

Linear task: ENG-2060

Test Plan

Insert a message into the messages table with the corresponding deviceID in the toDeviceID field.
Connect by the gRPC client to the bidirectional stream with the sessionID corresponding to the deviceID in metadata.

The expected result is that the message from the database would be delivered to the client as the MessagesToDeliver.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• max created this revision.Nov 6 2022, 6:08 AM

• max held this revision as a draft.

Herald added a reviewer: • jon. · View Herald TranscriptNov 6 2022, 6:08 AM

Herald added subscribers: • abosh, atul, tomek, ashoat. · View Herald Transcript

• max added a parent revision: D5536: [services] Tunnelbroker - Adding of requesting a new notification token if empty.Nov 6 2022, 6:09 AM

Harbormaster failed remote builds in B13190: Diff 18096!Nov 6 2022, 6:17 AM

Rebasing on parent changes.

• max retitled this revision from [services] Tunnelbroker - Adding delivering undelivered messages from the database in gRPC to [services] Tunnelbroker - Adding delivering of undelivered messages from the database to the messages stream.Nov 9 2022, 2:26 AM

• max removed a reviewer: • jon.

Herald added a reviewer: • jon. · View Herald TranscriptNov 9 2022, 2:26 AM

Harbormaster completed remote builds in B13303: Diff 18251.Nov 9 2022, 2:39 AM

• max added a child revision: D5580: [services] Tunnelbroker - Adding delivering messages from the deliveryBroker to the messages stream.Nov 9 2022, 3:28 AM

• max edited the summary of this revision. (Show Details)Nov 9 2022, 7:02 AM

• max edited the test plan for this revision. (Show Details)

• max added reviewers: varun, marcin. • max added 1 blocking reviewer(s): • jon.

• max edited the summary of this revision. (Show Details)Nov 9 2022, 7:12 AM

• max mentioned this in D5584: [services] Tunnelbroker - Removing processed messages.Nov 9 2022, 7:21 AM

• max published this revision for review.Nov 9 2022, 7:24 AM

• jon accepted this revision.Nov 9 2022, 11:03 AM

This revision is now accepted and ready to land.Nov 9 2022, 11:03 AM

• max added a reviewer: tomek.Nov 10 2022, 2:16 AM

This revision now requires review to proceed.Nov 10 2022, 2:16 AM

tomek requested changes to this revision.Nov 10 2022, 8:54 AM

tomek added inline comments.

services/tunnelbroker/src/server/mod.rs
161 ↗	(On Diff #18251)	Could you explain why we're deleting all the messages from AMQP here?
176–182 ↗	(On Diff #18251)	It seems like this message can get pretty large. Should we split the payload into multiple messages?

This revision now requires changes to proceed.Nov 10 2022, 8:54 AM

• max requested review of this revision.Nov 16 2022, 3:59 AM

• max marked 2 inline comments as done.

• max added inline comments.

services/tunnelbroker/src/server/mod.rs
161 ↗	(On Diff #18251)	Could you explain why we're deleting all the messages from AMQP here? We are cleaning the queue for the certain `deviceID` exactly before starting to send the messages from the database to avoid the condition when the new AMQP message can be pushed during the database lookups and writing messages to the stream. If we are cleaning up the queue after the database and writing to the stream but before listening to the new messages we can lose this message. If we are doing this before we are not because when the client sends a message we are pushing it into the database first and then pushing the AMQP message. So we are sure that the message is in the database in we will consume it from the database anyway in case of this condition.
176–182 ↗	(On Diff #18251)	It seems like this message can get pretty large. Should we split the payload into multiple messages? We are storing blobs outside of the gRPC messages itself so there is highly unlikely that we are exceeding the 4Mb gRPC messages limit. But we can make this check. I've created a follow-up task. Thanks, @tomek !