From 2f91a12143e8c33d896c794d470b98fddb572500 Mon Sep 17 00:00:00 2001 From: zeripath Date: Fri, 13 Jan 2023 21:29:16 +0000 Subject: [PATCH] Continue GCing other repos on error in one repo (#22422) (#22425) Backport #22422 The current code propagates all errors up to the iteration step meaning that a single malformed repo will prevent GC of other repos. This PR simply stops that propagation. Fix #21605 Signed-off-by: Andrew Thornton --- services/repository/check.go | 61 ++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/services/repository/check.go b/services/repository/check.go index 5529a61b3..7e243cb3b 100644 --- a/services/repository/check.go +++ b/services/repository/check.go @@ -73,32 +73,8 @@ func GitGcRepos(ctx context.Context, timeout time.Duration, args ...git.CmdArg) return db.ErrCancelledf("before GC of %s", repo.FullName()) default: } - log.Trace("Running git gc on %v", repo) - command := git.NewCommand(ctx, args...). - SetDescription(fmt.Sprintf("Repository Garbage Collection: %s", repo.FullName())) - var stdout string - var err error - stdout, _, err = command.RunStdString(&git.RunOpts{Timeout: timeout, Dir: repo.RepoPath()}) - - if err != nil { - log.Error("Repository garbage collection failed for %v. Stdout: %s\nError: %v", repo, stdout, err) - desc := fmt.Sprintf("Repository garbage collection failed for %s. Stdout: %s\nError: %v", repo.RepoPath(), stdout, err) - if err = system_model.CreateRepositoryNotice(desc); err != nil { - log.Error("CreateRepositoryNotice: %v", err) - } - return fmt.Errorf("Repository garbage collection failed in repo: %s: Error: %w", repo.FullName(), err) - } - - // Now update the size of the repository - if err := repo_module.UpdateRepoSize(ctx, repo); err != nil { - log.Error("Updating size as part of garbage collection failed for %v. Stdout: %s\nError: %v", repo, stdout, err) - desc := fmt.Sprintf("Updating size as part of garbage collection failed for %s. Stdout: %s\nError: %v", repo.RepoPath(), stdout, err) - if err = system_model.CreateRepositoryNotice(desc); err != nil { - log.Error("CreateRepositoryNotice: %v", err) - } - return fmt.Errorf("Updating size as part of garbage collection failed in repo: %s: Error: %w", repo.FullName(), err) - } - + // we can ignore the error here because it will be logged in GitGCRepo + _ = GitGcRepo(ctx, repo, timeout, args) return nil }, ); err != nil { @@ -109,6 +85,37 @@ func GitGcRepos(ctx context.Context, timeout time.Duration, args ...git.CmdArg) return nil } +// GitGcRepo calls 'git gc' to remove unnecessary files and optimize the local repository +func GitGcRepo(ctx context.Context, repo *repo_model.Repository, timeout time.Duration, args []git.CmdArg) error { + log.Trace("Running git gc on %-v", repo) + command := git.NewCommand(ctx, args...). + SetDescription(fmt.Sprintf("Repository Garbage Collection: %s", repo.FullName())) + var stdout string + var err error + stdout, _, err = command.RunStdString(&git.RunOpts{Timeout: timeout, Dir: repo.RepoPath()}) + + if err != nil { + log.Error("Repository garbage collection failed for %-v. Stdout: %s\nError: %v", repo, stdout, err) + desc := fmt.Sprintf("Repository garbage collection failed for %s. Stdout: %s\nError: %v", repo.RepoPath(), stdout, err) + if err := system_model.CreateRepositoryNotice(desc); err != nil { + log.Error("CreateRepositoryNotice: %v", err) + } + return fmt.Errorf("Repository garbage collection failed in repo: %s: Error: %w", repo.FullName(), err) + } + + // Now update the size of the repository + if err := repo_module.UpdateRepoSize(ctx, repo); err != nil { + log.Error("Updating size as part of garbage collection failed for %-v. Stdout: %s\nError: %v", repo, stdout, err) + desc := fmt.Sprintf("Updating size as part of garbage collection failed for %s. Stdout: %s\nError: %v", repo.RepoPath(), stdout, err) + if err := system_model.CreateRepositoryNotice(desc); err != nil { + log.Error("CreateRepositoryNotice: %v", err) + } + return fmt.Errorf("Updating size as part of garbage collection failed in repo: %s: Error: %w", repo.FullName(), err) + } + + return nil +} + func gatherMissingRepoRecords(ctx context.Context) ([]*repo_model.Repository, error) { repos := make([]*repo_model.Repository, 0, 10) if err := db.Iterate( @@ -162,7 +169,7 @@ func DeleteMissingRepositories(ctx context.Context, doer *user_model.User) error } log.Trace("Deleting %d/%d...", repo.OwnerID, repo.ID) if err := models.DeleteRepository(doer, repo.OwnerID, repo.ID); err != nil { - log.Error("Failed to DeleteRepository %s [%d]: Error: %v", repo.FullName(), repo.ID, err) + log.Error("Failed to DeleteRepository %-v: Error: %v", repo, err) if err2 := system_model.CreateRepositoryNotice("Failed to DeleteRepository %s [%d]: Error: %v", repo.FullName(), repo.ID, err); err2 != nil { log.Error("CreateRepositoryNotice: %v", err) }