upload: avoid reporting ignored entries twice (#1506)

Previously ignored entries were reported from the estimate goroutine
and from the main upload goroutine doubling each ignored counter in
the UI.
This commit is contained in:
Jarek Kowalski
2021-11-09 08:39:47 -08:00
committed by GitHub
parent ca4bf81b2f
commit e9303cec8d
2 changed files with 52 additions and 7 deletions

View File

@@ -489,7 +489,7 @@ func (u *Uploader) uploadDirWithCheckpointing(ctx context.Context, rootDir fs.Di
}
if overrideDir != nil {
rootDir = u.wrapIgnorefs(uploadLog(ctx), overrideDir, policyTree)
rootDir = u.wrapIgnorefs(uploadLog(ctx), overrideDir, policyTree, true)
}
defer u.executeAfterFolderAction(ctx, "after-snapshot-root", policyTree.EffectivePolicy().Actions.AfterSnapshotRoot, localDirPathOrEmpty, &hc)
@@ -1132,7 +1132,7 @@ func uploadDirInternal(
defer u.executeAfterFolderAction(ctx, "after-folder", definedActions.AfterFolder, localDirPathOrEmpty, &hc)
if overrideDir != nil {
directory = u.wrapIgnorefs(uploadLog(ctx), overrideDir, policyTree)
directory = u.wrapIgnorefs(uploadLog(ctx), overrideDir, policyTree, true)
}
if de, err := uploadShallowDirInternal(ctx, directory, u); de != nil || err != nil {
@@ -1305,14 +1305,14 @@ func (u *Uploader) Upload(
go func() {
defer scanWG.Done()
wrapped := u.wrapIgnorefs(estimateLog(ctx), entry, policyTree)
wrapped := u.wrapIgnorefs(estimateLog(ctx), entry, policyTree, false /* reportIgnoreStats */)
ds, _ := u.scanDirectory(scanctx, wrapped, policyTree)
u.Progress.EstimatedDataSize(ds.numFiles, ds.totalFileSize)
}()
wrapped := u.wrapIgnorefs(uploadLog(ctx), entry, policyTree)
wrapped := u.wrapIgnorefs(uploadLog(ctx), entry, policyTree, true /* reportIgnoreStats */)
s.RootEntry, err = u.uploadDirWithCheckpointing(ctx, wrapped, policyTree, previousDirs, sourceInfo)
@@ -1338,7 +1338,7 @@ func (u *Uploader) Upload(
return s, nil
}
func (u *Uploader) wrapIgnorefs(logger logging.Logger, entry fs.Directory, policyTree *policy.Tree) fs.Directory {
func (u *Uploader) wrapIgnorefs(logger logging.Logger, entry fs.Directory, policyTree *policy.Tree, reportIgnoreStats bool) fs.Directory {
if u.DisableIgnoreRules {
return entry
}
@@ -1350,14 +1350,18 @@ func (u *Uploader) wrapIgnorefs(logger logging.Logger, entry fs.Directory, polic
policyTree.EffectivePolicy().LoggingPolicy.Directories.Ignored.OrDefault(policy.LogDetailNone),
"ignored directory", fname, nil, nil, timetrack.StartTimer())
u.Progress.ExcludedDir(fname)
if reportIgnoreStats {
u.Progress.ExcludedDir(fname)
}
} else {
maybeLogEntryProcessed(
logger,
policyTree.EffectivePolicy().LoggingPolicy.Entries.Ignored.OrDefault(policy.LogDetailNone),
"ignored", fname, nil, nil, timetrack.StartTimer())
u.Progress.ExcludedFile(fname, md.Size())
if reportIgnoreStats {
u.Progress.ExcludedFile(fname, md.Size())
}
}
u.stats.AddExcluded(md)

View File

@@ -229,6 +229,47 @@ func TestUpload_TopLevelDirectoryReadFailure(t *testing.T) {
}
}
func TestUploadDoesNotReportProgressForIgnoredFilesTwice(t *testing.T) {
ctx := testlogging.Context(t)
th := newUploadTestHarness(ctx, t)
defer th.cleanup()
sourceDir := mockfs.NewDirectory()
sourceDir.AddFile("f1", []byte{1, 2, 3}, defaultPermissions)
sourceDir.AddFile("f2", []byte{1, 2, 3, 4}, defaultPermissions)
sourceDir.AddFile("f3", []byte{1, 2, 3, 4, 5}, defaultPermissions)
sourceDir.AddDir("d1", defaultPermissions)
sourceDir.AddFile("d1/f1", []byte{1, 2, 3}, defaultPermissions)
sourceDir.AddDir("d2", defaultPermissions)
sourceDir.AddFile("d2/f1", []byte{1, 2, 3}, defaultPermissions)
sourceDir.AddFile("d2/f2", []byte{1, 2, 3, 4}, defaultPermissions)
u := NewUploader(th.repo)
cup := &CountingUploadProgress{}
u.Progress = cup
u.OverrideEntryLogDetail = policy.NewLogDetail(10)
u.OverrideDirLogDetail = policy.NewLogDetail(10)
policyTree := policy.BuildTree(map[string]*policy.Policy{
".": {
FilesPolicy: policy.FilesPolicy{
IgnoreRules: []string{"d2", "f2"},
},
},
}, policy.DefaultPolicy)
_, err := u.Upload(ctx, sourceDir, policyTree, snapshot.SourceInfo{})
require.NoError(t, err)
// make sure ignored counter is only incremented by 1, even though we process each directory twice
// - once during estimation and once during upload.
require.EqualValues(t, 1, cup.counters.TotalExcludedFiles)
require.EqualValues(t, 1, cup.counters.TotalExcludedDirs)
}
func TestUpload_SubDirectoryReadFailureFailFast(t *testing.T) {
ctx := testlogging.Context(t)
th := newUploadTestHarness(ctx, t)