Page MenuHomePhabricator

[services] Tunnelbroker - Message expiration fulfill moved to database manager's insert
ClosedPublic

Authored by max on Apr 21 2022, 8:29 AM.
Tags
None
Referenced Files
F2787204: D3808.id11729.diff
Sat, Sep 21, 4:00 AM
F2787198: D3808.id12240.diff
Sat, Sep 21, 3:55 AM
F2787116: D3808.id12204.diff
Sat, Sep 21, 3:48 AM
F2787082: D3808.id12237.diff
Sat, Sep 21, 3:46 AM
F2787050: D3808.id12219.diff
Sat, Sep 21, 3:30 AM
F2786747: D3808.diff
Sat, Sep 21, 1:27 AM
Unknown Object (File)
Tue, Sep 17, 4:01 PM
Unknown Object (File)
Tue, Sep 17, 4:00 PM

Details

Summary

Removing the expire from the constructor because we will fulfill it at the DB insertion phase at DatabaseManager's insert.

Constant with the max message TTL in the database was added.

Test Plan

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

karol added inline comments.
services/tunnelbroker/src/Constants.h
51 ↗(On Diff #11729)

This is a bit unclear to me

tomek requested changes to this revision.Apr 26 2022, 4:29 AM
tomek added inline comments.
services/tunnelbroker/src/Constants.h
51 ↗(On Diff #11729)

Agree. What is the meaning of 10 at the end?

It also looks like we were using milliseconds for other TTLs. So shouldn't this be 300 * 24 * 60 * 60 * 1000? (we also need to verify if it works correctly, as the value can't fit in 32 bits).

This revision now requires changes to proceed.Apr 26 2022, 4:29 AM

Rebase on master. Multiplication of the MESSAGE_RECORD_TTL was changed due to the comment.

The database test was updated according to the changes.

max added inline comments.
services/tunnelbroker/src/Constants.h
51 ↗(On Diff #11729)

This is a bit unclear to me

That change makes sense to me. I've updated with these changes.

51 ↗(On Diff #11729)

Agree. What is the meaning of 10 at the end?

It also looks like we were using milliseconds for other TTLs.

This TTL is used in the DynamoDB TTL index to clear the old messages by the database itself.
We must use seconds (not milliseconds) from the Unix epoch for this purpose according to the DynamoDB documentation.

max marked an inline comment as done.
services/tunnelbroker/src/Constants.h
30 ↗(On Diff #12219)

This description comment was updated to be more clear, because we added one more TTL and we need to distinguish them between each other.

ashoat removed a reviewer: tomek.
This revision is now accepted and ready to land.May 4 2022, 9:07 AM