Skip to content

fix: scheduled forget never runs for repos that only receive external snapshots - #1285

Open
dobbry-vechur wants to merge 1 commit into
garethgeorge:mainfrom
dobbry-vechur:fix/scheduled-forget-external-snapshots
Open

fix: scheduled forget never runs for repos that only receive external snapshots#1285
dobbry-vechur wants to merge 1 commit into
garethgeorge:mainfrom
dobbry-vechur:fix/scheduled-forget-external-snapshots

Conversation

@dobbry-vechur

@dobbry-vechur dobbry-vechur commented Jul 7, 2026

Copy link
Copy Markdown

If a repo only receives snapshots created outside backrest, the scheduled forget never becomes due. My setup: rest-server on the backrest machine, laptop pushes restic backups to it, backrest handles retention with a repo-level forget policy.

External snapshots never produce an OperationBackup. The indexer records them as OperationIndexSnapshot. The foundBackup scan in Next only looked at OperationBackup, so an external-only repo always looks idle. With CLOCK_LAST_RUN_TIME that resets the schedule base to now when it sees no backups, so restarts and config changes kept pushing the run out.

Fix: count a live (Forgot == false) OperationIndexSnapshot as activity. Index ops are timestamped from the snapshot's own time, so they slot into the scan the same way backup ops do.

Also dropped a default instance-ID filter that testTaskRunner.QueryOperations was adding but prod taskRunnerImpl doesn't. It would have hidden unassociated ops from the regression test, since Next's query is deliberately instance-unscoped. Neutral for existing tests, they all seed ops with the config instance ID.

Tests: four scheduled-forget cases in TestScheduling. The indexed-snapshot case fails on main.

@CLAassistant

CLAassistant commented Jul 7, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@dobbry-vechur
dobbry-vechur force-pushed the fix/scheduled-forget-external-snapshots branch 2 times, most recently from 6610265 to 4a9f4a0 Compare July 8, 2026 20:32
@garethgeorge
garethgeorge force-pushed the main branch 3 times, most recently from 1a75a0a to aeade74 Compare July 10, 2026 09:45
@dobbry-vechur

Copy link
Copy Markdown
Author

@garethgeorge sorry to bother, but wdyt?

@garethgeorge

Copy link
Copy Markdown
Owner

Hi, appreciate the interest in contributing a fix here! Sorry for the late reply -- I've gone ahead and removed the shouldSkip behavior which is yielding merge conflicts, but I'm happy to merge the change to the Next() defn with merge conflicts resolved

@PixelJonas

Copy link
Copy Markdown

I've just hit this issue - @dobbry-vechur anything I can do to support resolving the merge-conflicts?

@dobbry-vechur

Copy link
Copy Markdown
Author

Hey @PixelJonas I'll make sure to update this PR today.

- count live indexed snapshots as activity in ScheduledForgetTask.Next
- rename foundBackup to foundActivity
- drop default instance ID filter in testTaskRunner.QueryOperations
- add scheduled forget cases to TestScheduling
@dobbry-vechur
dobbry-vechur force-pushed the fix/scheduled-forget-external-snapshots branch from 4a9f4a0 to 20414d5 Compare August 1, 2026 18:56
@dobbry-vechur

Copy link
Copy Markdown
Author

@garethgeorge - updated, resolved conflicts, should be ready for merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants