Page MenuHomePhabricator

[services] Tunnelbroker - Add remove MessageItems older than checkpoint database method
AbandonedPublic

Authored by max on Jun 6 2022, 6:46 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Sep 27, 12:23 AM
Unknown Object (File)
Fri, Sep 27, 12:23 AM
Unknown Object (File)
Fri, Sep 27, 12:23 AM
Unknown Object (File)
Fri, Sep 27, 12:22 AM
Unknown Object (File)
Fri, Sep 27, 12:19 AM
Unknown Object (File)
Wed, Sep 25, 7:17 AM
Unknown Object (File)
Sun, Sep 22, 3:12 AM
Unknown Object (File)
Sat, Sep 21, 11:08 PM

Details

Reviewers
karol
tomek
Summary

This diff introduces a database method to remove the message items from the database that are older than checkpoint time for the certain deviceID.

Linear task: ENG-1303

Test Plan

Successfully run test plan on D4221

Diff Detail

Repository
rCOMM Comm
Branch
add-messages-remove-by-time-method
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

max held this revision as a draft.
max published this revision for review.Jun 6 2022, 7:13 AM
max edited the summary of this revision. (Show Details)
max edited the test plan for this revision. (Show Details)
max added reviewers: karol, tomek.
tomek requested changes to this revision.Jun 7 2022, 2:52 AM
tomek added inline comments.
services/tunnelbroker/src/Database/DatabaseManager.cpp
212–222

This is inefficient - we have a lot of requests to the db. Is there a way to delete all the items that match a condition? (something like delete from ... where ... in SQL)

This revision now requires changes to proceed.Jun 7 2022, 2:52 AM

Switching to using batch deletion instead of one by one.

max added inline comments.
services/tunnelbroker/src/Database/DatabaseManager.cpp
212–222

This is inefficient - we have a lot of requests to the db. Is there a way to delete all the items that match a condition? (something like delete from ... where ... in SQL)

Unfortunately, we can't delete items from DynamoDB by anything except the primary key, which is a messageID in our case.
We need first to fetch these IDs and then remove items by them. But we can use a batch write to remove the items by batch. It's more efficient to put and delete items by batch than one by one.

I've updated this diff to use the batch write inner function from D4373.

tomek requested changes to this revision.Jun 28 2022, 4:06 AM
tomek added inline comments.
services/tunnelbroker/src/Database/DatabaseManager.cpp
214 ↗(On Diff #13868)

We use uint64_t createdAt in MessageItem - can we use it also here?

212–222

According to https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_DeleteItem.html

Deletes a single item in a table by primary key.

But also

You can perform a conditional delete operation that deletes the item if it exists, or if it has an expected attribute value.

Could you check if conditional delete can be used to avoid fetching all the messages? I think we could specify partition key and condition for the database to delete all the items that have this key and match condition. Fetching and filtering on services side is really inefficient.

If it isn't possible, what should we change about our data model to allow it?

This revision now requires changes to proceed.Jun 28 2022, 4:06 AM
max marked 2 inline comments as done.

Rebase on the parent changes. Change checkpoint variable type to uint64_t.

max added inline comments.
services/tunnelbroker/src/Database/DatabaseManager.cpp
214 ↗(On Diff #13868)

We use uint64_t createdAt in MessageItem - can we use it also here?

Yes, sure. It was changed.

This diff is canceled due to the architecture changes in ENG-1158 (comment):
We've moved from time-based checkpointing to the messageID-based and removing by a batch in D4476 and ENG-1361.