Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsafe defaults for Snapshot Maintenance #70

Open
chrestomanci opened this issue Dec 29, 2022 · 2 comments
Open

Unsafe defaults for Snapshot Maintenance #70

chrestomanci opened this issue Dec 29, 2022 · 2 comments
Labels
documentation Improvements or additions to documentation

Comments

@chrestomanci
Copy link

I have just installed restic-windows-backup on a machine that also dual boots to Linux, and had it delete most of my backup snapshots taken from Linux.

The issue appears to be that the script run prune then forget by default the first time it is run, there are no checks for deleting an excessive number of snapshots and the default Retention Policy says "--group-by host,tags" when the path is also significant for backups taken under Linux

Please change the default configuration so that SnapshotMaintenanceEnabled is set to false and add "--dry-run" to the default SnapshotRetentionPolicy, then in the Readme.md suggest that after installation the user should:

  1. Run the backup by hand in the PowerShell by running backup.ps1 and checking the output for errors
  2. Wait a week or so for some scheduled nightly backups to run.
  3. Change SnapshotMaintenanceEnabled to true, run backup.ps1 by hand and check that the proposed retention is sensible
  4. Only then, remove the "--dry-run" flag from SnapshotRetentionPolicy

You should also add a footnote to the Readme.md to warn of this issue on machines that dual boot.

I think that these defaults would be much safer than current. It after installation the user forgets to turn on SnapshotMaintenanceEnabled, then they will end up with hundreds of snapshots, that restic will cope with and only cause a small loss of performance. The current default is dangerous.

I can prepare a PR with these proposed changes if you like.

@kmwoley
Copy link
Owner

kmwoley commented Jan 12, 2023 via email

@chrestomanci
Copy link
Author

While adding a warning to the bottom of the readme is a useful step, and would have averted the problem for me because I do read them, I am still concerned that the proposed defaults are unsafe. I think that as a minimum you should change the default --group-by flag to host,tags,paths - It would make no difference in the default case of one backup taken from windows only, but would protect other backups, not just in the dual boot scenario but also if the user adds additional or more frequent backups of an important work directory.

I understand that your aim is to create a setup and forget backup solution, and it does sort of work that way in the simplest default case, but a prudent admin should not run with the default setup at first because it might not work or could do unexpected things and the log output from the windows scheduler is difficult or impossible to obtain.

My advice to a more careful sysadmin would be to skip the windows scheduler part of the setup, and also to and add "--dry-run" to the default SnapshotRetentionPolicy, and run the backup for the first time in a powershell console window monitoring the output, as the backup could easily and silently fail due to any number of miss configurations. (eg, wrong credentials, wrong destination, too much or too little data backed up).

Once that initial run has completed without anything unexpected then it would make sense to enable scheduled backups and automatic prune & forget but without that first test there are too many ways that the backup could go wrong so that it is not there when needed.

Would you agree that this is sensible advice, and adjust the Readme to reflect that ?

@kmwoley kmwoley added the documentation Improvements or additions to documentation label Jan 15, 2023
living180 added a commit to living180/restic-windows-backup that referenced this issue Mar 4, 2024
Address kmwoley#70 by only pruning snapshots from the current host by default when performing maintenance.  This avoids inadvertently pruning snapshots from other hosts when that might not be desired.
kmwoley added a commit that referenced this issue Nov 10, 2024
* Limit snapshot pruning to the current host

Address #70 by only pruning snapshots from the current host by default when performing maintenance.  This avoids inadvertently pruning snapshots from other hosts when that might not be desired.

* add back 'host' to group-by

I think it's safer to keep `--group-by 'host,tags'` even if the `--host` parameter is provided. This makes sure that the `forget` always groups together snapshots from the same host. It may be unneeded.

---------

Co-authored-by: Kevin Woley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants