Could we please discuss first how such a command should look like? How does this relate to the rewrite command in #2731 ? What if the prune command could handle certain types of repository damages? Which types of repair commands for a repository do we need and how to avoid cluttering the CLI?

That said, with the current flood of new pull requests which is growing much faster than we can handle them and the still huge pile of older ones waiting for a review, it will quite likely take some time until anyone of the maintainers has time to participate in that discussion.

This PR does not fix the underlying issue of #2334, it merely removes the symptoms (the broken file). But it's still not possible to restore at least the remaining parts of a file.

I thought there would be some changes appearing in near future on either rewrite or prune, so I waited with an answer.

So, basically the point is that restic is missing such kind of functionality and IMO this is critical as cases has already been reported where users are struggling with broken repos which can only be fixed by forgetting and pruning whole snapshots. This is why I proposed this PR.

About the name and where to put such functionality we can and should of course discuss and I'm fine to have the discussion here. I agree that the functionality is pretty similar to the rewrite command proposed in #2731 and maybe adding some flags there would solve the issue similarly. I however don't think that this functionality should be included into prune: prune removes data and should start only if the repository is sane. This functionality is proposed to help users the possibly best way if their repository is no sane to put it back into a sane state.
For now, we already do have a recover command which somehow stands beside the other restic commands and seems to be a "quick" solution for a problem that occured. So I used a new command in this "tradition".

I must admit that there is another reason why I used a new command repair and it is the situation you depicted, namely that I must take into account that it might take a long time before this PR is getting discussed or merged. In this situation, I really do want to have a patch that does not depend on another WIP patch (i.e. #2731) or is expected to need rebasing after some other PRs get merged into master. This is a patch that is able to help users who need a quick solution for a broken repo and therefore should stay easily mergeable. This can be best guaranteed by only adding new files to the source code.

This PR does not fix the underlying issue of #2334, it merely removes the symptoms (the broken file). But it's > still not possible to restore at least the remaining parts of a file.

If --append is given (defaults to ".repaired") this repair command will generate a " file.repaired" which contains the remaining parts of that file. Only if no parts are left to repair the file, it will delete the file (as shown in the example output where /home/thinkpad/data/test was a 1-blob file and that blob was missing in the repo).

So, basically the point is that restic is missing such kind of functionality and IMO this is critical as cases has already been reported where users are struggling with broken repos which can only be fixed by forgetting and pruning whole snapshots. This is why I proposed this PR.

It's definitely useful to have a WIP PR to point people to to repair damaged repositories.

I agree that the functionality is pretty similar to the rewrite command proposed in #2731 and maybe adding some flags there would solve the issue similarly.

From an implementation perspective both commands are pretty similar so it would be useful to make the recursive tree rewriting part reusable for both commands. (Assuming we end up with two separate commands)

I however don't think that this functionality should be included into prune

With "handle certain types of repository damages" I had in mind that it should be possible to let the prune command tolerate (= be able to remove unused blobs/packs) repositories which e.g. miss a few data blobs. Of course, the prune command shouldn't start to modify snapshots.

If --append is given (defaults to ".repaired") this repair command will generate a " file.repaired" which contains the remaining parts of that file. Only if no parts are left to repair the file, it will delete the file (as shown in the example output where /home/thinkpad/data/test was a 1-blob file and that blob was missing in the repo).

I've missed the --append option and yes it helps a lot if blob sizes are missing, but I'd still consider this as fixing the symptoms. The underlying problem I had in mind is the repository format itself: it can't store the size of missing blobs without risking further damaged snapshots, but when rebuilding the index to fix this we loose the information about the position of the data blobs.

With "handle certain types of repository damages" I had in mind that it should be possible to let the prune command tolerate (= be able to remove unused blobs/packs) repositories which e.g. miss a few data blobs. Of course, the prune command shouldn't start to modify snapshots.

You are right - in general missing data blobs are not a that big issue when thinking about the sanity of the repository. Of course, data might be lost which might be a bad thing for users. But starting with a repository where you have missing data blobs, probably the best is actually to remove them from the index but let the trees still reference them. If another backup run will stumble over the missing data, the repository will be healed. And if not, all functionality is working perfectly except if you need to access that specific data, i.e. a file which contains a missing blob. (and even restore and mount could work around this and include the functionality I propose here for missing blobs)

Another issue is missing tree blobs. This severely damages the repository as most commands like check and prune will no longer work. For this kind of damage IMO it is best to modify trees and snapshots to remove these damaged subtrees which is a much better solution than complete delete the snapshot.

I've missed the --append option and yes it helps a lot if blob sizes are missing, but I'd still consider this as fixing the symptoms. The underlying problem I had in mind is the repository format itself: it can't store the size of missing blobs without risking further damaged snapshots, but when rebuilding the index to fix this we loose the information about the position of the data blobs.

I recently thought about a check when loading the index whether the pack files referenced in the index still exist in the backend. This would allow users to just remove damaged packs (missing packs would be already detected) and then an information in the index could be saved whether the blob is present or only the length saved in the index may be used. I rejected to implement this as a regular check as it would mean that every 'LoadIndex' would need to 'List' the pack files which could be expensive. Maybe that might be a direction to think of?

I recently thought about a check when loading the index whether the pack files referenced in the index still exist in the backend. This would allow users to just remove damaged packs (missing packs would be already detected) and then an information in the index could be saved whether the blob is present or only the length saved in the index may be used. I rejected to implement this as a regular check as it would mean that every 'LoadIndex' would need to 'List' the pack files which could be expensive. Maybe that might be a direction to think of?

I don't think we want to list all pack files every time the repository index is loaded, it really sounds (needlessly) expensive. It would also only be a partial workaround, as having to rebuild the repository index would still loose the information. And we'd also get into a lot of trouble when mixing older and newer restic version which have differing opinions on which blobs actually exist and which don't. The only way to properly fix this is to handle this in a repository format v2 #628 .

For the second time I have a corrupted repo - too bad. Probably the reason was that the backup disk was full.
I tried ./restic -r$REPO repair and it proposes fixes, as expected.

However, when I run 'make test' I get
? github.com/restic/restic/internal/limiter [no test files]
? github.com/restic/restic/internal/migrations [no test files]
? github.com/restic/restic/internal/mock [no test files]
--- FAIL: TestOptionsApplyInvalid (0.00s)
--- FAIL: TestOptionsApplyInvalid/test-2 (0.00s)
options_test.go:216: expected error "time: missing unit in duration 2134", got "time: missing unit in duration "2134""
FAIL
FAIL github.com/restic/restic/internal/options 0.014s
ok github.com/restic/restic/internal/pack 0.018s

other tests are o.k.

Tested on ARCH 5.8.7-arch1-1

Hi @pddp
thanks for reporting the issue!

FAIL github.com/restic/restic/internal/options 0.014s

Actually, this PR doesn't change anything on /internal/options. Did you redo your test with master? Seems to me that this might be an uncorrelated issue..

Hi @aawsome

your guess was correct - this was uncorrelated. Also happens on master.

My repo seems to be corrupted after I ran rebuild-index. Is it a bad idea to check out this branch and invoke the repair command?

LGTM.

Just wanted to say that this PR saved me a lot of work. I had a corrupted repository, I couldn't figure out the issue, and every snapshot seemed to be corrupt missing at least one blob. By rebuilding restic from source to include this PR, I was able to fix the repository very easily, and have now recovered about 95% of the original files.

Thank you so much @aawsome and all others involved! This is evidently a very valuable feature for restic, it is greatly appreciated.