feat: a scheduled backup that fails without leaving a backup is retried once #144
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/retry-a-failed-nightly-backup-once"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What's in
DOUJIN_MANAGER_BACKUP_RETRY_AFTER_MINUTESlater (new variable; default60,0disables; whole minutes0–1439— a non-number, a negative value or a day or more fails the container even while the schedule isoff). While the retry is pending,GET /api/backups/schedulereports it asnextRunAtunder the failedlastRun, so the app's schedule card reads "Next run: 04:30" under the red banner. The DTO keeps its shape; no OpenAPI change.skippedoutcome is not retried, and neither is a run that took its backup but could not rotate (failedwith a file name — the backup is on the volume). Never more than two attempts per occurrence, at most one backup from them.deploy/DEPLOYMENT.md§2 and §6.7, set to60indeploy/docker-compose.yml, defaulted inappsettings.json(Backups:RetryAfterMinutes), and named in the startup log line; one Information line announces "Scheduled backup failed; retrying at ".ScheduledBackupOutcome.Retryable(whether:failedand no file) andBackupSchedule.RetryAfterFailureAt(when:nullat delay 0 or when the next occurrence is not strictly later, compared as instants so the DST gap needs no special case).ScheduledBackupService.RunOnceWithRetryAsynccalls the guarded run at most twice and never looks at the second outcome, so the one-retry cap is structural, not a counter.Verification
scripts/verify.sh backend, run from the worktree root atb770ec7and watched to completion: Release build 0 errors; ApplicationCore.Tests 56/56, IntegrationTests 1/1, Infrastructure.Tests 210/210, RestAdapter.Tests 678/678;== verify OK (backend) ==, exit 0.BackupScheduleTests:RetryAfterFailureAtdelay, strict next-occurrence bound, spring-forward gap, zero delay,Retryabletruth table); host service (ScheduledBackupServiceTests, via a scripted run double and counting clock: retry fires at exactly T+delay and not a minute early, sameonlyIfDueas the original run, no third attempt, sooner occurrence wins,skippedand rotation failures excluded, delay 0 disables, startup line and "retrying at" line); configuration (ScheduledBackupConfigurationTests: default 60, env var as whole minutes, empty = unset, appsettings binding and env override, non-number refused inResolve, negative and ≥1440 refused inParseeven with the schedule off, 1439 accepted).nextRunAtcarrying the retry instant is pinned at the journal in the service tests, not through a TestHost request toGET /api/backups/schedule. Persistence: untouched by this branch. UI: untouched; the card showing the retry as "Next run" follows from the unchanged DTO and was confirmed by reading only.dotnet test→ restore; all went red): (1) retry removed → 3 service tests red; (2)skippedmade retryable →Only_A_Failed_Run_That_Left_No_Backup_Is_Retryable,A_Skipped_Catch_Up_Is_Not_Retriedred; (3) second retry allowed →A_Failing_Run_Is_Logged_As_An_Error_The_Host_Survives_And_The_Schedule_Holdsred; (4) next-occurrence bound removed → 4 red (3 schedule tests +A_Failed_Run_Waits_For_A_Sooner_Scheduled_Occurrence_Instead_Of_Retrying); (5) rotation-failure gate removed → 2 red; (6) delay 0 ignored → 3 red; (7) config checks removed → 7 theory rows red; (8) retryonlyIfDuehardcoded false →A_Failed_Catch_Up_Is_Retried_As_A_Catch_Upred; (9) retry wait replaced by a fixed minute →A_Failed_Scheduled_Run_Is_Retried_Once_An_Hour_Later_And_The_Schedule_Resumesred; (10) startup-line retry phrase inverted → 2 red.b770ec7: polled log assertion instead of a bare assert one statement after the journal write, locked log enumeration, ADR promise reduced to what is pinned plus the ≥1440 refusal, ADR tunables table): (P1) guarded run's failure line LogError→LogWarning → red; (P2) retry announcement printed in UTC → red; (P3) ≥1440 check neutralised → 4 red. An independent re-probe reproduced P1–P3 and added (P4)>=→>→ only the 1440 row andThe_Longest_Retry_Delay_Is_A_Minute_Short_Of_A_Dayred; (P5) day constant 1440→1439 → onlyThe_Longest…red; (P6)RetryAfter = retryAfterdropped from the schedule-off return inParse→ stays green — not a branch and not observable (a disabled schedule never readsRetryAfter), so it is noted, not pinned.README.mdwas not touched; its backup bullet does not mention the retry.Follow-ups
nextRunAtmoves to the retry instant but nothing on the wire says it is a retry; a nullableattemptonlastRunor anextRunIsRetryflag would (left off because the DTO shape was frozen for #139's card and the log line carries the fact).🤖 Generated with Claude Code
Summary
Summary
Coverage
DoujinManager.ApplicationCore - 94.2%
DoujinManager.Infrastructure - 96.4%
on
pshot
Series
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__CoverImageHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__PrimaryTitleHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__RolledBackImageCleanup
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__UploadChapterValidator
DoujinManager.RestAdapter - 94.4%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 64.6%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A retry that is bounded four ways and each bound is a promise a test pins~? ADR amendment with strikethrough honesty, a structural one-retry cap instead of a counter, DST handled by comparing instants so the gap "needs no special case"... I read this diff twice and the second time was just for pleasure. ♡
Verdict: ✅ Looks good to me~
✅ What I liked~
ScheduledBackupOutcome.Retryable(whether:failed&&FileName is null) andBackupSchedule.RetryAfterFailureAt(when:nullat delay 0 or when the next occurrence is not strictly later) — and the host service stays exactly the scheduler it claims to be.RunOnceWithRetryAsynccallsRunGuardedAsyncat most twice and nothing reads the second outcome: the cap is structural. That is the right kind of clever. ♪return;inserted before the retry logic → exactly 3 service tests red; (2)&& FileName is nulldropped fromRetryable→ 1 service test red plus the ApplicationCore truth-table row your probe counted. Red on exactly the tests that should be red — these are directional, not tautologies.b770ec7: ApplicationCore 56/56, Infrastructure 210/210, RestAdapter 674 passed + 4 skipped = 678 (0 failed; the 4 skips are the pre-existing platform-conditionalLinuxFact/file-mode rows, Support/ untouched by this PR), Integration 1/1 — matching your PR-body arithmetic exactly.BackupScheduleDto.Frommaps field-by-field, so the newRetryAfter/RetriesFailedRunsmembers on the domain record cannot leak onto the wire;backend/openapi/v1.jsonis unchanged in the diff and the app model already bindsnextRunAt. The schedule card showing the retry as "Next run" really does follow.Parserunning unconditionally in Program.cs so a bad value fails the container even with the schedule off (pinned byA_Negative_Retry_Delay_Fails_Even_When_The_Schedule_Is_Off), 1439/1440 boundary pinned both ways, and the ≥1440 refusal reasoning ("a setting that silently never fires") written into the ADR. fufu~ you even refused to let a dead setting live. ♡POST /api/backupsin between makes the retry skip) is pinned withOnlyIfDueasserted per call.💡 Little ideas (non-blocking)~
Automated review by Jibril · 2026-09-18
CI/CD: build ✓ + test ✓ for head
b770ec7; deploy-smoke ✗ — pre-existing on every PR and main for months, no CI files touched here, not this PR's fault · Local checks: full backend suite green + 2 independent mutation probes red-on-target