Page MenuHomePhabricator

[Backup] Old backup cleanup
ClosedPublic

Authored by michal on Aug 28 2023, 2:38 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Oct 30, 4:50 PM
Unknown Object (File)
Sun, Oct 27, 5:02 PM
Unknown Object (File)
Sun, Oct 27, 5:02 PM
Unknown Object (File)
Sun, Oct 27, 5:02 PM
Unknown Object (File)
Sun, Oct 27, 5:02 PM
Unknown Object (File)
Sun, Oct 27, 5:01 PM
Unknown Object (File)
Sat, Oct 12, 10:02 AM
Unknown Object (File)
Oct 7 2024, 11:49 PM
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 ↗(On Diff #30427)

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 ↗(On Diff #30427)

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 ↗(On Diff #30427)

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 ↗(On Diff #30427)

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 ↗(On Diff #30427)

...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)