Page MenuHomePhabricator

[services] Tunnelbroker - Add `createdAt` timestamp to the `messageItem`
ClosedPublic

Authored by max on Apr 21 2022, 5:00 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 3, 9:31 PM
Unknown Object (File)
Tue, Oct 29, 2:10 AM
Unknown Object (File)
Mon, Oct 21, 12:24 PM
Unknown Object (File)
Mon, Oct 21, 12:23 PM
Unknown Object (File)
Mon, Oct 21, 12:22 PM
Unknown Object (File)
Mon, Oct 21, 12:22 PM
Unknown Object (File)
Oct 7 2024, 11:29 PM
Unknown Object (File)
Oct 7 2024, 11:29 PM

Details

Summary

While DynamoDB doesn't preserve the insert records order, we can sort the message records by the timestamp of its creation and create
a rule for sorting them inside the DynamoDB to preserve the delivery FIFO order.

Linear task: ENG-1027

Test Plan

Passed test at D3806.
Run yarn test-tunnelbroker-service all tests are run and succeed.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek requested changes to this revision.Apr 21 2022, 8:40 AM

Can't we just sort by message id? Is there a reason to avoid that?

Could two different messages have the same timestamp?

This revision now requires changes to proceed.Apr 21 2022, 8:40 AM
In D3805#105329, @palys-swm wrote:

Can't we just sort by message id? Is there a reason to avoid that?
Could two different messages have the same timestamp?

We use UUID-based messageID on the randomly generated string because the messageID must be unique - we will remove the delivered message by the unique id.
The message's timestamps theoretically can be the same in case they were sent at the same time. We can also use an atomic counter, but its realization in dynamoDB is not good.
I think using the timestamp (second or millisecond) is a better idea here.

tomek added a reviewer: ashoat.

Ok, that makes sense. But I'm not sure if this will work when there are multiple instances of the service. Do we need the clocks to be closely synchronized? Or maybe, we don't need to keep an order of messages handled by different instances?

Overall, this looks ok, but adding @ashoat to get his opinion

In D3805#105355, @palys-swm wrote:

Ok, that makes sense. But I'm not sure if this will work when there are multiple instances of the service. Do we need the clocks to be closely synchronized? Or maybe, we don't need to keep an order of messages handled by different instances?

Overall, this looks ok, but adding @ashoat to get his opinion

Adding more context here:
We are using the timestamp to preserve the order of the messages. There is only another option with the atomic counter, we can consider it too. In DynamoDB we should maintain this counter and read/increment it. DynamoDB doesn't have an auto-increment (( The records order preservation is not guaranteed either they are sorted somehow.

This makes sense to me.

Do we need the clocks to be closely synchronized?

Ideally, yes. But it's not the end of the world if they aren't perfectly synchronized, as long as everybody agrees on the order of the events.

Or maybe, we don't need to keep an order of messages handled by different instances?

I do think this will be necessary. If User A and User B send User C a message at the same time, those two messages will both get queued from User A's and User B's Tunnelbroker instances.

This revision is now accepted and ready to land.Apr 22 2022, 6:07 AM