From 16313bfcc9c30babad30144e94eeba653dfe69ab Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sat, 30 Jan 2021 16:35:05 +0100 Subject: [PATCH] errcheck: Add error check for MergeFinalIndexes() --- internal/checker/checker.go | 6 ++++- internal/repository/master_index.go | 10 +++++-- internal/repository/master_index_test.go | 33 ++++++++++++++++-------- internal/repository/repository.go | 11 ++++++-- 4 files changed, 44 insertions(+), 16 deletions(-) diff --git a/internal/checker/checker.go b/internal/checker/checker.go index 38b3c75c4..537a44d12 100644 --- a/internal/checker/checker.go +++ b/internal/checker/checker.go @@ -111,7 +111,11 @@ func (c *Checker) LoadIndex(ctx context.Context) (hints []error, errs []error) { } // Merge index before computing pack sizes, as this needs removed duplicates - c.masterIndex.MergeFinalIndexes() + err = c.masterIndex.MergeFinalIndexes() + if err != nil { + // abort if an error occurs merging the indexes + return hints, append(errs, err) + } // compute pack size using index entries c.packs = c.masterIndex.PackSize(ctx, false) diff --git a/internal/repository/master_index.go b/internal/repository/master_index.go index f24589e3d..5aa0ceccb 100644 --- a/internal/repository/master_index.go +++ b/internal/repository/master_index.go @@ -2,6 +2,7 @@ package repository import ( "context" + "fmt" "sync" "github.com/restic/restic/internal/debug" @@ -271,7 +272,7 @@ func (mi *MasterIndex) Each(ctx context.Context) <-chan restic.PackedBlob { // Indexes that are not final are left untouched. // This merging can only be called after all index files are loaded - as // removing of superseded index contents is only possible for unmerged indexes. -func (mi *MasterIndex) MergeFinalIndexes() { +func (mi *MasterIndex) MergeFinalIndexes() error { mi.idxMutex.Lock() defer mi.idxMutex.Unlock() @@ -284,10 +285,15 @@ func (mi *MasterIndex) MergeFinalIndexes() { if !idx.Final() { newIdx = append(newIdx, idx) } else { - mi.idx[0].merge(idx) + err := mi.idx[0].merge(idx) + if err != nil { + return fmt.Errorf("MergeFinalIndexes: %w", err) + } } } mi.idx = newIdx + + return nil } const saveIndexParallelism = 4 diff --git a/internal/repository/master_index_test.go b/internal/repository/master_index_test.go index 167cad2f8..f1fe9af7e 100644 --- a/internal/repository/master_index_test.go +++ b/internal/repository/master_index_test.go @@ -163,7 +163,11 @@ func TestMasterMergeFinalIndexes(t *testing.T) { finalIndexes := mIdx.FinalizeNotFinalIndexes() rtest.Equals(t, []*repository.Index{idx1, idx2}, finalIndexes) - mIdx.MergeFinalIndexes() + err := mIdx.MergeFinalIndexes() + if err != nil { + t.Fatal(err) + } + allIndexes := mIdx.All() rtest.Equals(t, 1, len(allIndexes)) @@ -191,7 +195,11 @@ func TestMasterMergeFinalIndexes(t *testing.T) { finalIndexes = mIdx.FinalizeNotFinalIndexes() rtest.Equals(t, []*repository.Index{idx3}, finalIndexes) - mIdx.MergeFinalIndexes() + err = mIdx.MergeFinalIndexes() + if err != nil { + t.Fatal(err) + } + allIndexes = mIdx.All() rtest.Equals(t, 1, len(allIndexes)) @@ -209,7 +217,7 @@ func TestMasterMergeFinalIndexes(t *testing.T) { rtest.Equals(t, 2, blobCount) } -func createRandomMasterIndex(rng *rand.Rand, num, size int) (*repository.MasterIndex, restic.BlobHandle) { +func createRandomMasterIndex(t testing.TB, rng *rand.Rand, num, size int) (*repository.MasterIndex, restic.BlobHandle) { mIdx := repository.NewMasterIndex() for i := 0; i < num-1; i++ { idx, _ := createRandomIndex(rng, size) @@ -219,7 +227,10 @@ func createRandomMasterIndex(rng *rand.Rand, num, size int) (*repository.MasterI mIdx.Insert(idx1) mIdx.FinalizeNotFinalIndexes() - mIdx.MergeFinalIndexes() + err := mIdx.MergeFinalIndexes() + if err != nil { + t.Fatal(err) + } return mIdx, lookupBh } @@ -229,12 +240,12 @@ func BenchmarkMasterIndexAlloc(b *testing.B) { b.ReportAllocs() for i := 0; i < b.N; i++ { - createRandomMasterIndex(rng, 10000, 5) + createRandomMasterIndex(b, rng, 10000, 5) } } func BenchmarkMasterIndexLookupSingleIndex(b *testing.B) { - mIdx, lookupBh := createRandomMasterIndex(rand.New(rand.NewSource(0)), 1, 200000) + mIdx, lookupBh := createRandomMasterIndex(b, rand.New(rand.NewSource(0)), 1, 200000) b.ResetTimer() @@ -244,7 +255,7 @@ func BenchmarkMasterIndexLookupSingleIndex(b *testing.B) { } func BenchmarkMasterIndexLookupMultipleIndex(b *testing.B) { - mIdx, lookupBh := createRandomMasterIndex(rand.New(rand.NewSource(0)), 100, 10000) + mIdx, lookupBh := createRandomMasterIndex(b, rand.New(rand.NewSource(0)), 100, 10000) b.ResetTimer() @@ -256,7 +267,7 @@ func BenchmarkMasterIndexLookupMultipleIndex(b *testing.B) { func BenchmarkMasterIndexLookupSingleIndexUnknown(b *testing.B) { lookupBh := restic.NewRandomBlobHandle() - mIdx, _ := createRandomMasterIndex(rand.New(rand.NewSource(0)), 1, 200000) + mIdx, _ := createRandomMasterIndex(b, rand.New(rand.NewSource(0)), 1, 200000) b.ResetTimer() @@ -267,7 +278,7 @@ func BenchmarkMasterIndexLookupSingleIndexUnknown(b *testing.B) { func BenchmarkMasterIndexLookupMultipleIndexUnknown(b *testing.B) { lookupBh := restic.NewRandomBlobHandle() - mIdx, _ := createRandomMasterIndex(rand.New(rand.NewSource(0)), 100, 10000) + mIdx, _ := createRandomMasterIndex(b, rand.New(rand.NewSource(0)), 100, 10000) b.ResetTimer() @@ -284,7 +295,7 @@ func BenchmarkMasterIndexLookupParallel(b *testing.B) { b.StopTimer() rng := rand.New(rand.NewSource(0)) - mIdx, lookupBh = createRandomMasterIndex(rng, numindices, 10000) + mIdx, lookupBh = createRandomMasterIndex(b, rng, numindices, 10000) b.StartTimer() name := fmt.Sprintf("known,indices=%d", numindices) @@ -310,7 +321,7 @@ func BenchmarkMasterIndexLookupParallel(b *testing.B) { func BenchmarkMasterIndexLookupBlobSize(b *testing.B) { rng := rand.New(rand.NewSource(0)) - mIdx, lookupBh := createRandomMasterIndex(rand.New(rng), 5, 200000) + mIdx, lookupBh := createRandomMasterIndex(b, rand.New(rng), 5, 200000) b.ResetTimer() diff --git a/internal/repository/repository.go b/internal/repository/repository.go index 298800008..c21bd520c 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -419,7 +419,11 @@ func (r *Repository) saveIndex(ctx context.Context, indexes ...*Index) error { debug.Log("Saved index %d as %v", i, sid) } - r.idx.MergeFinalIndexes() + + err := r.idx.MergeFinalIndexes() + if err != nil { + return err + } return nil } @@ -461,7 +465,10 @@ func (r *Repository) LoadIndex(ctx context.Context) error { return errors.Fatal(err.Error()) } - r.idx.MergeFinalIndexes() + err = r.idx.MergeFinalIndexes() + if err != nil { + return err + } // remove index files from the cache which have been removed in the repo return r.PrepareCache(validIndex)