From 27db3ec262ea0e3b61ae24802ec11035b4ebe454 Mon Sep 17 00:00:00 2001 From: greatroar <@> Date: Tue, 13 Oct 2020 20:39:54 +0200 Subject: [PATCH 1/2] Refactor index decoding Decoding old-format indices no longer requires loading and decrypting twice. --- internal/checker/checker.go | 12 +++++++---- internal/repository/index.go | 36 ++++++------------------------- internal/repository/index_test.go | 16 ++++++++------ internal/repository/repository.go | 27 +++++++++++------------ 4 files changed, 37 insertions(+), 54 deletions(-) diff --git a/internal/checker/checker.go b/internal/checker/checker.go index 1f3233a01..dfead2778 100644 --- a/internal/checker/checker.go +++ b/internal/checker/checker.go @@ -117,12 +117,16 @@ func (c *Checker) LoadIndex(ctx context.Context) (hints []error, errs []error) { debug.Log("worker got file %v", fi.ID.Str()) var err error var idx *repository.Index - idx, buf, err = repository.LoadIndexWithDecoder(ctx, c.repo, buf[:0], fi.ID, repository.DecodeIndex) - if errors.Cause(err) == repository.ErrOldIndexFormat { + oldFormat := false + + buf, err = c.repo.LoadAndDecrypt(ctx, buf[:0], restic.IndexFile, fi.ID) + if err == nil { + idx, oldFormat, err = repository.DecodeIndex(buf) + } + + if oldFormat { debug.Log("index %v has old format", fi.ID.Str()) hints = append(hints, ErrOldIndexFormat{fi.ID}) - - idx, buf, err = repository.LoadIndexWithDecoder(ctx, c.repo, buf[:0], fi.ID, repository.DecodeOldIndex) } err = errors.Wrapf(err, "error loading index %v", fi.ID.Str()) diff --git a/internal/repository/index.go b/internal/repository/index.go index 0a3aaead8..b247342e7 100644 --- a/internal/repository/index.go +++ b/internal/repository/index.go @@ -522,11 +522,8 @@ func isErrOldIndex(err error) bool { return false } -// ErrOldIndexFormat means an index with the old format was detected. -var ErrOldIndexFormat = errors.New("index has old format") - -// DecodeIndex loads and unserializes an index from rd. -func DecodeIndex(buf []byte) (idx *Index, err error) { +// DecodeIndex unserializes an index from buf. +func DecodeIndex(buf []byte) (idx *Index, oldFormat bool, err error) { debug.Log("Start decoding index") idxJSON := &jsonIndex{} @@ -536,10 +533,11 @@ func DecodeIndex(buf []byte) (idx *Index, err error) { if isErrOldIndex(err) { debug.Log("index is probably old format, trying that") - err = ErrOldIndexFormat + idx, err = decodeOldIndex(buf) + return idx, err == nil, err } - return nil, errors.Wrap(err, "Decode") + return nil, false, errors.Wrap(err, "DecodeIndex") } idx = NewIndex() @@ -571,11 +569,11 @@ func DecodeIndex(buf []byte) (idx *Index, err error) { idx.final = true debug.Log("done") - return idx, nil + return idx, false, nil } // DecodeOldIndex loads and unserializes an index in the old format from rd. -func DecodeOldIndex(buf []byte) (idx *Index, err error) { +func decodeOldIndex(buf []byte) (idx *Index, err error) { debug.Log("Start decoding old index") list := []*packJSON{} @@ -615,23 +613,3 @@ func DecodeOldIndex(buf []byte) (idx *Index, err error) { debug.Log("done") return idx, nil } - -// LoadIndexWithDecoder loads the index and decodes it with fn. -func LoadIndexWithDecoder(ctx context.Context, repo restic.Repository, buf []byte, id restic.ID, fn func([]byte) (*Index, error)) (*Index, []byte, error) { - debug.Log("Loading index %v", id) - - buf, err := repo.LoadAndDecrypt(ctx, buf[:0], restic.IndexFile, id) - if err != nil { - return nil, buf[:0], err - } - - idx, err := fn(buf) - if err != nil { - debug.Log("error while decoding index %v: %v", id, err) - return nil, buf[:0], err - } - - idx.ids = append(idx.ids, id) - - return idx, buf, nil -} diff --git a/internal/repository/index_test.go b/internal/repository/index_test.go index e65e663e4..a09f5866f 100644 --- a/internal/repository/index_test.go +++ b/internal/repository/index_test.go @@ -56,10 +56,11 @@ func TestIndexSerialize(t *testing.T) { err := idx.Encode(wr) rtest.OK(t, err) - idx2, err := repository.DecodeIndex(wr.Bytes()) + idx2, oldFormat, err := repository.DecodeIndex(wr.Bytes()) rtest.OK(t, err) rtest.Assert(t, idx2 != nil, "nil returned for decoded index") + rtest.Assert(t, !oldFormat, "new index format recognized as old format") wr2 := bytes.NewBuffer(nil) err = idx2.Encode(wr2) @@ -135,12 +136,13 @@ func TestIndexSerialize(t *testing.T) { rtest.OK(t, err) rtest.Equals(t, restic.IDs{id}, ids) - idx3, err := repository.DecodeIndex(wr3.Bytes()) + idx3, oldFormat, err := repository.DecodeIndex(wr3.Bytes()) rtest.OK(t, err) rtest.Assert(t, idx3 != nil, "nil returned for decoded index") rtest.Assert(t, idx3.Final(), "decoded index is not final") + rtest.Assert(t, !oldFormat, "new index format recognized as old format") // all new blobs must be in the index for _, testBlob := range newtests { @@ -285,8 +287,9 @@ var exampleLookupTest = struct { func TestIndexUnserialize(t *testing.T) { oldIdx := restic.IDs{restic.TestParseID("ed54ae36197f4745ebc4b54d10e0f623eaaaedd03013eb7ae90df881b7781452")} - idx, err := repository.DecodeIndex(docExample) + idx, oldFormat, err := repository.DecodeIndex(docExample) rtest.OK(t, err) + rtest.Assert(t, !oldFormat, "new index format recognized as old format") for _, test := range exampleTests { list := idx.Lookup(test.id, test.tpe, nil) @@ -338,7 +341,7 @@ func BenchmarkDecodeIndex(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { - _, err := repository.DecodeIndex(benchmarkIndexJSON) + _, _, err := repository.DecodeIndex(benchmarkIndexJSON) rtest.OK(b, err) } } @@ -349,15 +352,16 @@ func BenchmarkDecodeIndexParallel(b *testing.B) { b.RunParallel(func(pb *testing.PB) { for pb.Next() { - _, err := repository.DecodeIndex(benchmarkIndexJSON) + _, _, err := repository.DecodeIndex(benchmarkIndexJSON) rtest.OK(b, err) } }) } func TestIndexUnserializeOld(t *testing.T) { - idx, err := repository.DecodeOldIndex(docOldExample) + idx, oldFormat, err := repository.DecodeIndex(docOldExample) rtest.OK(t, err) + rtest.Assert(t, oldFormat, "old index format recognized as new format") for _, test := range exampleTests { list := idx.Lookup(test.id, test.tpe, nil) diff --git a/internal/repository/repository.go b/internal/repository/repository.go index 194d1e30c..d44e0fdda 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -457,14 +457,13 @@ func (r *Repository) LoadIndex(ctx context.Context) error { var buf []byte for fi := range ch { var err error - var idx *Index - idx, buf, err = LoadIndexWithDecoder(ctx, r, buf[:0], fi.ID, DecodeIndex) - if err != nil && errors.Cause(err) == ErrOldIndexFormat { - idx, buf, err = LoadIndexWithDecoder(ctx, r, buf[:0], fi.ID, DecodeOldIndex) - } - + buf, err = r.LoadAndDecrypt(ctx, buf[:0], restic.IndexFile, fi.ID) if err != nil { - return errors.Wrap(err, fmt.Sprintf("unable to load index %v", fi.ID.Str())) + return errors.Wrapf(err, "unable to load index %s", fi.ID.Str()) + } + idx, _, err := DecodeIndex(buf) + if err != nil { + return errors.Wrapf(err, "unable to decode index %s", fi.ID.Str()) } select { @@ -580,18 +579,16 @@ func (r *Repository) PrepareCache(indexIDs restic.IDSet) error { // LoadIndex loads the index id from backend and returns it. func LoadIndex(ctx context.Context, repo restic.Repository, id restic.ID) (*Index, error) { - idx, _, err := LoadIndexWithDecoder(ctx, repo, nil, id, DecodeIndex) - if err == nil { - return idx, nil + buf, err := repo.LoadAndDecrypt(ctx, nil, restic.IndexFile, id) + if err != nil { + return nil, err } - if errors.Cause(err) == ErrOldIndexFormat { + idx, oldFormat, err := DecodeIndex(buf) + if oldFormat { fmt.Fprintf(os.Stderr, "index %v has old format\n", id.Str()) - idx, _, err := LoadIndexWithDecoder(ctx, repo, nil, id, DecodeOldIndex) - return idx, err } - - return nil, err + return idx, err } // SearchKey finds a key with the supplied password, afterwards the config is From 720e0ee0c7274fe11b4a9b2831c562d8d8ffe8d3 Mon Sep 17 00:00:00 2001 From: greatroar <@> Date: Tue, 13 Oct 2020 20:56:43 +0200 Subject: [PATCH 2/2] if cond { return true }; return false => return cond --- internal/repository/index.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/internal/repository/index.go b/internal/repository/index.go index b247342e7..5164a4703 100644 --- a/internal/repository/index.go +++ b/internal/repository/index.go @@ -515,11 +515,8 @@ func (idx *Index) merge(idx2 *Index) error { // isErrOldIndex returns true if the error may be caused by an old index // format. func isErrOldIndex(err error) bool { - if e, ok := err.(*json.UnmarshalTypeError); ok && e.Value == "array" { - return true - } - - return false + e, ok := err.(*json.UnmarshalTypeError) + return ok && e.Value == "array" } // DecodeIndex unserializes an index from buf.