[Backup] Old backup cleanup
ClosedPublic
Actions

Authored by michal on Aug 28 2023, 2:38 AM.

Details

Reviewers

bartek
• patryk
kamil
• jon

Commits

rCOMM53fa1c4eaf6e: [Backup] Old backup cleanup

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

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

michal created this revision.Aug 28 2023, 2:38 AM

Herald added a reviewer: • jon. · View Herald TranscriptAug 28 2023, 2:38 AM

Herald added subscribers: tomek, ashoat. · View Herald Transcript

Harbormaster completed remote builds in B22161: Diff 30427.Aug 28 2023, 2:55 AM

michal requested review of this revision.Aug 28 2023, 2:55 AM

bartek added a subscriber: varun.Aug 28 2023, 6:05 AM

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

michal added inline comments.Aug 28 2023, 6:32 AM

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 accepted this revision.Aug 29 2023, 2:44 AM

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