Page MenuHomePhabricator

[keyserver] Add maxDirSizeMiB to BackupConfig
ClosedPublic

Authored by ashoat on Jun 15 2022, 5:41 PM.
Tags
None
Referenced Files
F3379699: D4283.diff
Wed, Nov 27, 6:33 PM
Unknown Object (File)
Sun, Nov 24, 10:55 PM
Unknown Object (File)
Sun, Nov 24, 10:55 PM
Unknown Object (File)
Fri, Nov 22, 12:39 PM
Unknown Object (File)
Fri, Nov 22, 12:39 PM
Unknown Object (File)
Fri, Nov 22, 12:39 PM
Unknown Object (File)
Fri, Nov 22, 12:39 PM
Unknown Object (File)
Fri, Nov 22, 12:38 PM
Subscribers

Details

Summary

This config allows a user to specify a maximum space usage of the backup directory. This should resolve ENG-1234.

We will always keep at least one backup, ignoring maxDirSizeMiB if the single most recent backup exceeds it. Otherwise we will delete old backups until the usage falls below the max.

Depends on D4282

Test Plan
  1. I tested setting maxDirSizeMiB to fit 2.5 backups, then changed cron.js so it ran a backup soon. I confirmed after running that there were two backups, that they were the two most recent, and that one of them was new.
  2. Then I tested setting it to some low number, smaller than one backup. Made sure after running that there was precisely one brand new backup.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Adding a minor comment about style, not sure about the rest of this revision because of lack of experience with this side of the codebase, so deferring to @atul on that. I'll investigate the backups aspect of the codebase further once he reviews it.

keyserver/src/cron/backups.js
261 ↗(On Diff #13537)

Referencing same style fix from D4282. It's clearer to name this sortedBackupInfos so that mostRecentBackup, and the backwards iteration through backupInfos makes sense.

(reviewed from phone, so didn’t look as thoroughly as usual.. trusting test plan)

This revision is now accepted and ready to land.Jun 16 2022, 8:51 AM