restic check with snapshot filters (continuation of PR #5213) by wplapper · Pull Request #5469 · restic/restic

wplapper · 2025-08-07T07:30:51Z

This is the continuation of PR #5213. I decided to use a git provided e-mail address instead of my private e-mail address. All changes up-to 2025-03-07 have been incorporated. Details to follow here:

changelog/unreleased/issue-3326:
Introduction of snapshot based filter for command restic check

cmd/restic/cmd_check.go:
Integration of snapshot based filtering. Some messages had to be changed so that the text does not refer to 'all packfiles' but to 'selected packfiles'.

cmd/restic/cmd_check_integration_test.go:
A new test for filtering via snapshots has been added.

doc/077_troubleshooting.rst:
A short paragraph in the manual has been added about snapshot based filtering.

internal/checker/checker.go:
A new func CheckWithSnapshots() has been added which collects the needed packfiles via restic.FindUsedBlobs().

internal/checker/checker_test.go:
A new test TestCheckRepoSnapshot() has been added to check out the new functionality.

What does this PR change? What problem does it solve?

This PR addresses snapshot based filters in command restic check.

Was the change previously discussed in an issue or on the forum?

#3326

Checklist

I have added tests for all code changes.
I have added documentation for relevant changes (in the manual).
There's a new file in changelog/unreleased/ that describes the changes for our users (see template).
I'm done! This pull request is ready for review.

MichaelEischer

The changes from #5532 require untangling how the pack file selection works here, see the comments for details. Structure(...) should also adhere to the snapshot selection. That method can also generate the pack file list, making CheckWithSnapshots unnecessary and also avoiding the massive performance penalty of iterating a second time over the snapshot trees.

Edit:
Just integrate the snapshot selection into checker.LoadSnapshots. Then checker.Snapshots naturally also adheres to the snapshot filter. The checker.Checker can the also wrap the ReadPacks method such that it replaces the pack list passed to the filter callback with the one only containing the packs relevant for the snapshots.

wplapper · 2025-10-04T11:49:22Z

Hi MichaelEischer,

looking at the existing changes after #5532 and the proposed changed in #5550 I have decided to wait until #5550 has been commited. Then I will start from a restic 0.18.1 base and adopt my changes and your recommendations accordingly. I will place my PR into "draft" status.

wplapper · 2025-10-14T19:35:29Z

I have no idea what this Windows test ``TestDirAttributeCombinationsOverwrite` does, I have seen it before, it seems to be transient, and I have no idea of how to fix it. I need some solid Windows advice. Thanks!

wplapper · 2025-10-17T06:17:19Z

I have removed these failing windows tests from the this PR. I will have to wait anyway until this issue has been resolved.

MichaelEischer

Thanks for the changes so far. This is definitely moving into the right direction, but needs a few further cleanups. The current ReadPacks extension is also far slower than it has to be (see comments).

MichaelEischer

Thanks for keeping up with all the review comments! I have a few further comments, although most of them are nits this time.

MichaelEischer · 2025-11-17T20:48:17Z

 		return summary, ctx.Err()
 	}

+	opts.filteredStatus = chkr.FilterStatus()


Let's move it below the if opts.CheckUnused { block. Then the call is directly next to the place where it's used.

moved the call to buildPacksFilter() directly before its use.

MichaelEischer · 2025-11-17T20:49:19Z

 	}

+	opts.filteredStatus = chkr.FilterStatus()
+	readDataFilter, err := buildPacksFilter(opts, printer)


filteredStatus should be passed as parameter to buildPacksFilter(...). It doesn't really belong into the opts struct.

MichaelEischer · 2025-11-17T20:50:38Z

 	c.snapshots, err = restic.MemorizeList(ctx, c.repo, restic.SnapshotFile)
+	c.snapshotFilter = snapshotFilter
+	c.args = args
+	c.trackUnused = true


That should only be set if the snapshotFilter is actually used. Aka. c.filterActive must also be set here. Or even better, get completely rid of filterActive and just use a helper function that determines whether there's an active filter.

moved setting of c.trackUnused into loadActiveTrees().

MichaelEischer · 2025-11-17T20:53:12Z

+	}
+
+	// convert used blobs into their encompassing packfiles
+	selectedPacks := restic.NewIDSet()


This should also only be computed within filteredPackfiles. The same applies for the variable definition of packsWithSize.

Your suggestion encapsulates the code better :)

MichaelEischer · 2025-11-17T20:53:43Z

+}
+
+// FilterStatus returns the filtering status
+func (c *Checker) FilterStatus() bool {


Should move right below LoadSnapshots().

MichaelEischer · 2025-11-17T20:54:46Z

+			return
+		}
+		c.filterActive = true
+		c.trees = trees


Should just return the trees instead of storing them in c.trees

MichaelEischer · 2025-11-17T20:56:02Z

+	errs = []error{}
+
+	if len(c.args) > 0 || !c.snapshotFilter.Empty() {
+		err := (&c.snapshotFilter).FindAll(ctx, c.snapshots, c.repo, c.args, func(_ string, sn *data.Snapshot, er error) error {


Suggested change

err := (&c.snapshotFilter).FindAll(ctx, c.snapshots, c.repo, c.args, func(_ string, sn *data.Snapshot, er error) error {

err := (&c.snapshotFilter).FindAll(ctx, c.snapshots, c.repo, c.args, func(_ string, sn *data.Snapshot, err error) error {

There's no problem with shadowing the outer err variable

MichaelEischer · 2025-11-17T20:59:53Z

+	return buf.String(), err
+}
+
+func TestCheckFullOutput(t *testing.T) {


Please merge the tree tests into a table test (see for example TestPrepareCheckCache; you bascially create a struct that contains the expected inputs and output and then use a loop to iterate over all of them). Almost all of their code is shared.

bult testCase table.

MichaelEischer

LGTM. Thanks!

I've added a few small cleanups, see the extra commits for details. In particular, only packsWithSize is built but not selectedPacks. As sets in go are under the hood also maps, we can just directly build the final map. FilterStatus() is now the authoritative source whether filtering happens and is less sensitive to whether it's called after certain other methods.

New implementation based on PR#5550 (refactor pack selection for read data): cmd/restic/cmd_check.go: had to move the filter creation code buildPacksFilter() to the point where the repo is open and the index has been loaded. Otherwise the new function selectPacks() would not work. Added the new function selectPack() which creates a packfile list based on current snapshot filters. selectPack() creates a list of used snapshots, uses data.FindUsedBlobs() to create a set of used blobs and uses this set to generate a map of used packfiles. In buildPacksFilter() code has been inserted for the print output to indicate snapshot filter selection. doc/077_troubleshooting.rst adds a paragraph about snapshot filtering. changelog/unreleased/issue-3326 announces the new enhancement.

linter caught me out!

cmd/restic/cmd_check.go: moved filter checking into internal/checker/checker.go as a consequence, checker.LoadSnapshots() gained thwo paramenters, which were just saved away. The actual processing of the snapshot filter got moved into checker.Structure(), where context and repository are available. function buildPacksFilter() gained the parameter 'chkr', so the selected packs and the active flag can be retrieved from the checker instance. buildPacksFilter() has been modified, so there is a choice between 'normal' processing and snapshot filter processing. The printed messages were adopted accordingly. cmd/restic/cmd_check_integration_test.go: three new tests verifying the filtering. internal/checker/checker.go: seee above internal/checker/checker.go and internal/checker/testing.go had to be modified for using the changed LoadSnapshots() interface.

linter caught me out with an incorrect spelling

add subtest in cmd_check_integration_test.go: show that exaxctly one snapshot has been selected in Structure()

I have no idea whu Windows chokes in a different place from here. Timing?

cmd/restic/cmd_check.go: moved a lot of code from the command to internal/checker/checker inserted one message line "Snapshot filtering is active" internal/checker/checker.go: split the code from cmd/restic/cmd_check.go into parts for LoadSnapshots(), Structure() and a new wrapper for ReadPacks() cmd/restic/cmd_check_integration_test.go: adopted the tests to the new environment

linter caught me out again

cmd/restic/cmd_check.go: passing printer parameter to chkr.Structure() cmd/restic/cmd_check_integration_test.go internal/checker/checker_test.go internal/checker/testing.go adopting changes to chker.Structure() internal/checker/checker.go: now printing error message to printer.E

linter caught me out

remove these pesky fialing windows tests for the time being

changelog/unreleased/issue-3326: changed wording doc/077_troubleshooting.rst: clarified usage of `restic check` with SnapshotFilter internal/restorer/restorer_windows_test.go: reinstated original windows test cmd/restic/cmd_check.go: added word 'filtered ' for output display in the various messages internal/checker/checker.go: reworked checker logic according to recommendations, removed extra parameter `printer`in `Structure()`. added helper function `loadActiveTrees()` modified `ReadPacks()` to pass filtered packfiles to `repository.ReadPacks()`. cmd/restic/cmd_check_integration_test.go: internal/checker/checker_test.go: internal/checker/testing.go: adjusted parameter lists for modified functions.

cmd/restic/cmd_check.go: `buildPacksFilter()` received parameter `filteredStatus`, rearranged code internal/checker/checker.go: worked on `ReadPacks()`, rearranged code cmd/restic/cmd_check_integration_test.go: build a table of tests to run, together with the expeced results internal/checker/checker_test.go: internal/checker/testing.go: adapt code changes in internal/checker/checker.go

MichaelEischer · 2025-11-28T19:05:22Z

And a trivial rebase + fix to work with the changed datatype for c.blobRefs.M

wplapper mentioned this pull request Aug 7, 2025

PR closed - wrong email: restic check - enable --read-data-* with snapshot filter #5213

Closed

4 tasks

MichaelEischer mentioned this pull request Oct 3, 2025

check: refactor pack selection for read data #5550

Merged

4 tasks

MichaelEischer requested changes Oct 3, 2025

View reviewed changes

Comment thread cmd/restic/cmd_check.go Outdated

Comment thread cmd/restic/cmd_check.go Outdated

Comment thread internal/checker/checker.go

Comment thread internal/checker/checker.go Outdated

wplapper marked this pull request as draft October 4, 2025 11:49

wplapper force-pushed the check_snapshots-v3 branch from 9ba1bf4 to 3d78e0b Compare October 13, 2025 16:25

wplapper marked this pull request as ready for review October 14, 2025 15:09

wplapper mentioned this pull request Oct 16, 2025

Bug: failing Windows tests make development of new enhancements painful and difficult #5563

Closed

MichaelEischer requested changes Nov 16, 2025

View reviewed changes

MichaelEischer requested changes Nov 17, 2025

View reviewed changes

MichaelEischer mentioned this pull request Nov 19, 2025

check/copy/diff/stats: reduce memory usage #5610

Merged

3 tasks

MichaelEischer approved these changes Nov 28, 2025

View reviewed changes

wplapper and others added 16 commits November 28, 2025 20:03

restic check - snapshots - linter error

637527a

linter caught me out!

restic check with snapshot filter - linter check

09826f9

linter caught me out with an incorrect spelling

restic check with filtered snapshot

9e8e7d4

add subtest in cmd_check_integration_test.go: show that exaxctly one snapshot has been selected in Structure()

restic check with snapshot filter - windows chokes

a230b62

I have no idea whu Windows chokes in a different place from here. Timing?

restic check - fix linter issues

84f8828

linter caught me out again

restic check snap filter error messages - linter

d2119ac

linter caught me out

test windows failures

f96164d

remove these pesky fialing windows tests for the time being

minor cleanups

7a1585f

simplify check if filter is active

34f03d2

remove temporary map from pack filtering

23218f2

fix compatiblity with recent changes

38a987d

MichaelEischer force-pushed the check_snapshots-v3 branch from ed06afc to 38a987d Compare November 28, 2025 19:04

MichaelEischer enabled auto-merge (squash) November 28, 2025 19:06

MichaelEischer merged commit ce57961 into restic:master Nov 28, 2025
11 checks passed

wplapper deleted the check_snapshots-v3 branch November 29, 2025 05:31

amrkmn mentioned this pull request Jun 9, 2026

restic 0.19.0 amrkmn/homebrew-tap#803

Closed

BrewTestBot mentioned this pull request Jun 9, 2026

restic 0.19.0 Homebrew/homebrew-core#287125

Merged

	err := (&c.snapshotFilter).FindAll(ctx, c.snapshots, c.repo, c.args, func(_ string, sn *data.Snapshot, er error) error {
	err := (&c.snapshotFilter).FindAll(ctx, c.snapshots, c.repo, c.args, func(_ string, sn *data.Snapshot, err error) error {

Conversation

wplapper commented Aug 7, 2025

What does this PR change? What problem does it solve?

Was the change previously discussed in an issue or on the forum?

Checklist

Uh oh!

MichaelEischer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wplapper commented Oct 4, 2025

Uh oh!

wplapper commented Oct 14, 2025

Uh oh!

wplapper commented Oct 17, 2025

Uh oh!

MichaelEischer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaelEischer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wplapper Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaelEischer left a comment

Choose a reason for hiding this comment

Uh oh!

MichaelEischer commented Nov 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

MichaelEischer left a comment •

edited

Loading

wplapper Nov 21, 2025 •

edited

Loading