From 22a3cea1b30db58deaaabd430a286f0141850514 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 27 Jan 2024 18:59:32 +0100 Subject: [PATCH 1/9] checker: wrap all pack errors in ErrPackData --- internal/checker/checker.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/checker/checker.go b/internal/checker/checker.go index 1e14a9e53..df126f539 100644 --- a/internal/checker/checker.go +++ b/internal/checker/checker.go @@ -521,7 +521,7 @@ func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []r debug.Log("checking pack %v", id.String()) if len(blobs) == 0 { - return errors.Errorf("pack %v is empty or not indexed", id) + return &ErrPackData{PackID: id, errs: []error{errors.New("pack is empty or not indexed")}} } // sanity check blobs in index @@ -542,7 +542,7 @@ func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []r var errs []error if nonContinuousPack { debug.Log("Index for pack contains gaps / overlaps, blobs: %v", blobs) - errs = append(errs, errors.New("Index for pack contains gaps / overlapping blobs")) + errs = append(errs, errors.New("index for pack contains gaps / overlapping blobs")) } // calculate hash on-the-fly while reading the pack and capture pack header @@ -591,21 +591,21 @@ func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []r if err != nil { // failed to load the pack file, return as further checks cannot succeed anyways debug.Log(" error streaming pack: %v", err) - return errors.Errorf("pack %v failed to download: %v", id, err) + return &ErrPackData{PackID: id, errs: append(errs, errors.Errorf("download error: %w", err))} } if !hash.Equal(id) { - debug.Log("Pack ID does not match, want %v, got %v", id, hash) - return errors.Errorf("Pack ID does not match, want %v, got %v", id, hash) + debug.Log("pack ID does not match, want %v, got %v", id, hash) + return &ErrPackData{PackID: id, errs: append(errs, errors.Errorf("unexpected pack id %v", hash))} } blobs, hdrSize, err := pack.List(r.Key(), bytes.NewReader(hdrBuf), int64(len(hdrBuf))) if err != nil { - return err + return &ErrPackData{PackID: id, errs: append(errs, err)} } if uint32(idxHdrSize) != hdrSize { debug.Log("Pack header size does not match, want %v, got %v", idxHdrSize, hdrSize) - errs = append(errs, errors.Errorf("Pack header size does not match, want %v, got %v", idxHdrSize, hdrSize)) + errs = append(errs, errors.Errorf("pack header size does not match, want %v, got %v", idxHdrSize, hdrSize)) } idx := r.Index() @@ -619,7 +619,7 @@ func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []r } } if !idxHas { - errs = append(errs, errors.Errorf("Blob %v is not contained in index or position is incorrect", blob.ID)) + errs = append(errs, errors.Errorf("blob %v is not contained in index or position is incorrect", blob.ID)) continue } } From 772e3416d1204bd43b539e9ffcfeb5c5602f02db Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 27 Jan 2024 18:59:54 +0100 Subject: [PATCH 2/9] repair pack: drop feature flag --- cmd/restic/cmd_check.go | 2 +- cmd/restic/cmd_repair_packs.go | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/cmd/restic/cmd_check.go b/cmd/restic/cmd_check.go index 22f462d75..21c9cc899 100644 --- a/cmd/restic/cmd_check.go +++ b/cmd/restic/cmd_check.go @@ -349,7 +349,7 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts GlobalOptions, args for _, id := range salvagePacks { strIDs = append(strIDs, id.String()) } - Warnf("RESTIC_FEATURES=repair-packs-v1 restic repair packs %v\nrestic repair snapshots --forget\n\n", strings.Join(strIDs, " ")) + Warnf("restic repair packs %v\nrestic repair snapshots --forget\n\n", strings.Join(strIDs, " ")) Warnf("Corrupted blobs are either caused by hardware problems or bugs in restic. Please open an issue at https://github.com/restic/restic/issues/new/choose for further troubleshooting!\n") } } diff --git a/cmd/restic/cmd_repair_packs.go b/cmd/restic/cmd_repair_packs.go index 04b06c33b..521b5859f 100644 --- a/cmd/restic/cmd_repair_packs.go +++ b/cmd/restic/cmd_repair_packs.go @@ -40,13 +40,6 @@ func init() { } func runRepairPacks(ctx context.Context, gopts GlobalOptions, term *termstatus.Terminal, args []string) error { - // FIXME discuss and add proper feature flag mechanism - flag, _ := os.LookupEnv("RESTIC_FEATURES") - if flag != "repair-packs-v1" { - return errors.Fatal("This command is experimental and may change/be removed without notice between restic versions. " + - "Set the environment variable 'RESTIC_FEATURES=repair-packs-v1' to enable it.") - } - ids := restic.NewIDSet() for _, arg := range args { id, err := restic.ParseID(arg) From 544fe38786eeab7aa4077a7c692d2f5e7d994ae8 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 27 Jan 2024 19:00:23 +0100 Subject: [PATCH 3/9] check: suggest repair pack for all damaged packs --- cmd/restic/cmd_check.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cmd/restic/cmd_check.go b/cmd/restic/cmd_check.go index 21c9cc899..8302c72bc 100644 --- a/cmd/restic/cmd_check.go +++ b/cmd/restic/cmd_check.go @@ -336,9 +336,7 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts GlobalOptions, args errorsFound = true Warnf("%v\n", err) if err, ok := err.(*checker.ErrPackData); ok { - if strings.Contains(err.Error(), "wrong data returned, hash is") { - salvagePacks = append(salvagePacks, err.PackID) - } + salvagePacks = append(salvagePacks, err.PackID) } } p.Done() From 6397615fbbca6425e71f4927e6b5c2188f363ed5 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 27 Jan 2024 19:04:45 +0100 Subject: [PATCH 4/9] check: document that check will show repair pack instructions --- doc/077_troubleshooting.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/077_troubleshooting.rst b/doc/077_troubleshooting.rst index 6a9a6ee15..512b41b0d 100644 --- a/doc/077_troubleshooting.rst +++ b/doc/077_troubleshooting.rst @@ -76,6 +76,8 @@ Similarly, if a repository is repeatedly damaged, please open an `issue on Githu somewhere. Please include the check output and additional information that might help locate the problem. +If ``check`` detects damaged pack files, it will show instructions on how to repair +them. Please follow the steps from the next section first. 2. Backup the repository ************************ From 4073299a7cba6874cfe30467cea86405027d8c42 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 4 Feb 2024 17:22:09 +0100 Subject: [PATCH 5/9] check: fix missing error if blob is invalid --- internal/checker/checker.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/checker/checker.go b/internal/checker/checker.go index df126f539..0fdd3d942 100644 --- a/internal/checker/checker.go +++ b/internal/checker/checker.go @@ -563,8 +563,8 @@ func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []r } debug.Log(" check blob %v: %v", val.Handle.ID, val.Handle) if val.Err != nil { - debug.Log(" error verifying blob %v: %v", val.Handle.ID, err) - errs = append(errs, errors.Errorf("blob %v: %v", val.Handle.ID, err)) + debug.Log(" error verifying blob %v: %v", val.Handle.ID, val.Err) + errs = append(errs, errors.Errorf("blob %v: %v", val.Handle.ID, val.Err)) } } From ed4a4f8748320a4aab2591c6d478fa442637f7de Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 10 Feb 2024 20:27:17 +0100 Subject: [PATCH 6/9] check: exclude inaccessible files from the repair pack suggestion --- internal/checker/checker.go | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/internal/checker/checker.go b/internal/checker/checker.go index 0fdd3d942..28f55ce3a 100644 --- a/internal/checker/checker.go +++ b/internal/checker/checker.go @@ -516,6 +516,14 @@ func (c *Checker) GetPacks() map[restic.ID]int64 { return c.packs } +type partialReadError struct { + err error +} + +func (e *partialReadError) Error() string { + return e.err.Error() +} + // checkPack reads a pack and checks the integrity of all blobs. func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []restic.Blob, size int64, bufRd *bufio.Reader, dec *zstd.Decoder) error { debug.Log("checking pack %v", id.String()) @@ -559,7 +567,7 @@ func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []r if err == repository.ErrPackEOF { break } else if err != nil { - return err + return &partialReadError{err} } debug.Log(" check blob %v: %v", val.Handle.ID, val.Handle) if val.Err != nil { @@ -574,7 +582,7 @@ func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []r if minHdrStart > curPos { _, err := bufRd.Discard(minHdrStart - curPos) if err != nil { - return err + return &partialReadError{err} } } @@ -582,16 +590,24 @@ func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []r var err error hdrBuf, err = io.ReadAll(bufRd) if err != nil { - return err + return &partialReadError{err} } hash = restic.IDFromHash(hrd.Sum(nil)) return nil }) if err != nil { + var e *partialReadError + isPartialReadError := errors.As(err, &e) // failed to load the pack file, return as further checks cannot succeed anyways - debug.Log(" error streaming pack: %v", err) - return &ErrPackData{PackID: id, errs: append(errs, errors.Errorf("download error: %w", err))} + debug.Log(" error streaming pack (partial %v): %v", isPartialReadError, err) + if isPartialReadError { + return &ErrPackData{PackID: id, errs: append(errs, errors.Errorf("partial download error: %w", err))} + } + + // The check command suggests to repair files for which a `ErrPackData` is returned. However, this file + // completely failed to download such that there's no point in repairing anything. + return errors.Errorf("download error: %w", err) } if !hash.Equal(id) { debug.Log("pack ID does not match, want %v, got %v", id, hash) From 527a3ff2b218902d9d4e3c6eab7e67b8b2edea74 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 10 Feb 2024 20:27:47 +0100 Subject: [PATCH 7/9] check: link to troubleshooting guide --- cmd/restic/cmd_check.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/restic/cmd_check.go b/cmd/restic/cmd_check.go index 8302c72bc..990702b61 100644 --- a/cmd/restic/cmd_check.go +++ b/cmd/restic/cmd_check.go @@ -342,7 +342,7 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts GlobalOptions, args p.Done() if len(salvagePacks) > 0 { - Warnf("\nThe repository contains pack files with damaged blobs. These blobs must be removed to repair the repository. This can be done using the following commands:\n\n") + Warnf("\nThe repository contains pack files with damaged blobs. These blobs must be removed to repair the repository. This can be done using the following commands. Please read the troubleshooting guide at https://restic.readthedocs.io/en/stable/077_troubleshooting.html first.\n\n") var strIDs []string for _, id := range salvagePacks { strIDs = append(strIDs, id.String()) From 69304cd74f9853c36fb6d34a07ae1a42aa552ff4 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 10 Feb 2024 20:29:11 +0100 Subject: [PATCH 8/9] check: clarify repair pack usage --- doc/077_troubleshooting.rst | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/doc/077_troubleshooting.rst b/doc/077_troubleshooting.rst index 512b41b0d..f80df29b8 100644 --- a/doc/077_troubleshooting.rst +++ b/doc/077_troubleshooting.rst @@ -77,7 +77,9 @@ somewhere. Please include the check output and additional information that might help locate the problem. If ``check`` detects damaged pack files, it will show instructions on how to repair -them. Please follow the steps from the next section first. +them using the ``repair pack`` command. Use that command instead of the "Repair the +index" section in this guide. + 2. Backup the repository ************************ @@ -106,6 +108,11 @@ whether your issue is already known and solved. Please take a look at the 3. Repair the index ******************* +.. note:: + + If the `check` command tells you to run `restic repair pack`, then use that + command instead. It will repair the damaged pack files and also update the index. + Restic relies on its index to contain correct information about what data is stored in the repository. Thus, the first step to repair a repository is to repair the index: From 0a36d193d84a13dea9911fb7ad9e071f24db40ec Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 12 Feb 2024 20:38:20 +0100 Subject: [PATCH 9/9] add changelog for enhanced repair packs --- changelog/unreleased/pull-4644 | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 changelog/unreleased/pull-4644 diff --git a/changelog/unreleased/pull-4644 b/changelog/unreleased/pull-4644 new file mode 100644 index 000000000..8000bce7e --- /dev/null +++ b/changelog/unreleased/pull-4644 @@ -0,0 +1,10 @@ +Enhancement: Improve `repair packs` command + +The `repair packs` command has been improved to also be able to process +truncated pack files. The `check --read-data` command will provide instructions +on using the command if necessary to repair a repository. See the guide at +https://restic.readthedocs.io/en/stable/077_troubleshooting.html for further +instructions. + +https://github.com/restic/restic/pull/4644 +https://github.com/restic/restic/pull/4655