diff --git a/cmd/restic/cmd_copy.go b/cmd/restic/cmd_copy.go index 7b9eccad2..1e01e170b 100644 --- a/cmd/restic/cmd_copy.go +++ b/cmd/restic/cmd_copy.go @@ -244,7 +244,7 @@ func copyTree(ctx context.Context, srcRepo restic.Repository, dstRepo restic.Rep bar := printer.NewCounter("packs copied") bar.SetMax(uint64(len(packList))) - _, err = repository.Repack(ctx, srcRepo, dstRepo, packList, copyBlobs, bar, printer.P) + err = repository.Repack(ctx, srcRepo, dstRepo, packList, copyBlobs, bar, printer.P) bar.Done() if err != nil { return errors.Fatalf("%s", err) diff --git a/internal/repository/prune.go b/internal/repository/prune.go index 30152e208..022d32a10 100644 --- a/internal/repository/prune.go +++ b/internal/repository/prune.go @@ -564,7 +564,7 @@ func (plan *PrunePlan) Execute(ctx context.Context, printer progress.Printer) er printer.P("repacking packs\n") bar := printer.NewCounter("packs repacked") bar.SetMax(uint64(len(plan.repackPacks))) - _, err := Repack(ctx, repo, repo, plan.repackPacks, plan.keepBlobs, bar, printer.P) + err := Repack(ctx, repo, repo, plan.repackPacks, plan.keepBlobs, bar, printer.P) bar.Done() if err != nil { return errors.Fatalf("%s", err) diff --git a/internal/repository/repack.go b/internal/repository/repack.go index 6ee86eb22..838017f48 100644 --- a/internal/repository/repack.go +++ b/internal/repository/repack.go @@ -36,7 +36,7 @@ func Repack( keepBlobs repackBlobSet, p *progress.Counter, logf LogFunc, -) (obsoletePacks restic.IDSet, err error) { +) error { debug.Log("repacking %d packs while keeping %d blobs", len(packs), keepBlobs.Len()) if logf == nil { @@ -44,18 +44,12 @@ func Repack( } if repo == dstRepo && dstRepo.Connections() < 2 { - return nil, errors.New("repack step requires a backend connection limit of at least two") + return errors.New("repack step requires a backend connection limit of at least two") } - err = dstRepo.WithBlobUploader(ctx, func(ctx context.Context, uploader restic.BlobSaver) error { - var err error - obsoletePacks, err = repack(ctx, repo, dstRepo, uploader, packs, keepBlobs, p, logf) - return err + return dstRepo.WithBlobUploader(ctx, func(ctx context.Context, uploader restic.BlobSaver) error { + return repack(ctx, repo, dstRepo, uploader, packs, keepBlobs, p, logf) }) - if err != nil { - return nil, err - } - return obsoletePacks, nil } func repack( @@ -67,18 +61,19 @@ func repack( keepBlobs repackBlobSet, p *progress.Counter, logf LogFunc, -) (obsoletePacks restic.IDSet, err error) { +) error { + wg, wgCtx := errgroup.WithContext(ctx) if feature.Flag.Enabled(feature.S3Restore) { job, err := repo.StartWarmup(ctx, packs) if err != nil { - return nil, err + return err } if job.HandleCount() != 0 { logf("warming up %d packs from cold storage, this may take a while...", job.HandleCount()) if err := job.Wait(ctx); err != nil { - return nil, err + return err } } } @@ -156,9 +151,5 @@ func repack( wg.Go(worker) } - if err := wg.Wait(); err != nil { - return nil, err - } - - return packs, nil + return wg.Wait() } diff --git a/internal/repository/repack_test.go b/internal/repository/repack_test.go index 599178371..4d285681f 100644 --- a/internal/repository/repack_test.go +++ b/internal/repository/repack_test.go @@ -150,16 +150,10 @@ func findPacksForBlobs(t *testing.T, repo restic.Repository, blobs restic.BlobSe } func repack(t *testing.T, repo restic.Repository, be backend.Backend, packs restic.IDSet, blobs restic.BlobSet) { - repackedBlobs, err := repository.Repack(context.TODO(), repo, repo, packs, blobs, nil, nil) - if err != nil { - t.Fatal(err) - } + rtest.OK(t, repository.Repack(context.TODO(), repo, repo, packs, blobs, nil, nil)) - for id := range repackedBlobs { - err = be.Remove(context.TODO(), backend.Handle{Type: restic.PackFile, Name: id.String()}) - if err != nil { - t.Fatal(err) - } + for id := range packs { + rtest.OK(t, be.Remove(context.TODO(), backend.Handle{Type: restic.PackFile, Name: id.String()})) } } @@ -269,10 +263,7 @@ func testRepackCopy(t *testing.T, version uint) { _, keepBlobs := selectBlobs(t, random, repo, 0.2) copyPacks := findPacksForBlobs(t, repo, keepBlobs) - _, err := repository.Repack(context.TODO(), repoWrapped, dstRepoWrapped, copyPacks, keepBlobs, nil, nil) - if err != nil { - t.Fatal(err) - } + rtest.OK(t, repository.Repack(context.TODO(), repoWrapped, dstRepoWrapped, copyPacks, keepBlobs, nil, nil)) rebuildAndReloadIndex(t, dstRepo) for h := range keepBlobs { @@ -308,7 +299,7 @@ func testRepackWrongBlob(t *testing.T, version uint) { _, keepBlobs := selectBlobs(t, random, repo, 0) rewritePacks := findPacksForBlobs(t, repo, keepBlobs) - _, err := repository.Repack(context.TODO(), repo, repo, rewritePacks, keepBlobs, nil, nil) + err := repository.Repack(context.TODO(), repo, repo, rewritePacks, keepBlobs, nil, nil) if err == nil { t.Fatal("expected repack to fail but got no error") } @@ -355,8 +346,7 @@ func testRepackBlobFallback(t *testing.T, version uint) { })) // repack must fallback to valid copy - _, err := repository.Repack(context.TODO(), repo, repo, rewritePacks, keepBlobs, nil, nil) - rtest.OK(t, err) + rtest.OK(t, repository.Repack(context.TODO(), repo, repo, rewritePacks, keepBlobs, nil, nil)) keepBlobs = restic.NewBlobSet(restic.BlobHandle{Type: restic.DataBlob, ID: id}) packs := findPacksForBlobs(t, repo, keepBlobs)