[services] Tunnelbroker - Removing processed messages
ClosedPublic
Actions

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

Details

Reviewers

• jon
varun
marcin
tomek

Commits

rCOMMd35c221aae8c: [services] Tunnelbroker - Removing processed messages

Summary

This diff adds removing of processed messages by the client from the database and handling of the incoming processedMessages gRPC message.
When the Tunnelbroker sends messages to the client by MessagesToDeliver (in D5540, D5580) we are expecting messages IDs back from the client as ProcessedMessages when they are successfully processed. After receiving the messages IDs the Tunnelbroker will remove messages from the database. Using this mechanism we are sure that the messages were successfully processed by the client before removing them.

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 message from the database would be delivered to the client as the MessagesToDeliver.
Get the messageID identifier from the delivered message.
Send back to the stream a processedMessages message with the message identifier.
The expected result is that the message with the corresponding messageID will be removed from the database in messages table.

Diff Detail

Repository

rCOMM Comm

Branch

add-grpc-messages-removing-processed

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

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

• max held this revision as a draft.

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

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

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

• max mentioned this in D5540: [services] Tunnelbroker - Adding delivering of undelivered messages from the database to the messages stream.Nov 9 2022, 7:02 AM

Harbormaster completed remote builds in B13320: Diff 18270.Nov 9 2022, 7:05 AM

• max mentioned this in D5580: [services] Tunnelbroker - Adding delivering messages from the deliveryBroker to the messages stream.Nov 9 2022, 7:12 AM

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

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

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

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

• max edited the summary of this revision. (Show Details)

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

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

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

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

Change to pass the pointer of the Vector to c++ function.

Harbormaster completed remote builds in B13353: Diff 18309.Nov 10 2022, 3:33 AM

• max added a child revision: D5592: [services] Tunnelbroker - Adding handling of messages to send from the client.Nov 16 2022, 6:50 AM

tomek accepted this revision.Nov 17 2022, 3:24 AM

tomek added inline comments.

services/tunnelbroker/src/server/mod.rs
290 ↗	(On Diff #18309)	This is slightly confusing: this parameter `messagesIDs` is a vector of ids, but we seem to pass a single id `processed_messages.message_id` to it. Should we change some names to make it consistent?

This revision is now accepted and ready to land.Nov 17 2022, 3:24 AM

• max mentioned this in rCOMM18ef8636ed87: [services] Tunnelbroker - Adding delivering of undelivered messages from the….Nov 21 2022, 5:47 AM

• max marked an inline comment as done.Nov 21 2022, 6:02 AM

• max added inline comments.

services/tunnelbroker/src/server/mod.rs
290 ↗	(On Diff #18309)	This is slightly confusing: this parameter `messagesIDs` is a vector of ids, but we seem to pass a single id `processed_messages.message_id` to it. Should we change some names to make it consistent? That came from the gRPC proto file field name. We should change it to be plural. I've created a task for this. Thanks!