diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-03-29 13:07:05 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-03-29 13:38:26 +0300 |
commit | a70761f20e4313499f32526a6f43a8cf4b9c3505 (patch) | |
tree | c0ae21f9d5dc17cab90a1e32e6bfc78af1562d38 /internal/git | |
parent | da5041d983e51e90f5202afd68511a9bb242da6b (diff) |
housekeeping: Simplify reporting of cleaned stale data
The current reporting logic we have for cleaned-up stale data is kind of
all-over-the place and inconsistent. Refactor the code to report data in
a single location, only, to make this easier to extend and consistent.
Diffstat (limited to 'internal/git')
-rw-r--r-- | internal/git/housekeeping/clean_stale_data.go | 36 |
1 files changed, 19 insertions, 17 deletions
diff --git a/internal/git/housekeeping/clean_stale_data.go b/internal/git/housekeeping/clean_stale_data.go index 931a97553..a1d3f5eff 100644 --- a/internal/git/housekeeping/clean_stale_data.go +++ b/internal/git/housekeeping/clean_stale_data.go @@ -49,10 +49,22 @@ func (m *RepositoryManager) CleanStaleData(ctx context.Context, repo *localrepo. return fmt.Errorf("housekeeping failed to get repo path: %w", err) } - logEntry := myLogger(ctx) - var filesToPrune []string + staleDataByType := map[string]int{} + defer func() { + if len(staleDataByType) == 0 { + return + } + + logEntry := myLogger(ctx) + for staleDataType, count := range staleDataByType { + logEntry = logEntry.WithField(staleDataType, count) + m.prunedFilesTotal.WithLabelValues(staleDataType).Add(float64(count)) + } + logEntry.Info("removed files") + }() - for field, staleFileFinder := range map[string]staleFileFinderFn{ + var filesToPrune []string + for staleFileType, staleFileFinder := range map[string]staleFileFinderFn{ "objects": findTemporaryObjects, "locks": findStaleLockfiles, "refs": findBrokenLooseReferences, @@ -63,38 +75,28 @@ func (m *RepositoryManager) CleanStaleData(ctx context.Context, repo *localrepo. } { staleFiles, err := staleFileFinder(ctx, repoPath) if err != nil { - return fmt.Errorf("housekeeping failed to find %s: %w", field, err) + return fmt.Errorf("housekeeping failed to find %s: %w", staleFileType, err) } filesToPrune = append(filesToPrune, staleFiles...) - logEntry = logEntry.WithField(field, len(staleFiles)) - - m.prunedFilesTotal.WithLabelValues(field).Add(float64(len(staleFiles))) + staleDataByType[staleFileType] = len(staleFiles) } - unremovableFiles := 0 for _, path := range filesToPrune { if err := os.Remove(path); err != nil { if os.IsNotExist(err) { continue } - unremovableFiles++ - // We cannot use `logEntry` here as it's already seeded - // with the statistics fields. + staleDataByType["failures"]++ myLogger(ctx).WithError(err).WithField("path", path).Warn("unable to remove stale file") } } prunedRefDirs, err := removeRefEmptyDirs(ctx, repo) - m.prunedFilesTotal.WithLabelValues("refsemptydir").Add(float64(prunedRefDirs)) + staleDataByType["refsemptydir"] = prunedRefDirs if err != nil { return fmt.Errorf("housekeeping could not remove empty refs: %w", err) } - logEntry = logEntry.WithField("refsemptydir", prunedRefDirs) - - if len(filesToPrune) > 0 || prunedRefDirs > 0 { - logEntry.WithField("failures", unremovableFiles).Info("removed files") - } // TODO: https://gitlab.com/gitlab-org/gitaly/-/issues/3987 // This is a temporary code and needs to be removed once it will be run on all repositories at least once. |