Repository: restic/restic
Pull request #5185 (2024-12-18 16:39:40)

Issue: 4942: cmd_rewrite: add snapshot summary data to an existing snapshot.
cmd_rewrite: add capability to add snapshot summary data to an existing snapshot.

Add option --snapshot-summary [-s] to enable this enhancement.

<!--
Thank you very much for contributing code or documentation to restic! Please
fill out the following questions to make it easier for us to review your
changes.
-->

What does this PR change? What problem does it solve?
-----------------------------------------------------
This pull request add the capability of adding snapshot summary data to an existing snapshot.
<!--
Describe the changes and their purpose here, as detailed as needed.
-->

Was the change previously discussed in an issue or on the forum?
----------------------------------------------------------------
https://github.com/restic/restic/issues/4942
closes https://github.com/restic/restic/issues/4942
<!--
Link issues and relevant forum posts here.

If this PR resolves an issue on GitHub, use "Closes #1234" so that the issue
is closed automatically when this PR is merged.
-->

Checklist
---------

<!--
You do not need to check all the boxes below all at once. Feel free to take
your time and add more commits. If you're done and ready for review, please
check the last box. Enable a checkbox by replacing [ ] with [x].

Please always follow these steps:
- Read the [contribution guidelines](https://github.com/restic/restic/blob/master/CONTRIBUTING.md#providing-patches).
- Enable [maintainer edits](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork).
- Run `gofmt` on the code in all commits.
- Format all commit messages in the same style as [the other commits in the repository](https://github.com/restic/restic/blob/master/CONTRIBUTING.md#git-commits).
-->

- [x] I have added tests for all code changes.
- [x] I have added documentation for relevant changes (in the manual).
- [X] There's a new file in `changelog/unreleased/` that describes the changes for our users (see [template](https://github.com/restic/restic/blob/master/changelog/TEMPLATE)).
- [x] I'm done! This pull request is ready for review.


wplapper: There will be a subsequent PR which will allow include filters to be used for the restic rewrite command, see https://github.com/restic/restic/issues/4278 for details.
wplapper: @MichaelEischer: Thank you for reviewing my code. I have updated the code as you requested. The new tag for creating snapshot summary data is "snap-summary". See changes in PR #5185.
MichaelEischer: I've reverted a bunch of the changes to an older state and made the refactoring I had in mind during the last round of reviews. I'll take another look for a final review in the next days.

Patch:
diff --git a/changelog/unreleased/issue-4942 b/changelog/unreleased/issue-4942
new file mode 100644
index 000000000..ee3820b59
--- /dev/null
+++ b/changelog/unreleased/issue-4942
@@ -0,0 +1,11 @@
+Enhancement: support creating snapshot summary statistics for old snapshots
+
+When `rewrite` is used with the `--snapshot-summary` option, a new snapshot is
+created containing statistics summary data. Only two fields in the summary will
+be non-zero: `TotalFilesProcessed` and `TotalBytesProcessed`.
+
+When rewrite is called with one of the `--exclude` options, `TotalFilesProcessed`
+and `TotalBytesProcessed` will be updated in the snapshot summary.
+
+https://github.com/restic/restic/issues/4942
+https://github.com/restic/restic/pull/5185
diff --git a/cmd/restic/cmd_repair_snapshots.go b/cmd/restic/cmd_repair_snapshots.go
index ba952432a..34c02b3ff 100644
--- a/cmd/restic/cmd_repair_snapshots.go
+++ b/cmd/restic/cmd_repair_snapshots.go
@@ -143,8 +143,9 @@ func runRepairSnapshots(ctx context.Context, gopts GlobalOptions, opts RepairOpt
 	for sn := range FindFilteredSnapshots(ctx, snapshotLister, repo, &opts.SnapshotFilter, args) {
 		Verbosef("\n%v\n", sn)
 		changed, err := filterAndReplaceSnapshot(ctx, repo, sn,
-			func(ctx context.Context, sn *restic.Snapshot) (restic.ID, error) {
-				return rewriter.RewriteTree(ctx, repo, "/", *sn.Tree)
+			func(ctx context.Context, sn *restic.Snapshot) (restic.ID, *restic.SnapshotSummary, error) {
+				id, err := rewriter.RewriteTree(ctx, repo, "/", *sn.Tree)
+				return id, nil, err
 			}, opts.DryRun, opts.Forget, nil, "repaired")
 		if err != nil {
 			return errors.Fatalf("unable to rewrite snapshot ID %q: %v", sn.ID().Str(), err)
diff --git a/cmd/restic/cmd_rewrite.go b/cmd/restic/cmd_rewrite.go
index 707f8af9b..f847aa372 100644
--- a/cmd/restic/cmd_rewrite.go
+++ b/cmd/restic/cmd_rewrite.go
@@ -35,6 +35,13 @@ Please note that the --forget option only removes the snapshots and not the actu
 data stored in the repository. In order to delete the no longer referenced data,
 use the "prune" command.
 
+When rewrite is used with the --snapshot-summary option, a new snapshot is
+created containing statistics summary data. Only two fields in the summary will
+be non-zero: TotalFilesProcessed and TotalBytesProcessed.
+
+When rewrite is called with one of the --exclude options, TotalFilesProcessed
+and TotalBytesProcessed will be updated in the snapshot summary.
+
 EXIT STATUS
 ===========
 
@@ -83,8 +90,9 @@ func (sma snapshotMetadataArgs) convert() (*snapshotMetadata, error) {
 
 // RewriteOptions collects all options for the rewrite command.
 type RewriteOptions struct {
-	Forget bool
-	DryRun bool
+	Forget          bool
+	DryRun          bool
+	SnapshotSummary bool
 
 	Metadata snapshotMetadataArgs
 	restic.SnapshotFilter
@@ -101,12 +109,15 @@ func init() {
 	f.BoolVarP(&rewriteOptions.DryRun, "dry-run", "n", false, "do not do anything, just print what would be done")
 	f.StringVar(&rewriteOptions.Metadata.Hostname, "new-host", "", "replace hostname")
 	f.StringVar(&rewriteOptions.Metadata.Time, "new-time", "", "replace time of the backup")
+	f.BoolVarP(&rewriteOptions.SnapshotSummary, "snapshot-summary", "s", false, "create snapshot summary record if it does not exist")
 
 	initMultiSnapshotFilter(f, &rewriteOptions.SnapshotFilter, true)
 	rewriteOptions.ExcludePatternOptions.Add(f)
 }
 
-type rewriteFilterFunc func(ctx context.Context, sn *restic.Snapshot) (restic.ID, error)
+// rewriteFilterFunc returns the filtered tree ID or an error. If a snapshot summary is returned, the snapshot will
+// be updated accordingly.
+type rewriteFilterFunc func(ctx context.Context, sn *restic.Snapshot) (restic.ID, *restic.SnapshotSummary, error)
 
 func rewriteSnapshot(ctx context.Context, repo *repository.Repository, sn *restic.Snapshot, opts RewriteOptions) (bool, error) {
 	if sn.Tree == nil {
@@ -126,7 +137,7 @@ func rewriteSnapshot(ctx context.Context, repo *repository.Repository, sn *resti
 
 	var filter rewriteFilterFunc
 
-	if len(rejectByNameFuncs) > 0 {
+	if len(rejectByNameFuncs) > 0 || opts.SnapshotSummary {
 		selectByName := func(nodepath string) bool {
 			for _, reject := range rejectByNameFuncs {
 				if reject(nodepath) {
@@ -146,22 +157,24 @@ func rewriteSnapshot(ctx context.Context, repo *repository.Repository, sn *resti
 
 		rewriter, querySize := walker.NewSnapshotSizeRewriter(rewriteNode)
 
-		filter = func(ctx context.Context, sn *restic.Snapshot) (restic.ID, error) {
+		filter = func(ctx context.Context, sn *restic.Snapshot) (restic.ID, *restic.SnapshotSummary, error) {
 			id, err := rewriter.RewriteTree(ctx, repo, "/", *sn.Tree)
 			if err != nil {
-				return restic.ID{}, err
+				return restic.ID{}, nil, err
 			}
 			ss := querySize()
+			summary := &restic.SnapshotSummary{}
 			if sn.Summary != nil {
-				sn.Summary.TotalFilesProcessed = ss.FileCount
-				sn.Summary.TotalBytesProcessed = ss.FileSize
+				*summary = *sn.Summary
 			}
-			return id, err
+			summary.TotalFilesProcessed = ss.FileCount
+			summary.TotalBytesProcessed = ss.FileSize
+			return id, summary, err
 		}
 
 	} else {
-		filter = func(_ context.Context, sn *restic.Snapshot) (restic.ID, error) {
-			return *sn.Tree, nil
+		filter = func(_ context.Context, sn *restic.Snapshot) (restic.ID, *restic.SnapshotSummary, error) {
+			return *sn.Tree, nil, nil
 		}
 	}
 
@@ -176,9 +189,10 @@ func filterAndReplaceSnapshot(ctx context.Context, repo restic.Repository, sn *r
 	repo.StartPackUploader(wgCtx, wg)
 
 	var filteredTree restic.ID
+	var summary *restic.SnapshotSummary
 	wg.Go(func() error {
 		var err error
-		filteredTree, err = filter(ctx, sn)
+		filteredTree, summary, err = filter(ctx, sn)
 		if err != nil {
 			return err
 		}
@@ -203,7 +217,12 @@ func filterAndReplaceSnapshot(ctx context.Context, repo restic.Repository, sn *r
 		return true, nil
 	}
 
-	if filteredTree == *sn.Tree && newMetadata == nil {
+	matchingSummary := true
+	if summary != nil {
+		matchingSummary = sn.Summary != nil && *summary == *sn.Summary
+	}
+
+	if filteredTree == *sn.Tree && newMetadata == nil && matchingSummary {
 		debug.Log("Snapshot %v not modified", sn)
 		return false, nil
 	}
@@ -230,6 +249,9 @@ func filterAndReplaceSnapshot(ctx context.Context, repo restic.Repository, sn *r
 	// Always set the original snapshot id as this essentially a new snapshot.
 	sn.Original = sn.ID()
 	sn.Tree = &filteredTree
+	if summary != nil {
+		sn.Summary = summary
+	}
 
 	if !forget {
 		sn.AddTags([]string{addTag})
@@ -263,7 +285,7 @@ func filterAndReplaceSnapshot(ctx context.Context, repo restic.Repository, sn *r
 }
 
 func runRewrite(ctx context.Context, opts RewriteOptions, gopts GlobalOptions, args []string) error {
-	if opts.ExcludePatternOptions.Empty() && opts.Metadata.empty() {
+	if !opts.SnapshotSummary && opts.ExcludePatternOptions.Empty() && opts.Metadata.empty() {
 		return errors.Fatal("Nothing to do: no excludes provided and no new metadata provided")
 	}
 
diff --git a/doc/045_working_with_repos.rst b/doc/045_working_with_repos.rst
index f8ff390f0..d5f2240b8 100644
--- a/doc/045_working_with_repos.rst
+++ b/doc/045_working_with_repos.rst
@@ -332,6 +332,10 @@ command, see :ref:`backup-excluding-files` for details.
 It is possible to rewrite only a subset of snapshots by filtering them the same
 way as for the ``copy`` command, see :ref:`copy-filtering-snapshots`.
 
+The option ``--snapshot-summary`` can be used to attach summary data to existing
+snapshots that do not have this information. When a snapshot summary is created
+the only fields added are ``TotalFilesProcessed`` and ``TotalBytesProcessed``.
+
 By default, the ``rewrite`` command will keep the original snapshots and create
 new ones for every snapshot which was modified during rewriting. The new
 snapshots are marked with the tag ``rewrite`` to differentiate them from the


Test patch:
diff --git a/cmd/restic/cmd_rewrite_integration_test.go b/cmd/restic/cmd_rewrite_integration_test.go
index 6471d49ba..188353333 100644
--- a/cmd/restic/cmd_rewrite_integration_test.go
+++ b/cmd/restic/cmd_rewrite_integration_test.go
@@ -139,3 +139,36 @@ func TestRewriteMetadata(t *testing.T) {
 		testRewriteMetadata(t, metadata)
 	}
 }
+
+func TestRewriteSnaphotSummary(t *testing.T) {
+	env, cleanup := withTestEnvironment(t)
+	defer cleanup()
+	createBasicRewriteRepo(t, env)
+
+	rtest.OK(t, runRewrite(context.TODO(), RewriteOptions{SnapshotSummary: true}, env.gopts, []string{}))
+	// no new snapshot should be created as the snapshot already has a summary
+	snapshots := testListSnapshots(t, env.gopts, 1)
+
+	// replace snapshot by one without a summary
+	_, repo, unlock, err := openWithExclusiveLock(context.TODO(), env.gopts, false)
+	rtest.OK(t, err)
+	sn, err := restic.LoadSnapshot(context.TODO(), repo, snapshots[0])
+	rtest.OK(t, err)
+	oldSummary := sn.Summary
+	sn.Summary = nil
+	rtest.OK(t, repo.RemoveUnpacked(context.TODO(), restic.WriteableSnapshotFile, snapshots[0]))
+	snapshots[0], err = restic.SaveSnapshot(context.TODO(), repo, sn)
+	rtest.OK(t, err)
+	unlock()
+
+	// rewrite snapshot and lookup ID of new snapshot
+	rtest.OK(t, runRewrite(context.TODO(), RewriteOptions{SnapshotSummary: true}, env.gopts, []string{}))
+	newSnapshots := testListSnapshots(t, env.gopts, 2)
+	newSnapshot := restic.NewIDSet(newSnapshots...).Sub(restic.NewIDSet(snapshots...)).List()[0]
+
+	sn, err = restic.LoadSnapshot(context.TODO(), repo, newSnapshot)
+	rtest.OK(t, err)
+	rtest.Assert(t, sn.Summary != nil, "snapshot should have summary attached")
+	rtest.Equals(t, oldSummary.TotalBytesProcessed, sn.Summary.TotalBytesProcessed, "unexpected TotalBytesProcessed value")
+	rtest.Equals(t, oldSummary.TotalFilesProcessed, sn.Summary.TotalFilesProcessed, "unexpected TotalFilesProcessed value")
+}