From a77d5c4d11f0324107718ded8a8123ef1b6cf33c Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 26 May 2022 12:38:18 +0200 Subject: [PATCH 01/14] repository: index saving belongs into the MasterIndex --- internal/checker/checker_test.go | 1 - internal/repository/master_index.go | 39 +++++++++++++++++++++++++ internal/repository/packer_manager.go | 2 +- internal/repository/repack_test.go | 8 +++--- internal/repository/repository.go | 40 +------------------------- internal/repository/repository_test.go | 5 ++-- internal/restic/repository.go | 13 +++++---- 7 files changed, 54 insertions(+), 54 deletions(-) diff --git a/internal/checker/checker_test.go b/internal/checker/checker_test.go index f2ee0c732..bbb538d7a 100644 --- a/internal/checker/checker_test.go +++ b/internal/checker/checker_test.go @@ -508,7 +508,6 @@ func TestCheckerBlobTypeConfusion(t *testing.T) { test.OK(t, err) test.OK(t, repo.Flush(ctx)) - test.OK(t, repo.SaveIndex(ctx)) snapshot, err := restic.NewSnapshot([]string{"/damaged"}, []string{"test"}, "foo", time.Now()) test.OK(t, err) diff --git a/internal/repository/master_index.go b/internal/repository/master_index.go index 22e763c35..f52060d51 100644 --- a/internal/repository/master_index.go +++ b/internal/repository/master_index.go @@ -1,6 +1,7 @@ package repository import ( + "bytes" "context" "fmt" "sync" @@ -405,6 +406,44 @@ func (mi *MasterIndex) Save(ctx context.Context, repo restic.Repository, packBla return obsolete, err } +// SaveIndex saves an index in the repository. +func SaveIndex(ctx context.Context, repo restic.SaverUnpacked, index *Index) (restic.ID, error) { + buf := bytes.NewBuffer(nil) + + err := index.Encode(buf) + if err != nil { + return restic.ID{}, err + } + + return repo.SaveUnpacked(ctx, restic.IndexFile, buf.Bytes()) +} + +// saveIndex saves all indexes in the backend. +func (mi *MasterIndex) saveIndex(ctx context.Context, r restic.SaverUnpacked, indexes ...*Index) error { + for i, idx := range indexes { + debug.Log("Saving index %d", i) + + sid, err := SaveIndex(ctx, r, idx) + if err != nil { + return err + } + + debug.Log("Saved index %d as %v", i, sid) + } + + return mi.MergeFinalIndexes() +} + +// SaveIndex saves all new indexes in the backend. +func (mi *MasterIndex) SaveIndex(ctx context.Context, r restic.SaverUnpacked) error { + return mi.saveIndex(ctx, r, mi.FinalizeNotFinalIndexes()...) +} + +// SaveFullIndex saves all full indexes in the backend. +func (mi *MasterIndex) SaveFullIndex(ctx context.Context, r restic.SaverUnpacked) error { + return mi.saveIndex(ctx, r, mi.FinalizeFullIndexes()...) +} + // ListPacks returns the blobs of the specified pack files grouped by pack file. func (mi *MasterIndex) ListPacks(ctx context.Context, packs restic.IDSet) <-chan restic.PackBlobs { out := make(chan restic.PackBlobs) diff --git a/internal/repository/packer_manager.go b/internal/repository/packer_manager.go index 7347a0e1a..7d0306a03 100644 --- a/internal/repository/packer_manager.go +++ b/internal/repository/packer_manager.go @@ -154,7 +154,7 @@ func (r *Repository) savePacker(ctx context.Context, t restic.BlobType, p *Packe if r.noAutoIndexUpdate { return nil } - return r.SaveFullIndex(ctx) + return r.idx.SaveFullIndex(ctx, r) } // countPacker returns the number of open (unfinished) packers. diff --git a/internal/repository/repack_test.go b/internal/repository/repack_test.go index b86c8c95d..8106a2f0c 100644 --- a/internal/repository/repack_test.go +++ b/internal/repository/repack_test.go @@ -155,8 +155,8 @@ func repack(t *testing.T, repo restic.Repository, packs restic.IDSet, blobs rest } } -func saveIndex(t *testing.T, repo restic.Repository) { - if err := repo.SaveIndex(context.TODO()); err != nil { +func flush(t *testing.T, repo restic.Repository) { + if err := repo.Flush(context.TODO()); err != nil { t.Fatalf("repo.SaveIndex() %v", err) } } @@ -237,7 +237,7 @@ func testRepack(t *testing.T, version uint) { packsBefore, packsAfter) } - saveIndex(t, repo) + flush(t, repo) removeBlobs, keepBlobs := selectBlobs(t, repo, 0.2) @@ -297,7 +297,7 @@ func testRepackCopy(t *testing.T, version uint) { t.Logf("rand seed is %v", seed) createRandomBlobs(t, repo, 100, 0.7) - saveIndex(t, repo) + flush(t, repo) _, keepBlobs := selectBlobs(t, repo, 0.2) copyPacks := findPacksForBlobs(t, repo, keepBlobs) diff --git a/internal/repository/repository.go b/internal/repository/repository.go index 6c095dcc5..1bc4d3245 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -527,7 +527,7 @@ func (r *Repository) Flush(ctx context.Context) error { if r.noAutoIndexUpdate { return nil } - return r.SaveIndex(ctx) + return r.idx.SaveIndex(ctx, r) } // FlushPacks saves all remaining packs. @@ -573,44 +573,6 @@ func (r *Repository) SetIndex(i restic.MasterIndex) error { return r.PrepareCache() } -// SaveIndex saves an index in the repository. -func SaveIndex(ctx context.Context, repo restic.Repository, index *Index) (restic.ID, error) { - buf := bytes.NewBuffer(nil) - - err := index.Encode(buf) - if err != nil { - return restic.ID{}, err - } - - return repo.SaveUnpacked(ctx, restic.IndexFile, buf.Bytes()) -} - -// saveIndex saves all indexes in the backend. -func (r *Repository) saveIndex(ctx context.Context, indexes ...*Index) error { - for i, idx := range indexes { - debug.Log("Saving index %d", i) - - sid, err := SaveIndex(ctx, r, idx) - if err != nil { - return err - } - - debug.Log("Saved index %d as %v", i, sid) - } - - return r.idx.MergeFinalIndexes() -} - -// SaveIndex saves all new indexes in the backend. -func (r *Repository) SaveIndex(ctx context.Context) error { - return r.saveIndex(ctx, r.idx.FinalizeNotFinalIndexes()...) -} - -// SaveFullIndex saves all full indexes in the backend. -func (r *Repository) SaveFullIndex(ctx context.Context) error { - return r.saveIndex(ctx, r.idx.FinalizeFullIndexes()...) -} - // LoadIndex loads all index files from the backend in parallel and stores them // in the master index. The first error that occurred is returned. func (r *Repository) LoadIndex(ctx context.Context) error { diff --git a/internal/repository/repository_test.go b/internal/repository/repository_test.go index 497fd2906..81604dea3 100644 --- a/internal/repository/repository_test.go +++ b/internal/repository/repository_test.go @@ -421,8 +421,7 @@ func testRepositoryIncrementalIndex(t *testing.T, version uint) { saveRandomDataBlobs(t, repo, 5, 1<<15) rtest.OK(t, repo.FlushPacks(context.Background())) } - - rtest.OK(t, repo.SaveFullIndex(context.TODO())) + rtest.OK(t, repo.Flush(context.TODO())) } // add another 5 packs @@ -432,7 +431,7 @@ func testRepositoryIncrementalIndex(t *testing.T, version uint) { } // save final index - rtest.OK(t, repo.SaveIndex(context.TODO())) + rtest.OK(t, repo.Flush(context.TODO())) packEntries := make(map[restic.ID]map[restic.ID]struct{}) diff --git a/internal/restic/repository.go b/internal/restic/repository.go index 61654fa1d..0d608e9bf 100644 --- a/internal/restic/repository.go +++ b/internal/restic/repository.go @@ -15,17 +15,13 @@ type Repository interface { Key() *crypto.Key - SetIndex(MasterIndex) error - Index() MasterIndex - SaveFullIndex(context.Context) error - SaveIndex(context.Context) error LoadIndex(context.Context) error + SetIndex(MasterIndex) error + LookupBlobSize(ID, BlobType) (uint, bool) Config() Config - LookupBlobSize(ID, BlobType) (uint, bool) - // List calls the function fn for each file of type t in the repository. // When an error is returned by fn, processing stops and List() returns the // error. @@ -65,6 +61,11 @@ type LoadJSONUnpackeder interface { LoadJSONUnpacked(ctx context.Context, t FileType, id ID, dest interface{}) error } +// SaverUnpacked allows saving a blob not stored in a pack file +type SaverUnpacked interface { + SaveUnpacked(context.Context, FileType, []byte) (ID, error) +} + type PackBlobs struct { PackID ID Blobs []Blob From ed8aa15376be01bd7780690d3cb6418402ec39d5 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 26 May 2022 12:49:03 +0200 Subject: [PATCH 02/14] repository: add Save method to MasterIndex interface --- cmd/restic/cmd_prune.go | 6 ++---- internal/repository/master_index.go | 4 +++- internal/repository/master_index_test.go | 6 ++---- internal/repository/repack_test.go | 4 +--- internal/restic/repository.go | 3 +++ 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/cmd/restic/cmd_prune.go b/cmd/restic/cmd_prune.go index 76801dea4..ad306858b 100644 --- a/cmd/restic/cmd_prune.go +++ b/cmd/restic/cmd_prune.go @@ -596,10 +596,8 @@ func prune(opts PruneOptions, gopts GlobalOptions, repo restic.Repository, usedB func writeIndexFiles(gopts GlobalOptions, repo restic.Repository, removePacks restic.IDSet, extraObsolete restic.IDs) (restic.IDSet, error) { Verbosef("rebuilding index\n") - idx := (repo.Index()).(*repository.MasterIndex) - packcount := uint64(len(idx.Packs(removePacks))) - bar := newProgressMax(!gopts.Quiet, packcount, "packs processed") - obsoleteIndexes, err := idx.Save(gopts.ctx, repo, removePacks, extraObsolete, bar) + bar := newProgressMax(!gopts.Quiet, 0, "packs processed") + obsoleteIndexes, err := repo.Index().Save(gopts.ctx, repo, removePacks, extraObsolete, bar) bar.Done() return obsoleteIndexes, err } diff --git a/internal/repository/master_index.go b/internal/repository/master_index.go index f52060d51..171411bf4 100644 --- a/internal/repository/master_index.go +++ b/internal/repository/master_index.go @@ -321,7 +321,9 @@ const saveIndexParallelism = 4 // The new index contains the IDs of all known indexes in the "supersedes" // field. The IDs are also returned in the IDSet obsolete. // After calling this function, you should remove the obsolete index files. -func (mi *MasterIndex) Save(ctx context.Context, repo restic.Repository, packBlacklist restic.IDSet, extraObsolete restic.IDs, p *progress.Counter) (obsolete restic.IDSet, err error) { +func (mi *MasterIndex) Save(ctx context.Context, repo restic.SaverUnpacked, packBlacklist restic.IDSet, extraObsolete restic.IDs, p *progress.Counter) (obsolete restic.IDSet, err error) { + p.SetMax(uint64(len(mi.Packs(packBlacklist)))) + mi.idxMutex.Lock() defer mi.idxMutex.Unlock() diff --git a/internal/repository/master_index_test.go b/internal/repository/master_index_test.go index 79932af07..c393382c3 100644 --- a/internal/repository/master_index_test.go +++ b/internal/repository/master_index_test.go @@ -291,14 +291,12 @@ func BenchmarkMasterIndexLookupMultipleIndexUnknown(b *testing.B) { } func BenchmarkMasterIndexLookupParallel(b *testing.B) { - mIdx := repository.NewMasterIndex() - for _, numindices := range []int{25, 50, 100} { var lookupBh restic.BlobHandle b.StopTimer() rng := rand.New(rand.NewSource(0)) - mIdx, lookupBh = createRandomMasterIndex(b, rng, numindices, 10000) + mIdx, lookupBh := createRandomMasterIndex(b, rng, numindices, 10000) b.StartTimer() name := fmt.Sprintf("known,indices=%d", numindices) @@ -361,7 +359,7 @@ func testIndexSave(t *testing.T, version uint) { t.Fatal(err) } - obsoletes, err := repo.Index().(*repository.MasterIndex).Save(context.TODO(), repo, nil, nil, nil) + obsoletes, err := repo.Index().Save(context.TODO(), repo, nil, nil, nil) if err != nil { t.Fatalf("unable to save new index: %v", err) } diff --git a/internal/repository/repack_test.go b/internal/repository/repack_test.go index 8106a2f0c..f8e375a52 100644 --- a/internal/repository/repack_test.go +++ b/internal/repository/repack_test.go @@ -192,9 +192,7 @@ func rebuildIndex(t *testing.T, repo restic.Repository) { t.Fatal(err) } - _, err = (repo.Index()).(*repository.MasterIndex). - Save(context.TODO(), repo, restic.NewIDSet(), nil, nil) - + _, err = repo.Index().Save(context.TODO(), repo, restic.NewIDSet(), nil, nil) if err != nil { t.Fatal(err) } diff --git a/internal/restic/repository.go b/internal/restic/repository.go index 0d608e9bf..0e388bb5d 100644 --- a/internal/restic/repository.go +++ b/internal/restic/repository.go @@ -4,6 +4,7 @@ import ( "context" "github.com/restic/restic/internal/crypto" + "github.com/restic/restic/internal/ui/progress" ) // Repository stores data in a backend. It provides high-level functions and @@ -82,4 +83,6 @@ type MasterIndex interface { // blocks any modification of the index. Each(ctx context.Context) <-chan PackedBlob ListPacks(ctx context.Context, packs IDSet) <-chan PackBlobs + + Save(ctx context.Context, repo SaverUnpacked, packBlacklist IDSet, extraObsolete IDs, p *progress.Counter) (obsolete IDSet, err error) } From 628ae799cace9a922bfba36bd6a755e515b81ee4 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 26 May 2022 13:30:52 +0200 Subject: [PATCH 03/14] repository: make flushPacks private --- internal/repository/repository.go | 6 +++--- internal/repository/repository_test.go | 15 +++------------ 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/internal/repository/repository.go b/internal/repository/repository.go index 1bc4d3245..77cf01989 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -519,7 +519,7 @@ func (r *Repository) SaveUnpacked(ctx context.Context, t restic.FileType, p []by // Flush saves all remaining packs and the index func (r *Repository) Flush(ctx context.Context) error { - if err := r.FlushPacks(ctx); err != nil { + if err := r.flushPacks(ctx); err != nil { return err } @@ -530,8 +530,8 @@ func (r *Repository) Flush(ctx context.Context) error { return r.idx.SaveIndex(ctx, r) } -// FlushPacks saves all remaining packs. -func (r *Repository) FlushPacks(ctx context.Context) error { +// flushPacks saves all remaining packs. +func (r *Repository) flushPacks(ctx context.Context) error { pms := []struct { t restic.BlobType pm *packerManager diff --git a/internal/repository/repository_test.go b/internal/repository/repository_test.go index 81604dea3..d32b16410 100644 --- a/internal/repository/repository_test.go +++ b/internal/repository/repository_test.go @@ -414,22 +414,13 @@ func testRepositoryIncrementalIndex(t *testing.T, version uint) { repository.IndexFull = func(*repository.Index, bool) bool { return true } - // add 15 packs + // add a few rounds of packs for j := 0; j < 5; j++ { - // add 3 packs, write intermediate index - for i := 0; i < 3; i++ { - saveRandomDataBlobs(t, repo, 5, 1<<15) - rtest.OK(t, repo.FlushPacks(context.Background())) - } + // add some packs, write intermediate index + saveRandomDataBlobs(t, repo, 20, 1<<15) rtest.OK(t, repo.Flush(context.TODO())) } - // add another 5 packs - for i := 0; i < 5; i++ { - saveRandomDataBlobs(t, repo, 5, 1<<15) - rtest.OK(t, repo.FlushPacks(context.Background())) - } - // save final index rtest.OK(t, repo.Flush(context.TODO())) From fe5a8e137a4cc7eafb21c2b593f59a4362be5e2d Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 26 May 2022 13:41:06 +0200 Subject: [PATCH 04/14] repository: remove unused index.Store --- internal/repository/index.go | 21 -------------- internal/repository/index_test.go | 36 +++++++++++++----------- internal/repository/master_index_test.go | 16 +++++------ internal/repository/repository_test.go | 5 ++-- 4 files changed, 30 insertions(+), 48 deletions(-) diff --git a/internal/repository/index.go b/internal/repository/index.go index 520fcbd8e..09a09b2c3 100644 --- a/internal/repository/index.go +++ b/internal/repository/index.go @@ -131,27 +131,6 @@ var IndexFull = func(idx *Index, compress bool) bool { } -// Store remembers the id and pack in the index. -func (idx *Index) Store(pb restic.PackedBlob) { - idx.m.Lock() - defer idx.m.Unlock() - - if idx.final { - panic("store new item in finalized index") - } - - debug.Log("%v", pb) - - // get packIndex and save if new packID - packIndex, ok := idx.packIDToIndex[pb.PackID] - if !ok { - packIndex = idx.addToPacks(pb.PackID) - idx.packIDToIndex[pb.PackID] = packIndex - } - - idx.store(packIndex, pb.Blob) -} - // StorePack remembers the ids of all blobs of a given pack // in the index func (idx *Index) StorePack(id restic.ID, blobs []restic.Blob) { diff --git a/internal/repository/index_test.go b/internal/repository/index_test.go index 6940afe2f..138555100 100644 --- a/internal/repository/index_test.go +++ b/internal/repository/index_test.go @@ -19,6 +19,7 @@ func TestIndexSerialize(t *testing.T) { // create 50 packs with 20 blobs each for i := 0; i < 50; i++ { packID := restic.NewRandomID() + var blobs []restic.Blob pos := uint(0) for j := 0; j < 20; j++ { @@ -37,10 +38,11 @@ func TestIndexSerialize(t *testing.T) { }, PackID: packID, } - idx.Store(pb) + blobs = append(blobs, pb.Blob) tests = append(tests, pb) pos += length } + idx.StorePack(packID, blobs) } wr := bytes.NewBuffer(nil) @@ -83,6 +85,7 @@ func TestIndexSerialize(t *testing.T) { newtests := []restic.PackedBlob{} for i := 0; i < 10; i++ { packID := restic.NewRandomID() + var blobs []restic.Blob pos := uint(0) for j := 0; j < 10; j++ { @@ -95,10 +98,11 @@ func TestIndexSerialize(t *testing.T) { }, PackID: packID, } - idx.Store(pb) + blobs = append(blobs, pb.Blob) newtests = append(newtests, pb) pos += length } + idx.StorePack(packID, blobs) } // finalize; serialize idx, unserialize to idx3 @@ -141,24 +145,23 @@ func TestIndexSize(t *testing.T) { idx := repository.NewIndex() packs := 200 - blobs := 100 + blobCount := 100 for i := 0; i < packs; i++ { packID := restic.NewRandomID() + var blobs []restic.Blob pos := uint(0) - for j := 0; j < blobs; j++ { + for j := 0; j < blobCount; j++ { length := uint(i*100 + j) - idx.Store(restic.PackedBlob{ - Blob: restic.Blob{ - BlobHandle: restic.NewRandomBlobHandle(), - Offset: pos, - Length: length, - }, - PackID: packID, + blobs = append(blobs, restic.Blob{ + BlobHandle: restic.NewRandomBlobHandle(), + Offset: pos, + Length: length, }) pos += length } + idx.StorePack(packID, blobs) } wr := bytes.NewBuffer(nil) @@ -166,7 +169,7 @@ func TestIndexSize(t *testing.T) { err := idx.Encode(wr) rtest.OK(t, err) - t.Logf("Index file size for %d blobs in %d packs is %d", blobs*packs, packs, wr.Len()) + t.Logf("Index file size for %d blobs in %d packs is %d", blobCount*packs, packs, wr.Len()) } // example index serialization from doc/Design.rst @@ -419,13 +422,12 @@ func TestIndexPacks(t *testing.T) { for i := 0; i < 20; i++ { packID := restic.NewRandomID() - idx.Store(restic.PackedBlob{ - Blob: restic.Blob{ + idx.StorePack(packID, []restic.Blob{ + { BlobHandle: restic.NewRandomBlobHandle(), Offset: 0, Length: 23, }, - PackID: packID, }) packs.Insert(packID) @@ -529,6 +531,7 @@ func TestIndexHas(t *testing.T) { // create 50 packs with 20 blobs each for i := 0; i < 50; i++ { packID := restic.NewRandomID() + var blobs []restic.Blob pos := uint(0) for j := 0; j < 20; j++ { @@ -547,10 +550,11 @@ func TestIndexHas(t *testing.T) { }, PackID: packID, } - idx.Store(pb) + blobs = append(blobs, pb.Blob) tests = append(tests, pb) pos += length } + idx.StorePack(packID, blobs) } for _, testBlob := range tests { diff --git a/internal/repository/master_index_test.go b/internal/repository/master_index_test.go index c393382c3..d1f240063 100644 --- a/internal/repository/master_index_test.go +++ b/internal/repository/master_index_test.go @@ -57,12 +57,12 @@ func TestMasterIndex(t *testing.T) { } idx1 := repository.NewIndex() - idx1.Store(blob1) - idx1.Store(blob12a) + idx1.StorePack(blob1.PackID, []restic.Blob{blob1.Blob}) + idx1.StorePack(blob12a.PackID, []restic.Blob{blob12a.Blob}) idx2 := repository.NewIndex() - idx2.Store(blob2) - idx2.Store(blob12b) + idx2.StorePack(blob2.PackID, []restic.Blob{blob2.Blob}) + idx2.StorePack(blob12b.PackID, []restic.Blob{blob12b.Blob}) mIdx := repository.NewMasterIndex() mIdx.Insert(idx1) @@ -154,10 +154,10 @@ func TestMasterMergeFinalIndexes(t *testing.T) { } idx1 := repository.NewIndex() - idx1.Store(blob1) + idx1.StorePack(blob1.PackID, []restic.Blob{blob1.Blob}) idx2 := repository.NewIndex() - idx2.Store(blob2) + idx2.StorePack(blob2.PackID, []restic.Blob{blob2.Blob}) mIdx := repository.NewMasterIndex() mIdx.Insert(idx1) @@ -191,8 +191,8 @@ func TestMasterMergeFinalIndexes(t *testing.T) { // merge another index containing identical blobs idx3 := repository.NewIndex() - idx3.Store(blob1) - idx3.Store(blob2) + idx3.StorePack(blob1.PackID, []restic.Blob{blob1.Blob}) + idx3.StorePack(blob2.PackID, []restic.Blob{blob2.Blob}) mIdx.Insert(idx3) finalIndexes = mIdx.FinalizeNotFinalIndexes() diff --git a/internal/repository/repository_test.go b/internal/repository/repository_test.go index d32b16410..f2c00a2e6 100644 --- a/internal/repository/repository_test.go +++ b/internal/repository/repository_test.go @@ -362,13 +362,12 @@ func benchmarkLoadIndex(b *testing.B, version uint) { idx := repository.NewIndex() for i := 0; i < 5000; i++ { - idx.Store(restic.PackedBlob{ - Blob: restic.Blob{ + idx.StorePack(restic.NewRandomID(), []restic.Blob{ + { BlobHandle: restic.NewRandomBlobHandle(), Length: 1234, Offset: 1235, }, - PackID: restic.NewRandomID(), }) } From e4f20dea61ea59a75cb323e34c013db16aea1906 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 26 May 2022 13:41:24 +0200 Subject: [PATCH 05/14] repository: inline index.encode --- internal/repository/index.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/internal/repository/index.go b/internal/repository/index.go index 09a09b2c3..139b5a173 100644 --- a/internal/repository/index.go +++ b/internal/repository/index.go @@ -409,13 +409,6 @@ func (idx *Index) Encode(w io.Writer) error { idx.m.Lock() defer idx.m.Unlock() - return idx.encode(w) -} - -// encode writes the JSON serialization of the index to the writer w. -func (idx *Index) encode(w io.Writer) error { - debug.Log("encoding index") - list, err := idx.generatePackList() if err != nil { return err From 8ef2968f28a48ac90930c8e6a30321b6216ecc8e Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 26 May 2022 14:42:22 +0200 Subject: [PATCH 06/14] repository: remove unused index.ListPack --- internal/repository/index.go | 18 ------------------ internal/repository/index_test.go | 12 +++++++++++- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/internal/repository/index.go b/internal/repository/index.go index 139b5a173..92f0da0e9 100644 --- a/internal/repository/index.go +++ b/internal/repository/index.go @@ -176,24 +176,6 @@ func (idx *Index) Lookup(bh restic.BlobHandle, pbs []restic.PackedBlob) []restic return pbs } -// ListPack returns a list of blobs contained in a pack. -func (idx *Index) ListPack(id restic.ID) (pbs []restic.PackedBlob) { - idx.m.Lock() - defer idx.m.Unlock() - - for typ := range idx.byType { - m := &idx.byType[typ] - m.foreach(func(e *indexEntry) bool { - if idx.packs[e.packIndex] == id { - pbs = append(pbs, idx.toPackedBlob(e, restic.BlobType(typ))) - } - return true - }) - } - - return pbs -} - // Has returns true iff the id is listed in the index. func (idx *Index) Has(bh restic.BlobHandle) bool { idx.m.Lock() diff --git a/internal/repository/index_test.go b/internal/repository/index_test.go index 138555100..dab52a6bc 100644 --- a/internal/repository/index_test.go +++ b/internal/repository/index_test.go @@ -2,6 +2,7 @@ package repository_test import ( "bytes" + "context" "math/rand" "sync" "testing" @@ -336,7 +337,7 @@ func TestIndexUnserialize(t *testing.T) { rtest.Equals(t, oldIdx, idx.Supersedes()) - blobs := idx.ListPack(exampleLookupTest.packID) + blobs := listPack(idx, exampleLookupTest.packID) if len(blobs) != len(exampleLookupTest.blobs) { t.Fatalf("expected %d blobs in pack, got %d", len(exampleLookupTest.blobs), len(blobs)) } @@ -353,6 +354,15 @@ func TestIndexUnserialize(t *testing.T) { } } +func listPack(idx *repository.Index, id restic.ID) (pbs []restic.PackedBlob) { + for pb := range idx.Each(context.TODO()) { + if pb.PackID.Equal(id) { + pbs = append(pbs, pb) + } + } + return pbs +} + var ( benchmarkIndexJSON []byte benchmarkIndexJSONOnce sync.Once From e0a7852b8bfba882412e09bda94840ee82f1e903 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 26 May 2022 15:05:29 +0200 Subject: [PATCH 07/14] repository: remove unused (Master)Index.Count --- internal/repository/index.go | 9 --------- internal/repository/master_index.go | 13 ------------- internal/repository/master_index_test.go | 6 ------ internal/repository/repository_test.go | 2 +- internal/restic/repository.go | 1 - 5 files changed, 1 insertion(+), 30 deletions(-) diff --git a/internal/repository/index.go b/internal/repository/index.go index 92f0da0e9..fd7ac1b94 100644 --- a/internal/repository/index.go +++ b/internal/repository/index.go @@ -314,15 +314,6 @@ func (idx *Index) Packs() restic.IDSet { return packs } -// Count returns the number of blobs of type t in the index. -func (idx *Index) Count(t restic.BlobType) (n uint) { - debug.Log("counting blobs of type %v", t) - idx.m.Lock() - defer idx.m.Unlock() - - return idx.byType[t].len() -} - type packJSON struct { ID restic.ID `json:"id"` Blobs []blobJSON `json:"blobs"` diff --git a/internal/repository/master_index.go b/internal/repository/master_index.go index 171411bf4..4fdd4a0ce 100644 --- a/internal/repository/master_index.go +++ b/internal/repository/master_index.go @@ -158,19 +158,6 @@ func (mi *MasterIndex) Packs(packBlacklist restic.IDSet) restic.IDSet { return packs } -// Count returns the number of blobs of type t in the index. -func (mi *MasterIndex) Count(t restic.BlobType) (n uint) { - mi.idxMutex.RLock() - defer mi.idxMutex.RUnlock() - - var sum uint - for _, idx := range mi.idx { - sum += idx.Count(t) - } - - return sum -} - // Insert adds a new index to the MasterIndex. func (mi *MasterIndex) Insert(idx *Index) { mi.idxMutex.Lock() diff --git a/internal/repository/master_index_test.go b/internal/repository/master_index_test.go index d1f240063..0aeaf5eb1 100644 --- a/internal/repository/master_index_test.go +++ b/internal/repository/master_index_test.go @@ -122,12 +122,6 @@ func TestMasterIndex(t *testing.T) { rtest.Assert(t, blobs == nil, "Expected no blobs when fetching with a random id") _, found = mIdx.LookupSize(restic.NewRandomBlobHandle()) rtest.Assert(t, !found, "Expected no blobs when fetching with a random id") - - // Test Count - num := mIdx.Count(restic.DataBlob) - rtest.Equals(t, uint(2), num) - num = mIdx.Count(restic.TreeBlob) - rtest.Equals(t, uint(2), num) } func TestMasterMergeFinalIndexes(t *testing.T) { diff --git a/internal/repository/repository_test.go b/internal/repository/repository_test.go index f2c00a2e6..2265ba453 100644 --- a/internal/repository/repository_test.go +++ b/internal/repository/repository_test.go @@ -374,7 +374,7 @@ func benchmarkLoadIndex(b *testing.B, version uint) { id, err := repository.SaveIndex(context.TODO(), repo, idx) rtest.OK(b, err) - b.Logf("index saved as %v (%v entries)", id.Str(), idx.Count(restic.DataBlob)) + b.Logf("index saved as %v", id.Str()) fi, err := repo.Backend().Stat(context.TODO(), restic.Handle{Type: restic.IndexFile, Name: id.String()}) rtest.OK(b, err) b.Logf("filesize is %v", fi.Size) diff --git a/internal/restic/repository.go b/internal/restic/repository.go index 0e388bb5d..fea151164 100644 --- a/internal/restic/repository.go +++ b/internal/restic/repository.go @@ -76,7 +76,6 @@ type PackBlobs struct { type MasterIndex interface { Has(BlobHandle) bool Lookup(BlobHandle) []PackedBlob - Count(BlobType) uint // Each returns a channel that yields all blobs known to the index. When // the context is cancelled, the background goroutine terminates. This From bf81bf079586a626a5233ed2099b87aab69aa7e6 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 5 Jun 2022 21:57:16 +0200 Subject: [PATCH 08/14] repository: Properly set id for finalized index As MergeFinalIndex and index uploads can occur concurrently, it is necessary for MergeFinalIndex to check whether the IDs for an index were already set before merging it. Otherwise, we'd loose the ID of an index which is set _after_ uploading it. --- internal/repository/master_index.go | 12 ++++++++++-- internal/repository/master_index_test.go | 22 ++++------------------ internal/repository/repository.go | 6 ------ internal/repository/testing.go | 10 ++++++++++ 4 files changed, 24 insertions(+), 26 deletions(-) diff --git a/internal/repository/master_index.go b/internal/repository/master_index.go index 4fdd4a0ce..660fb40aa 100644 --- a/internal/repository/master_index.go +++ b/internal/repository/master_index.go @@ -287,7 +287,9 @@ func (mi *MasterIndex) MergeFinalIndexes() error { idx := mi.idx[i] // clear reference in masterindex as it may become stale mi.idx[i] = nil - if !idx.Final() { + // do not merge indexes that have no id set + ids, _ := idx.IDs() + if !idx.Final() || len(ids) == 0 { newIdx = append(newIdx, idx) } else { err := mi.idx[0].merge(idx) @@ -404,7 +406,13 @@ func SaveIndex(ctx context.Context, repo restic.SaverUnpacked, index *Index) (re return restic.ID{}, err } - return repo.SaveUnpacked(ctx, restic.IndexFile, buf.Bytes()) + id, err := repo.SaveUnpacked(ctx, restic.IndexFile, buf.Bytes()) + ierr := index.SetID(id) + if ierr != nil { + // logic bug + panic(ierr) + } + return id, err } // saveIndex saves all indexes in the backend. diff --git a/internal/repository/master_index_test.go b/internal/repository/master_index_test.go index 0aeaf5eb1..ffdaf7af9 100644 --- a/internal/repository/master_index_test.go +++ b/internal/repository/master_index_test.go @@ -157,16 +157,9 @@ func TestMasterMergeFinalIndexes(t *testing.T) { mIdx.Insert(idx1) mIdx.Insert(idx2) - finalIndexes := mIdx.FinalizeNotFinalIndexes() + finalIndexes, idxCount := repository.TestMergeIndex(t, mIdx) rtest.Equals(t, []*repository.Index{idx1, idx2}, finalIndexes) - - err := mIdx.MergeFinalIndexes() - if err != nil { - t.Fatal(err) - } - - allIndexes := mIdx.All() - rtest.Equals(t, 1, len(allIndexes)) + rtest.Equals(t, 1, idxCount) blobCount := 0 for range mIdx.Each(context.TODO()) { @@ -189,16 +182,9 @@ func TestMasterMergeFinalIndexes(t *testing.T) { idx3.StorePack(blob2.PackID, []restic.Blob{blob2.Blob}) mIdx.Insert(idx3) - finalIndexes = mIdx.FinalizeNotFinalIndexes() + finalIndexes, idxCount = repository.TestMergeIndex(t, mIdx) rtest.Equals(t, []*repository.Index{idx3}, finalIndexes) - - err = mIdx.MergeFinalIndexes() - if err != nil { - t.Fatal(err) - } - - allIndexes = mIdx.All() - rtest.Equals(t, 1, len(allIndexes)) + rtest.Equals(t, 1, idxCount) // Index should have same entries as before! blobs = mIdx.Lookup(bhInIdx1) diff --git a/internal/repository/repository.go b/internal/repository/repository.go index 77cf01989..01a7cad8a 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -582,12 +582,6 @@ func (r *Repository) LoadIndex(ctx context.Context) error { if err != nil { return err } - - _, err = idx.IDs() - if err != nil { - return err - } - r.idx.Insert(idx) return nil }) diff --git a/internal/repository/testing.go b/internal/repository/testing.go index 05dfab64d..a24148a1b 100644 --- a/internal/repository/testing.go +++ b/internal/repository/testing.go @@ -132,3 +132,13 @@ func BenchmarkAllVersions(b *testing.B, bench VersionedBenchmark) { }) } } + +func TestMergeIndex(t testing.TB, mi *MasterIndex) ([]*Index, int) { + finalIndexes := mi.FinalizeNotFinalIndexes() + for _, idx := range finalIndexes { + test.OK(t, idx.SetID(restic.NewRandomID())) + } + + test.OK(t, mi.MergeFinalIndexes()) + return finalIndexes, len(mi.idx) +} From ef53ca4a5a911069ca5326b1e5b49520a1fd21a7 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 5 Jun 2022 21:59:38 +0200 Subject: [PATCH 09/14] repository: remove MasterIndex.All() --- internal/repository/master_index.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/internal/repository/master_index.go b/internal/repository/master_index.go index 660fb40aa..43888bf77 100644 --- a/internal/repository/master_index.go +++ b/internal/repository/master_index.go @@ -234,14 +234,6 @@ func (mi *MasterIndex) FinalizeFullIndexes() []*Index { return list } -// All returns all indexes. -func (mi *MasterIndex) All() []*Index { - mi.idxMutex.Lock() - defer mi.idxMutex.Unlock() - - return mi.idx -} - // Each returns a channel that yields all blobs known to the index. When the // context is cancelled, the background goroutine terminates. This blocks any // modification of the index. From 1974ad7ce2c981e89cd1337d9de9333fa2ae6c6f Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 26 May 2022 15:43:04 +0200 Subject: [PATCH 10/14] repository: hide MasterIndex.FinalizeFullIndexes / FinalizeNotFinalIndexes --- internal/repository/master_index.go | 13 ++++++------- internal/repository/master_index_test.go | 6 +----- internal/repository/testing.go | 2 +- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/internal/repository/master_index.go b/internal/repository/master_index.go index 43888bf77..3dd627c6c 100644 --- a/internal/repository/master_index.go +++ b/internal/repository/master_index.go @@ -188,9 +188,9 @@ func (mi *MasterIndex) StorePack(id restic.ID, blobs []restic.Blob) { mi.idx = append(mi.idx, newIdx) } -// FinalizeNotFinalIndexes finalizes all indexes that +// finalizeNotFinalIndexes finalizes all indexes that // have not yet been saved and returns that list -func (mi *MasterIndex) FinalizeNotFinalIndexes() []*Index { +func (mi *MasterIndex) finalizeNotFinalIndexes() []*Index { mi.idxMutex.Lock() defer mi.idxMutex.Unlock() @@ -207,8 +207,8 @@ func (mi *MasterIndex) FinalizeNotFinalIndexes() []*Index { return list } -// FinalizeFullIndexes finalizes all indexes that are full and returns that list. -func (mi *MasterIndex) FinalizeFullIndexes() []*Index { +// finalizeFullIndexes finalizes all indexes that are full and returns that list. +func (mi *MasterIndex) finalizeFullIndexes() []*Index { mi.idxMutex.Lock() defer mi.idxMutex.Unlock() @@ -217,7 +217,6 @@ func (mi *MasterIndex) FinalizeFullIndexes() []*Index { debug.Log("checking %d indexes", len(mi.idx)) for _, idx := range mi.idx { if idx.Final() { - debug.Log("index %p is final", idx) continue } @@ -425,12 +424,12 @@ func (mi *MasterIndex) saveIndex(ctx context.Context, r restic.SaverUnpacked, in // SaveIndex saves all new indexes in the backend. func (mi *MasterIndex) SaveIndex(ctx context.Context, r restic.SaverUnpacked) error { - return mi.saveIndex(ctx, r, mi.FinalizeNotFinalIndexes()...) + return mi.saveIndex(ctx, r, mi.finalizeNotFinalIndexes()...) } // SaveFullIndex saves all full indexes in the backend. func (mi *MasterIndex) SaveFullIndex(ctx context.Context, r restic.SaverUnpacked) error { - return mi.saveIndex(ctx, r, mi.FinalizeFullIndexes()...) + return mi.saveIndex(ctx, r, mi.finalizeFullIndexes()...) } // ListPacks returns the blobs of the specified pack files grouped by pack file. diff --git a/internal/repository/master_index_test.go b/internal/repository/master_index_test.go index ffdaf7af9..d7e4f1678 100644 --- a/internal/repository/master_index_test.go +++ b/internal/repository/master_index_test.go @@ -209,11 +209,7 @@ func createRandomMasterIndex(t testing.TB, rng *rand.Rand, num, size int) (*repo idx1, lookupBh := createRandomIndex(rng, size) mIdx.Insert(idx1) - mIdx.FinalizeNotFinalIndexes() - err := mIdx.MergeFinalIndexes() - if err != nil { - t.Fatal(err) - } + repository.TestMergeIndex(t, mIdx) return mIdx, lookupBh } diff --git a/internal/repository/testing.go b/internal/repository/testing.go index a24148a1b..b9b38b1f4 100644 --- a/internal/repository/testing.go +++ b/internal/repository/testing.go @@ -134,7 +134,7 @@ func BenchmarkAllVersions(b *testing.B, bench VersionedBenchmark) { } func TestMergeIndex(t testing.TB, mi *MasterIndex) ([]*Index, int) { - finalIndexes := mi.FinalizeNotFinalIndexes() + finalIndexes := mi.finalizeNotFinalIndexes() for _, idx := range finalIndexes { test.OK(t, idx.SetID(restic.NewRandomID())) } From e68c3a4e62a73906baebea165098eb8baaff7a8f Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 26 May 2022 16:13:41 +0200 Subject: [PATCH 11/14] repository: simplify CreateIndexFromPacks --- internal/repository/repository.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/internal/repository/repository.go b/internal/repository/repository.go index 01a7cad8a..cab0dda83 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -643,7 +643,6 @@ func (r *Repository) CreateIndexFromPacks(ctx context.Context, packsize map[rest return nil }) - idx := NewIndex() // a worker receives an pack ID from ch, reads the pack contents, and adds them to idx worker := func() error { for fi := range ch { @@ -654,7 +653,7 @@ func (r *Repository) CreateIndexFromPacks(ctx context.Context, packsize map[rest invalid = append(invalid, fi.ID) m.Unlock() } - idx.StorePack(fi.ID, entries) + r.idx.StorePack(fi.ID, entries) p.Add(1) } @@ -671,9 +670,6 @@ func (r *Repository) CreateIndexFromPacks(ctx context.Context, packsize map[rest return invalid, errors.Fatal(err.Error()) } - // Add idx to MasterIndex - r.idx.Insert(idx) - return invalid, nil } From c1a8fa4290cd111c5c852a7dcf64162e4f948f78 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 26 May 2022 16:15:59 +0200 Subject: [PATCH 12/14] repository: remove unused packIDToIndex field --- internal/repository/index.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/internal/repository/index.go b/internal/repository/index.go index fd7ac1b94..6676d0635 100644 --- a/internal/repository/index.go +++ b/internal/repository/index.go @@ -46,8 +46,6 @@ type Index struct { byType [restic.NumBlobTypes]indexMap packs restic.IDs mixedPacks restic.IDSet - // only used by Store, StorePacks does not check for already saved packIDs - packIDToIndex map[restic.ID]int final bool // set to true for all indexes read from the backend ("finalized") ids restic.IDs // set to the IDs of the contained finalized indexes @@ -58,9 +56,8 @@ type Index struct { // NewIndex returns a new index. func NewIndex() *Index { return &Index{ - packIDToIndex: make(map[restic.ID]int), - mixedPacks: restic.NewIDSet(), - created: time.Now(), + mixedPacks: restic.NewIDSet(), + created: time.Now(), } } @@ -402,8 +399,6 @@ func (idx *Index) Finalize() { defer idx.m.Unlock() idx.final = true - // clear packIDToIndex as no more elements will be added - idx.packIDToIndex = nil } // IDs returns the IDs of the index, if available. If the index is not yet From 2cd7e90ad1a3b66585fc55e5da49cac8c57fcef8 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 26 May 2022 16:49:31 +0200 Subject: [PATCH 13/14] repository: cleanup --- internal/repository/master_index.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/internal/repository/master_index.go b/internal/repository/master_index.go index 3dd627c6c..5b1b4754c 100644 --- a/internal/repository/master_index.go +++ b/internal/repository/master_index.go @@ -243,13 +243,10 @@ func (mi *MasterIndex) Each(ctx context.Context) <-chan restic.PackedBlob { go func() { defer mi.idxMutex.RUnlock() - defer func() { - close(ch) - }() + defer close(ch) for _, idx := range mi.idx { - idxCh := idx.Each(ctx) - for pb := range idxCh { + for pb := range idx.Each(ctx) { select { case <-ctx.Done(): return From ec7c9ce88b01240615c4c0c05c4f2766f901b0b0 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 12 Jun 2022 14:23:14 +0200 Subject: [PATCH 14/14] drop unused repository.Loader interface --- internal/repository/repository.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/internal/repository/repository.go b/internal/repository/repository.go index cab0dda83..1312ea754 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -871,11 +871,6 @@ func (r *Repository) SaveTree(ctx context.Context, t *restic.Tree) (restic.ID, e return id, err } -// Loader allows loading data from a backend. -type Loader interface { - Load(ctx context.Context, h restic.Handle, length int, offset int64, fn func(rd io.Reader) error) error -} - type BackendLoadFn func(ctx context.Context, h restic.Handle, length int, offset int64, fn func(rd io.Reader) error) error // StreamPack loads the listed blobs from the specified pack file. The plaintext blob is passed to