Fail check, restore and backup jobs when they encounter errors#1
Merged
Conversation
A check over a corrupt store, a restore where some entries failed to export, and a backup that couldn't read some files were all reported as successful. The job status comes from the error each operation returns to Main: a nil error becomes ReplySuccess. check counted snapshot.Check() failures into the report but always returned nil, so a store with missing or corrupt packfiles still "succeeded". It now returns an error when any snapshot fails its check. restore and backup both commit/return nil even when some entries could not be written or read: restore's per-entry failures live only in events (snapshot.Export never returns them), and backup's live in the report's error count that nothing downstream inspected. A new terminalError helper, run after listener.Wait() has drained the event bus, turns either into a job failure. This drops the previous backup success-with-warnings behaviour: a partial backup now fails like a partial restore or a failed check, rather than committing the snapshot and only warning. All three operations now share the same "a partial run is not a clean success" rule.
poolpOrg
force-pushed
the
fix/check-restore-job-status
branch
from
July 11, 2026 17:20
9eb9872 to
8e62b0b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A check over a corrupt store, a restore where some entries failed to export, and a backup that couldn't read some files were all being reported as successful. The job's status comes from the error each operation hands back to
Main— a nil error becomesReplySuccess— and none of these three surfaced their failures that way:snapshot.Check()failures into the report's error tally but always returnednil, so a store with missing or corrupt packfiles still came back green. It now returns an error when any snapshot fails its check.nileven when some entries couldn't be written or read: restore's per-entry failures live only in events (snapshot.Exportnever returns them), and backup's live in the report's error count that nothing downstream inspected. A newterminalErrorhelper — run afterlistener.Wait()has drained the event bus, the race-free point — turns either into a job failure.This intentionally drops backup's previous success-with-warnings behaviour: a partial backup now fails like a partial restore or a failed check, rather than committing the snapshot and only emitting a warning. All three operations now share one rule — a partial run is not a clean success.
terminalErroris a small pure helper so the verdict is unit-tested directly (terminal_error_test.go, replacing the oldbackup_warning_test.go).This mirrors the check/restore fix in
plakman/cmd/plaklet(PlakarKorp/plakman#1076); this repo'scmd/plakletis a parallel implementation with the same gaps. The plakman check path was validated end-to-end against a MinIO store: backup → check (passes) → remove packfiles → check reportsfailedwithcheck failed: 2 of 2 snapshot(s) had errors.Verified:
go build ./...,go vet ./...,go test ./...(32 tests pass).