Repository navigation
fix(backup): refuse incomplete restore points after interruption - #1579
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8b2580cb4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| func currentRestoreStateComplete(dataDir string) bool { | ||
| entries, err := os.ReadDir(dataDir) | ||
| return err == nil && !hasRestoreStaging(entries) && checkRestorePointsBeforeRestore(dataDir) == nil |
There was a problem hiding this comment.
In the native ftw-backup revert path, a SIGKILL after the rejected state is marked complete but before the previous files are reactivated leaves the active root empty while both restore-point directories verify successfully. On retry, this predicate therefore returns true, records the empty root as a complete safety point, and reports success; reverting that returned point again then activates an empty installation. Track the in-progress revert independently rather than inferring active-root completeness solely from the validity of sibling points.
Useful? React with 👍 / 👎.
|
|
||
| func hasRestoreStaging(entries []os.DirEntry) bool { | ||
| for _, entry := range entries { | ||
| if strings.HasPrefix(entry.Name(), ".ftw-restore-stage-") { |
There was a problem hiding this comment.
When restore is killed during extraction, this stage directory remains, and RestoreContents explicitly permits a retry but never removes the old stage. Even after that retry successfully activates a complete archive, a later revert sees the stale directory here and marks the rejected archive state incomplete, so it cannot be switched back to and all subsequent restores into this data directory are refused. Remove or otherwise distinguish pre-retry extraction stages once a retry completes successfully.
Useful? React with 👍 / 👎.
A process kill while
ftw-backup restorepreserves the current directory can leave only part of it in the safety point. The old helper then acceptsrevertand reports success with that subset. A local reproduction retained all 12,000 original files but reactivated only 68.Record the expected file list before moving files and confirm it only after the complete previous directory is durable. Revert checks the record before any move; retry refuses unresolved points. Recovery after a kill during activation can restore the complete original while retaining the partial target without certifying it. Failed revert rollback retains new readings and cannot overwrite an original that failed to move back.
Older full backup archives still import into a separate target. Older safety points without a completeness record now stop before revert or another import; keep Core stopped, preserve both directories and recover from a verified full backup in a separate directory.
Validation: real subprocess SIGKILL tests during preservation and activation, malformed and missing records, failed rollback and shared-filename regressions, normal revert in both directions, and full
make verify. Separate local CLI interruption checks use synthetic data and an older helper archive; they are not published Linux package, physical power-loss or installation-cutover evidence.This changes the installed helper and requires a new exact beta and a new 48-hour field trial. Holds #1554 and #1556 remain; this does not explain the first history writer delay. Guide #1530 owns the operator documentation and will follow the qualified beta.