Page MenuHomePhabricator

[Backup] Old backup cleanup
ClosedPublic

Authored by michal on Aug 28 2023, 2:38 AM.
Tags
None
Referenced Files
F2019516: D8969.diff
Sun, Jun 16, 9:47 AM
Unknown Object (File)
Wed, Jun 12, 7:44 AM
Unknown Object (File)
Wed, Jun 12, 6:37 AM
Unknown Object (File)
Tue, Jun 11, 8:49 PM
Unknown Object (File)
Mon, Jun 10, 9:55 AM
Unknown Object (File)
Sat, Jun 8, 2:51 AM
Unknown Object (File)
Sun, May 26, 9:26 AM
Unknown Object (File)
Sun, May 26, 9:26 AM
Subscribers

Details

Summary

ENG-4501

For the initial version of the backup service we only want to keep one backup for every user. In this diff we remove all old backups after every new backup is successfuly uploaded.

Depends on D8967

Test Plan

Run integration tests

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek added inline comments.
services/backup/src/database/mod.rs
182–183

Are we sure this will always skip the latest one? We could consider explicitly sorting the collection or (IMO better) finding max(created_at) and skipping

186–195

Are we going to replace this with the batch_write helper? (D8895)

IIRC @varun had some issues with writing reserved usernames in a loop, but these were larger quantities than this

services/backup/src/database/mod.rs
182–183

From query documentation: "Query results are always sorted by the sort key value"

...although later in the same paragraph they say "To reverse the order, set the ScanIndexForward parameter to false.", which wasn't correct in my experience (in both cases the order was the same). I will implement your second suggestion.

186–195

We need the returned values to cleanup user_data (it's only in the full backup table and not in index). Barring any errors, there should always be only one backup (the previous one) that is being deleted anyway.

bartek added inline comments.
services/backup/src/database/mod.rs
182–183

...although later in the same paragraph they say "To reverse the order, set the ScanIndexForward parameter to false.", which wasn't correct in my experience (in both cases the order was the same). I will implement your second suggestion.

Yes, that's why I'm asking

This revision is now accepted and ready to land.Aug 29 2023, 2:44 AM

Rebase, improve how we skip the latest backup when removing old backups (search for the backup with max creation time)