From c79fb6fcdd254382705753957bd299472696a2c6 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Thu, 15 Jun 2017 14:40:34 +0200 Subject: [PATCH] prune: Delete repacked files as the very last step --- src/cmds/restic/cmd_prune.go | 4 ++- src/restic/index/index_test.go | 10 +++--- src/restic/repository/repack.go | 48 +++++++++++----------------- src/restic/repository/repack_test.go | 11 +++++-- 4 files changed, 36 insertions(+), 37 deletions(-) diff --git a/src/cmds/restic/cmd_prune.go b/src/cmds/restic/cmd_prune.go index 71b83369e..e4441c8f3 100644 --- a/src/cmds/restic/cmd_prune.go +++ b/src/cmds/restic/cmd_prune.go @@ -216,10 +216,11 @@ func pruneRepository(gopts GlobalOptions, repo restic.Repository) error { Verbosef("will delete %d packs and rewrite %d packs, this frees %s\n", len(removePacks), len(rewritePacks), formatBytes(uint64(removeBytes))) + var repackedBlobs restic.IDSet if len(rewritePacks) != 0 { bar = newProgressMax(!gopts.Quiet, uint64(len(rewritePacks)), "packs rewritten") bar.Start() - err = repository.Repack(ctx, repo, rewritePacks, usedBlobs, bar) + repackedBlobs, err = repository.Repack(ctx, repo, rewritePacks, usedBlobs, bar) if err != nil { return err } @@ -230,6 +231,7 @@ func pruneRepository(gopts GlobalOptions, repo restic.Repository) error { return err } + removePacks.Merge(repackedBlobs) if len(removePacks) != 0 { bar = newProgressMax(!gopts.Quiet, uint64(len(removePacks)), "packs deleted") bar.Start() diff --git a/src/restic/index/index_test.go b/src/restic/index/index_test.go index 11d0cc08a..81c3d6e4b 100644 --- a/src/restic/index/index_test.go +++ b/src/restic/index/index_test.go @@ -43,7 +43,7 @@ func TestIndexNew(t *testing.T) { repo, cleanup := createFilledRepo(t, 3, 0) defer cleanup() - idx, err := New(context.TODO(), repo, nil) + idx, err := New(context.TODO(), repo, restic.NewIDSet(), nil) if err != nil { t.Fatalf("New() returned error %v", err) } @@ -70,7 +70,7 @@ func TestIndexLoad(t *testing.T) { validateIndex(t, repo, loadIdx) - newIdx, err := New(context.TODO(), repo, nil) + newIdx, err := New(context.TODO(), repo, restic.NewIDSet(), nil) if err != nil { t.Fatalf("New() returned error %v", err) } @@ -134,7 +134,7 @@ func BenchmarkIndexNew(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { - idx, err := New(context.TODO(), repo, nil) + idx, err := New(context.TODO(), repo, restic.NewIDSet(), nil) if err != nil { b.Fatalf("New() returned error %v", err) @@ -151,7 +151,7 @@ func BenchmarkIndexSave(b *testing.B) { repo, cleanup := repository.TestRepository(b) defer cleanup() - idx, err := New(context.TODO(), repo, nil) + idx, err := New(context.TODO(), repo, restic.NewIDSet(), nil) test.OK(b, err) for i := 0; i < 8000; i++ { @@ -184,7 +184,7 @@ func TestIndexDuplicateBlobs(t *testing.T) { repo, cleanup := createFilledRepo(t, 3, 0.01) defer cleanup() - idx, err := New(context.TODO(), repo, nil) + idx, err := New(context.TODO(), repo, restic.NewIDSet(), nil) if err != nil { t.Fatal(err) } diff --git a/src/restic/repository/repack.go b/src/restic/repository/repack.go index 36a000783..e613d8da2 100644 --- a/src/restic/repository/repack.go +++ b/src/restic/repository/repack.go @@ -16,9 +16,9 @@ import ( // Repack takes a list of packs together with a list of blobs contained in // these packs. Each pack is loaded and the blobs listed in keepBlobs is saved -// into a new pack. Afterwards, the packs are removed. This operation requires -// an exclusive lock on the repo. -func Repack(ctx context.Context, repo restic.Repository, packs restic.IDSet, keepBlobs restic.BlobSet, p *restic.Progress) (err error) { +// into a new pack. Returned is the list of obsolete packs which can then +// be removed. +func Repack(ctx context.Context, repo restic.Repository, packs restic.IDSet, keepBlobs restic.BlobSet, p *restic.Progress) (obsoletePacks restic.IDSet, err error) { debug.Log("repacking %d packs while keeping %d blobs", len(packs), len(keepBlobs)) for packID := range packs { @@ -27,39 +27,39 @@ func Repack(ctx context.Context, repo restic.Repository, packs restic.IDSet, kee tempfile, err := fs.TempFile("", "restic-temp-repack-") if err != nil { - return errors.Wrap(err, "TempFile") + return nil, errors.Wrap(err, "TempFile") } beRd, err := repo.Backend().Load(ctx, h, 0, 0) if err != nil { - return err + return nil, err } hrd := hashing.NewReader(beRd, sha256.New()) packLength, err := io.Copy(tempfile, hrd) if err != nil { - return errors.Wrap(err, "Copy") + return nil, errors.Wrap(err, "Copy") } if err = beRd.Close(); err != nil { - return errors.Wrap(err, "Close") + return nil, errors.Wrap(err, "Close") } hash := restic.IDFromHash(hrd.Sum(nil)) debug.Log("pack %v loaded (%d bytes), hash %v", packID.Str(), packLength, hash.Str()) if !packID.Equal(hash) { - return errors.Errorf("hash does not match id: want %v, got %v", packID, hash) + return nil, errors.Errorf("hash does not match id: want %v, got %v", packID, hash) } _, err = tempfile.Seek(0, 0) if err != nil { - return errors.Wrap(err, "Seek") + return nil, errors.Wrap(err, "Seek") } blobs, err := pack.List(repo.Key(), tempfile, packLength) if err != nil { - return err + return nil, err } debug.Log("processing pack %v, blobs: %v", packID.Str(), len(blobs)) @@ -80,30 +80,30 @@ func Repack(ctx context.Context, repo restic.Repository, packs restic.IDSet, kee n, err := tempfile.ReadAt(buf, int64(entry.Offset)) if err != nil { - return errors.Wrap(err, "ReadAt") + return nil, errors.Wrap(err, "ReadAt") } if n != len(buf) { - return errors.Errorf("read blob %v from %v: not enough bytes read, want %v, got %v", + return nil, errors.Errorf("read blob %v from %v: not enough bytes read, want %v, got %v", h, tempfile.Name(), len(buf), n) } n, err = crypto.Decrypt(repo.Key(), buf, buf) if err != nil { - return err + return nil, err } buf = buf[:n] id := restic.Hash(buf) if !id.Equal(entry.ID) { - return errors.Errorf("read blob %v from %v: wrong data returned, hash is %v", + return nil, errors.Errorf("read blob %v from %v: wrong data returned, hash is %v", h, tempfile.Name(), id) } _, err = repo.SaveBlob(ctx, entry.Type, buf, entry.ID) if err != nil { - return err + return nil, err } debug.Log(" saved blob %v", entry.ID.Str()) @@ -112,11 +112,11 @@ func Repack(ctx context.Context, repo restic.Repository, packs restic.IDSet, kee } if err = tempfile.Close(); err != nil { - return errors.Wrap(err, "Close") + return nil, errors.Wrap(err, "Close") } if err = fs.RemoveIfExists(tempfile.Name()); err != nil { - return errors.Wrap(err, "Remove") + return nil, errors.Wrap(err, "Remove") } if p != nil { p.Report(restic.Stat{Blobs: 1}) @@ -124,18 +124,8 @@ func Repack(ctx context.Context, repo restic.Repository, packs restic.IDSet, kee } if err := repo.Flush(); err != nil { - return err + return nil, err } - for packID := range packs { - h := restic.Handle{Type: restic.DataFile, Name: packID.String()} - err := repo.Backend().Remove(ctx, h) - if err != nil { - debug.Log("error removing pack %v: %v", packID.Str(), err) - return err - } - debug.Log("removed pack %v", packID.Str()) - } - - return nil + return packs, nil } diff --git a/src/restic/repository/repack_test.go b/src/restic/repository/repack_test.go index d339cf2b8..fa64f8eff 100644 --- a/src/restic/repository/repack_test.go +++ b/src/restic/repository/repack_test.go @@ -127,10 +127,17 @@ func findPacksForBlobs(t *testing.T, repo restic.Repository, blobs restic.BlobSe } func repack(t *testing.T, repo restic.Repository, packs restic.IDSet, blobs restic.BlobSet) { - err := repository.Repack(context.TODO(), repo, packs, blobs, nil) + repackedBlobs, err := repository.Repack(context.TODO(), repo, packs, blobs, nil) if err != nil { t.Fatal(err) } + + for id := range repackedBlobs { + err = repo.Backend().Remove(context.TODO(), restic.Handle{Type: restic.DataFile, Name: id.String()}) + if err != nil { + t.Fatal(err) + } + } } func saveIndex(t *testing.T, repo restic.Repository) { @@ -140,7 +147,7 @@ func saveIndex(t *testing.T, repo restic.Repository) { } func rebuildIndex(t *testing.T, repo restic.Repository) { - idx, err := index.New(context.TODO(), repo, nil) + idx, err := index.New(context.TODO(), repo, restic.NewIDSet(), nil) if err != nil { t.Fatal(err) }