From 4b1641e9786996783624c176636eb4f8d4a9cf1e Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Wed, 11 Jan 2023 16:28:39 -0800 Subject: [PATCH] Don't set updated in backup details for cached items (#2119) ## Description If an item is discovered to be cached in kopia (i.e. kopia-assisted incremental), set the backup details for the item to note that it was not updated. Cached items are discovered by checking the item path and mod time against the snapshots passed into kopia's Upload function ## Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No ## Type of change - [ ] :sunflower: Feature - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup ## Issue(s) * closes #2115 ## Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- src/internal/kopia/upload.go | 19 +++++++- src/internal/kopia/upload_test.go | 73 +++++++++++++++++++++--------- src/internal/kopia/wrapper_test.go | 12 ++++- 3 files changed, 79 insertions(+), 25 deletions(-) diff --git a/src/internal/kopia/upload.go b/src/internal/kopia/upload.go index c5e7a5c5a..a64715fc3 100644 --- a/src/internal/kopia/upload.go +++ b/src/internal/kopia/upload.go @@ -121,6 +121,7 @@ type itemDetails struct { info *details.ItemInfo repoPath path.Path prevPath path.Path + cached bool } type corsoProgress struct { @@ -179,7 +180,7 @@ func (cp *corsoProgress) FinishedFile(relativePath string, err error) { d.repoPath.String(), d.repoPath.ShortRef(), parent.ShortRef(), - true, + !d.cached, *d.info, ) @@ -187,7 +188,7 @@ func (cp *corsoProgress) FinishedFile(relativePath string, err error) { cp.deets.AddFoldersForItem( folders, *d.info, - true, // itemUpdated = true + !d.cached, ) } @@ -199,6 +200,20 @@ func (cp *corsoProgress) FinishedHashingFile(fname string, bs int64) { atomic.AddInt64(&cp.totalBytes, bs) } +// Kopia interface function used as a callback when kopia detects a previously +// uploaded file that matches the current file and skips uploading the new +// (duplicate) version. +func (cp *corsoProgress) CachedFile(fname string, size int64) { + defer cp.UploadProgress.CachedFile(fname, size) + + d := cp.get(fname) + if d == nil { + return + } + + d.cached = true +} + func (cp *corsoProgress) put(k string, v *itemDetails) { cp.mu.Lock() defer cp.mu.Unlock() diff --git a/src/internal/kopia/upload_test.go b/src/internal/kopia/upload_test.go index c382bb8ca..a3a865cd8 100644 --- a/src/internal/kopia/upload_test.go +++ b/src/internal/kopia/upload_test.go @@ -433,29 +433,58 @@ var finishedFileTable = []struct { } func (suite *CorsoProgressUnitSuite) TestFinishedFile() { - for _, test := range finishedFileTable { - suite.T().Run(test.name, func(t *testing.T) { - bd := &details.Builder{} - cp := corsoProgress{ - UploadProgress: &snapshotfs.NullUploadProgress{}, - deets: bd, - pending: map[string]*itemDetails{}, + table := []struct { + name string + cached bool + }{ + { + name: "all updated", + cached: false, + }, + { + name: "all cached", + cached: true, + }, + } + + for _, cachedTest := range table { + suite.T().Run(cachedTest.name, func(outerT *testing.T) { + for _, test := range finishedFileTable { + outerT.Run(test.name, func(t *testing.T) { + bd := &details.Builder{} + cp := corsoProgress{ + UploadProgress: &snapshotfs.NullUploadProgress{}, + deets: bd, + pending: map[string]*itemDetails{}, + } + + ci := test.cachedItems(suite.targetFileName, suite.targetFilePath) + + for k, v := range ci { + cp.put(k, v.info) + } + + require.Len(t, cp.pending, len(ci)) + + for k, v := range ci { + if cachedTest.cached { + cp.CachedFile(k, 42) + } + + cp.FinishedFile(k, v.err) + } + + assert.Empty(t, cp.pending) + + entries := bd.Details().Entries + + assert.Len(t, entries, test.expectedNumEntries) + + for _, entry := range entries { + assert.Equal(t, !cachedTest.cached, entry.Updated) + } + }) } - - ci := test.cachedItems(suite.targetFileName, suite.targetFilePath) - - for k, v := range ci { - cp.put(k, v.info) - } - - require.Len(t, cp.pending, len(ci)) - - for k, v := range ci { - cp.FinishedFile(k, v.err) - } - - assert.Empty(t, cp.pending) - assert.Len(t, bd.Details().Entries, test.expectedNumEntries) }) } } diff --git a/src/internal/kopia/wrapper_test.go b/src/internal/kopia/wrapper_test.go index 07fa78567..3654d8846 100644 --- a/src/internal/kopia/wrapper_test.go +++ b/src/internal/kopia/wrapper_test.go @@ -241,16 +241,20 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections() { name string expectedUploadedFiles int expectedCachedFiles int + // Whether entries in the resulting details should be marked as updated. + deetsUpdated bool }{ { name: "Uncached", expectedUploadedFiles: 47, expectedCachedFiles: 0, + deetsUpdated: true, }, { name: "Cached", expectedUploadedFiles: 0, expectedCachedFiles: 47, + deetsUpdated: false, }, } @@ -274,13 +278,19 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections() { assert.Equal(t, 0, stats.IgnoredErrorCount) assert.Equal(t, 0, stats.ErrorCount) assert.False(t, stats.Incomplete) + // 47 file and 6 folder entries. + details := deets.Details().Entries assert.Len( t, - deets.Details().Entries, + details, test.expectedUploadedFiles+test.expectedCachedFiles+6, ) + for _, entry := range details { + assert.Equal(t, test.deetsUpdated, entry.Updated) + } + checkSnapshotTags( t, suite.ctx,