diff --git a/cmd/restic/cmd_copy.go b/cmd/restic/cmd_copy.go index 7b9eccad2..096ce76c6 100644 --- a/cmd/restic/cmd_copy.go +++ b/cmd/restic/cmd_copy.go @@ -243,9 +243,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) - bar.Done() + err = repository.Repack(ctx, srcRepo, dstRepo, packList, copyBlobs, bar, printer.P) if err != nil { return errors.Fatalf("%s", err) } diff --git a/internal/repository/prune.go b/internal/repository/prune.go index 30152e208..463efee11 100644 --- a/internal/repository/prune.go +++ b/internal/repository/prune.go @@ -563,9 +563,7 @@ func (plan *PrunePlan) Execute(ctx context.Context, printer progress.Printer) er if len(plan.repackPacks) != 0 { 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) - bar.Done() + err := Repack(ctx, repo, repo, plan.repackPacks, plan.keepBlobs, bar, printer.P) if err != nil { return errors.Fatalf("%s", err) } diff --git a/internal/repository/repack.go b/internal/repository/repack.go index 6ee86eb22..730325afd 100644 --- a/internal/repository/repack.go +++ b/internal/repository/repack.go @@ -36,26 +36,22 @@ 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 { logf = func(_ string, _ ...interface{}) {} } + p.SetMax(uint64(len(packs))) + defer p.Done() 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 +63,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 +153,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) diff --git a/internal/restic/repository.go b/internal/restic/repository.go index 236171cd2..cf3ec7834 100644 --- a/internal/restic/repository.go +++ b/internal/restic/repository.go @@ -53,8 +53,9 @@ type Repository interface { LoadRaw(ctx context.Context, t FileType, id ID) (data []byte, err error) // LoadUnpacked loads and decrypts the file with the given type and ID. LoadUnpacked(ctx context.Context, t FileType, id ID) (data []byte, err error) + // SaveUnpacked stores a file in the repository. This is restricted to snapshots. SaveUnpacked(ctx context.Context, t WriteableFileType, buf []byte) (ID, error) - // RemoveUnpacked removes a file from the repository. This will eventually be restricted to deleting only snapshots. + // RemoveUnpacked removes a file from the repository. This is restricted to snapshots. RemoveUnpacked(ctx context.Context, t WriteableFileType, id ID) error // StartWarmup creates a new warmup job, requesting the backend to warmup the specified packs.