also looking at this
fix(db): bound on-demand database dumps, and publish them atomically
#200
2 files
+188
-25
| @@ -4,16 +4,19 @@ import { | ||
| 4 | 4 | buildBackupScript, |
| 5 | 5 | buildCreateDatabaseScript, |
| 6 | 6 | buildCreateUserScript, |
| 7 | buildListBackupsScript, | |
| 7 | 8 | buildListScript, |
| 8 | 9 | buildVitessListScript, |
| 10 | DEFAULT_DB_BACKUP_RETENTION, | |
| 9 | 11 | isExternalEngine, |
| 10 | 12 | isValidDbIdentifier, |
| 11 | parseVitessTopology, | |
| 12 | shardsMissingPrimary, | |
| 13 | unhealthyTablets, | |
| 14 | 13 | parseBackups, |
| 15 | 14 | parseDbList, |
| 15 | parseVitessTopology, | |
| 16 | resolveDbBackupRetention, | |
| 16 | 17 | resolveDbEngine, |
| 18 | shardsMissingPrimary, | |
| 19 | unhealthyTablets, | |
| 17 | 20 | } from './dashboard-database' |
| 18 | 21 | |
| 19 | 22 | describe('resolveDbEngine', () => { |
| @@ -149,10 +152,12 @@ describe('buildListScript + parseDbList', () => { | ||
| 149 | 152 | expect(script.filter(line => line.includes('$(date'))).toHaveLength(1) |
| 150 | 153 | expect(script.some(line => line.startsWith('TS_CLOUD_BACKUP_STAMP='))).toBe(true) |
| 151 | 154 | |
| 152 | // The write, the report and the listing must all name the same thing. | |
| 153 | const named = script.filter(line => line.includes('acme-')) | |
| 155 | // The publish, the report and the listing must all name the same thing. | |
| 156 | const named = script.filter(line => line.includes('acme-$TS_CLOUD_BACKUP_STAMP.sql.gz')) | |
| 154 | 157 | expect(named).toHaveLength(3) |
| 155 | for (const line of named) expect(line).toContain('acme-$TS_CLOUD_BACKUP_STAMP.sql.gz') | |
| 158 | // And the scratch file is named once and consistently: written to, | |
| 159 | // renamed from, removed on failure. | |
| 160 | expect(script.filter(line => line.includes('.acme-$TS_CLOUD_BACKUP_STAMP.part'))).toHaveLength(3) | |
| 156 | 161 | |
| 157 | 162 | // And the stamp is set before anything uses it. |
| 158 | 163 | expect(script.findIndex(l => l.startsWith('TS_CLOUD_BACKUP_STAMP='))) |
| @@ -161,6 +166,91 @@ describe('buildListScript + parseDbList', () => { | ||
| 161 | 166 | }) |
| 162 | 167 | |
| 163 | 168 | |
| 169 | /** | |
| 170 | * Every on-demand dump used to be kept forever: `buildBackupScript` wrote a | |
| 171 | * timestamped file and nothing pruned the directory, so each dashboard | |
| 172 | * "Backup" click left a full dump on the box. The scheduled path has honoured | |
| 173 | * `retentionCount`/`retentionDays` all along; this path had no bound at all. | |
| 174 | */ | |
| 175 | describe('on-demand dump retention', () => { | |
| 176 | it('keeps the newest N dumps of that database and drops the rest', () => { | |
| 177 | const script = buildBackupScript('mysql', 'acme', '/var/backups/ts-cloud/databases', undefined, { count: 3 }).join('\n') | |
| 178 | // Newest first, so `+4` is everything past the newest 3. The dump just | |
| 179 | // taken is newest, so it always survives. | |
| 180 | expect(script).toContain('ls -1t /var/backups/ts-cloud/databases/acme-*.sql.gz 2>/dev/null | tail -n +4 | xargs -r rm -f') | |
| 181 | }) | |
| 182 | ||
| 183 | it('drops dumps older than the configured age', () => { | |
| 184 | const script = buildBackupScript('mysql', 'acme', '/var/backups/ts-cloud/databases', undefined, { days: 7 }).join('\n') | |
| 185 | expect(script).toContain('-name "acme-*.sql.gz" -mtime +7 -delete') | |
| 186 | }) | |
| 187 | ||
| 188 | it('defaults to a bound rather than to unbounded', () => { | |
| 189 | const script = buildBackupScript('mysql', 'acme').join('\n') | |
| 190 | expect(script).toContain(`tail -n +${DEFAULT_DB_BACKUP_RETENTION.count + 1}`) | |
| 191 | expect(script).toContain(`-mtime +${DEFAULT_DB_BACKUP_RETENTION.days} -delete`) | |
| 192 | }) | |
| 193 | ||
| 194 | /** A shared box holds every database's dumps in one directory. */ | |
| 195 | it('never prunes another database\'s dumps', () => { | |
| 196 | const script = buildBackupScript('mysql', 'acme').join('\n') | |
| 197 | for (const line of script.split('\n').filter(l => l.includes('rm -f') || l.includes('-delete'))) | |
| 198 | expect(line).toContain('acme-') | |
| 199 | }) | |
| 200 | ||
| 201 | it('can be turned off for a caller that owns the directory itself', () => { | |
| 202 | const script = buildBackupScript('postgres', 'acme', '/tmp', undefined, false).join('\n') | |
| 203 | expect(script).not.toContain('-delete') | |
| 204 | expect(script).not.toContain('tail -n +') | |
| 205 | }) | |
| 206 | ||
| 207 | /** | |
| 208 | * `runDb` runs these under `set -uo pipefail` with NO `-e`, so a failed dump | |
| 209 | * does not stop the script — and `gzip` exits 0 even when its upstream died. | |
| 210 | * If retention ran anyway it would count a 20-byte gzip header as the newest | |
| 211 | * backup and delete a good one to make room: adding a bound would then | |
| 212 | * destroy data that accumulating never did. | |
| 213 | */ | |
| 214 | it('prunes nothing when the dump fails', () => { | |
| 215 | const script = buildBackupScript('mysql', 'acme') | |
| 216 | const elseAt = script.indexOf('else') | |
| 217 | expect(elseAt).toBeGreaterThan(-1) | |
| 218 | const failureBranch = script.slice(elseAt).join('\n') | |
| 219 | expect(failureBranch).not.toContain('-delete') | |
| 220 | expect(failureBranch).not.toContain('tail -n +') | |
| 221 | // And it reports the failure rather than exiting 0 with no dump. | |
| 222 | expect(failureBranch).toContain('exit 1') | |
| 223 | }) | |
| 224 | ||
| 225 | /** | |
| 226 | * The stamp is only second-granular. Redirecting straight to the final name | |
| 227 | * opens and truncates it before the dump is known to work, so a failed retry | |
| 228 | * within the same second as a good dump destroyed that dump. Reproduced | |
| 229 | * before the fix. | |
| 230 | */ | |
| 231 | it('publishes by rename, so a failed retry cannot clobber a good dump', () => { | |
| 232 | const script = buildBackupScript('mysql', 'acme') | |
| 233 | const dumpLine = script.find(l => l.startsWith('if '))! | |
| 234 | // The dump writes to the scratch name, never to the published one. | |
| 235 | expect(dumpLine).toContain('.acme-$TS_CLOUD_BACKUP_STAMP.part"') | |
| 236 | expect(dumpLine).not.toContain('acme-$TS_CLOUD_BACKUP_STAMP.sql.gz') | |
| 237 | expect(script.join('\n')).toContain('mv -f "/var/backups/ts-cloud/databases/.acme-$TS_CLOUD_BACKUP_STAMP.part"') | |
| 238 | // Scratch files are outside the `*.sql.gz` glob, so they are never listed | |
| 239 | // as a backup — and stale ones get reaped. | |
| 240 | expect(buildListBackupsScript()).toEqual([expect.stringContaining('*.sql.gz')]) | |
| 241 | expect(script.join('\n')).toContain('-name ".acme-*.part" -mmin +60 -delete') | |
| 242 | }) | |
| 243 | ||
| 244 | it('reads its bounds from the same config the scheduled path uses', () => { | |
| 245 | const base = { project: { name: 'acme', slug: 'acme' } } as any | |
| 246 | expect(resolveDbBackupRetention(base)).toEqual(DEFAULT_DB_BACKUP_RETENTION) | |
| 247 | expect(resolveDbBackupRetention({ | |
| 248 | ...base, | |
| 249 | infrastructure: { compute: { backups: { retentionCount: 2, retentionDays: 3 } } }, | |
| 250 | })).toEqual({ count: 2, days: 3 }) | |
| 251 | }) | |
| 252 | }) | |
| 253 | ||
| 164 | 254 | it('parses BACKUP= lines into database + file, deriving the db from the filename', () => { |
| 165 | 255 | const parsed = parseBackups( |
| 166 | 256 | 'BACKUP=/var/backups/ts-cloud/databases/acme-20260702-101500.sql.gz\nnoise\nBACKUP=/var/backups/ts-cloud/databases/blog-20260701-090000.sql.gz', |