From 182b9796e4135b756d90e7c68f2faaad89fa6ebe Mon Sep 17 00:00:00 2001 From: Gautam Menghani Date: Fri, 16 Jun 2023 23:13:41 +0530 Subject: [PATCH 1/4] Issue #3624: Preserve oldest snapshot when keep-* values are not satisfied --- changelog/unreleased/issue-3624 | 12 ++++++++++++ internal/restic/snapshot_policy.go | 2 +- .../restic/testdata/policy_keep_snapshots_16 | 18 ++++++++++++++++++ .../restic/testdata/policy_keep_snapshots_17 | 18 ++++++++++++++++++ .../restic/testdata/policy_keep_snapshots_39 | 17 +++++++++++++++++ 5 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 changelog/unreleased/issue-3624 diff --git a/changelog/unreleased/issue-3624 b/changelog/unreleased/issue-3624 new file mode 100644 index 000000000..53837e61c --- /dev/null +++ b/changelog/unreleased/issue-3624 @@ -0,0 +1,12 @@ +Enhancement: Keep oldest snapshot when there aren't enough snapshots + +The `forget` command does not preserve the oldest snapshot incase the +keep-* parameters are not satisfied, which led to users not being able to +preserve old data. Now, restic will always preserve the oldest snapshot +whenever any of the keep-* options to the `forget` command are not +satisfied. + + +https://github.com/restic/restic/issues/3624 +https://github.com/restic/restic/pull/4366 +https://forum.restic.net/t/keeping-yearly-snapshots-policy-when-backup-began-during-the-year/4670/2 diff --git a/internal/restic/snapshot_policy.go b/internal/restic/snapshot_policy.go index bec594707..1d6e129a2 100644 --- a/internal/restic/snapshot_policy.go +++ b/internal/restic/snapshot_policy.go @@ -256,7 +256,7 @@ func ApplyPolicy(list Snapshots, p ExpirePolicy) (keep, remove Snapshots, reason // -1 means "keep all" if b.Count > 0 || b.Count == -1 { val := b.bucker(cur.Time, nr) - if val != b.Last { + if val != b.Last || nr == len(list)-1 { debug.Log("keep %v %v, bucker %v, val %v\n", cur.Time, cur.id.Str(), i, val) keepSnap = true buckets[i].Last = val diff --git a/internal/restic/testdata/policy_keep_snapshots_16 b/internal/restic/testdata/policy_keep_snapshots_16 index d0cae94b5..da6f43a1c 100644 --- a/internal/restic/testdata/policy_keep_snapshots_16 +++ b/internal/restic/testdata/policy_keep_snapshots_16 @@ -14,6 +14,11 @@ "time": "2014-11-22T10:20:30Z", "tree": null, "paths": null + }, + { + "time": "2014-08-08T10:20:30Z", + "tree": null, + "paths": null } ], "reasons": [ @@ -55,6 +60,19 @@ "counters": { "yearly": 7 } + }, + { + "snapshot": { + "time": "2014-08-08T10:20:30Z", + "tree": null, + "paths": null + }, + "matches": [ + "yearly snapshot" + ], + "counters": { + "yearly": 6 + } } ] } \ No newline at end of file diff --git a/internal/restic/testdata/policy_keep_snapshots_17 b/internal/restic/testdata/policy_keep_snapshots_17 index 742b8005b..ee728d4e0 100644 --- a/internal/restic/testdata/policy_keep_snapshots_17 +++ b/internal/restic/testdata/policy_keep_snapshots_17 @@ -49,6 +49,11 @@ "time": "2014-11-22T10:20:30Z", "tree": null, "paths": null + }, + { + "time": "2014-08-08T10:20:30Z", + "tree": null, + "paths": null } ], "reasons": [ @@ -201,6 +206,19 @@ "counters": { "yearly": 7 } + }, + { + "snapshot": { + "time": "2014-08-08T10:20:30Z", + "tree": null, + "paths": null + }, + "matches": [ + "yearly snapshot" + ], + "counters": { + "yearly": 6 + } } ] } \ No newline at end of file diff --git a/internal/restic/testdata/policy_keep_snapshots_39 b/internal/restic/testdata/policy_keep_snapshots_39 index a8e6ca827..4b111503b 100644 --- a/internal/restic/testdata/policy_keep_snapshots_39 +++ b/internal/restic/testdata/policy_keep_snapshots_39 @@ -57,6 +57,11 @@ "time": "2014-08-22T10:20:30Z", "tree": null, "paths": null + }, + { + "time": "2014-08-08T10:20:30Z", + "tree": null, + "paths": null } ], "reasons": [ @@ -189,6 +194,18 @@ "monthly snapshot" ], "counters": {"Monthly": -1, "Yearly": -1} + }, + { + "snapshot": { + "time": "2014-08-08T10:20:30Z", + "tree": null, + "paths": null + }, + "matches": [ + "monthly snapshot", + "yearly snapshot" + ], + "counters": {"Monthly": -1, "Yearly": -1} } ] } \ No newline at end of file From 1257c2c075ea866cb589fa3ec570a6f8bd11a14d Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 16 Jun 2023 21:28:49 +0200 Subject: [PATCH 2/4] forget: Add comments to snapshot policy --- internal/restic/snapshot_policy.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/restic/snapshot_policy.go b/internal/restic/snapshot_policy.go index 1d6e129a2..22939bd6c 100644 --- a/internal/restic/snapshot_policy.go +++ b/internal/restic/snapshot_policy.go @@ -183,6 +183,7 @@ type KeepReason struct { // according to the policy p. list is sorted in the process. reasons contains // the reasons to keep each snapshot, it is in the same order as keep. func ApplyPolicy(list Snapshots, p ExpirePolicy) (keep, remove Snapshots, reasons []KeepReason) { + // sort newest snapshots first sort.Stable(list) if p.Empty() { @@ -256,6 +257,8 @@ func ApplyPolicy(list Snapshots, p ExpirePolicy) (keep, remove Snapshots, reason // -1 means "keep all" if b.Count > 0 || b.Count == -1 { val := b.bucker(cur.Time, nr) + // also keep the oldest snapshot if the bucket has some counts left. This maximizes the + // the history length kept while some counts are left. if val != b.Last || nr == len(list)-1 { debug.Log("keep %v %v, bucker %v, val %v\n", cur.Time, cur.id.Str(), i, val) keepSnap = true From 3888c21a27d807cba58f1dcc56e455b3adab40c9 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 16 Jun 2023 21:35:20 +0200 Subject: [PATCH 3/4] reword changelog --- changelog/unreleased/issue-3624 | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/changelog/unreleased/issue-3624 b/changelog/unreleased/issue-3624 index 53837e61c..ce3fe57aa 100644 --- a/changelog/unreleased/issue-3624 +++ b/changelog/unreleased/issue-3624 @@ -1,11 +1,8 @@ -Enhancement: Keep oldest snapshot when there aren't enough snapshots - -The `forget` command does not preserve the oldest snapshot incase the -keep-* parameters are not satisfied, which led to users not being able to -preserve old data. Now, restic will always preserve the oldest snapshot -whenever any of the keep-* options to the `forget` command are not -satisfied. +Enhancement: Keep oldest snapshot when there are not enough snapshots +The `forget` command now additionally preserves the oldest snapshot if fewer +snapshots are kept than allowed by the `--keep-*` parameters. This maximizes +amount of history kept while the specified limits are not yet reached. https://github.com/restic/restic/issues/3624 https://github.com/restic/restic/pull/4366 From 8da5a6649bd2016a164b6bb471bafaf7fe2d3d1a Mon Sep 17 00:00:00 2001 From: Gautam Menghani Date: Sat, 17 Jun 2023 18:04:35 +0530 Subject: [PATCH 4/4] Preserve oldest snapshot when keep-within* does not collect enough --- internal/restic/snapshot_policy.go | 2 +- .../restic/testdata/policy_keep_snapshots_35 | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/internal/restic/snapshot_policy.go b/internal/restic/snapshot_policy.go index 22939bd6c..0ff0c5ec8 100644 --- a/internal/restic/snapshot_policy.go +++ b/internal/restic/snapshot_policy.go @@ -278,7 +278,7 @@ func ApplyPolicy(list Snapshots, p ExpirePolicy) (keep, remove Snapshots, reason if cur.Time.After(t) { val := b.bucker(cur.Time, nr) - if val != b.Last { + if val != b.Last || nr == len(list)-1 { debug.Log("keep %v, time %v, ID %v, bucker %v, val %v %v\n", b.reason, cur.Time, cur.id.Str(), i, val, b.Last) keepSnap = true bucketsWithin[i].Last = val diff --git a/internal/restic/testdata/policy_keep_snapshots_35 b/internal/restic/testdata/policy_keep_snapshots_35 index a4def907a..ece4ddbd2 100644 --- a/internal/restic/testdata/policy_keep_snapshots_35 +++ b/internal/restic/testdata/policy_keep_snapshots_35 @@ -44,6 +44,11 @@ "time": "2014-11-22T10:20:30Z", "tree": null, "paths": null + }, + { + "time": "2014-08-08T10:20:30Z", + "tree": null, + "paths": null } ], "reasons": [ @@ -152,6 +157,17 @@ "yearly within 9999y" ], "counters": {} + }, + { + "snapshot": { + "time": "2014-08-08T10:20:30Z", + "tree": null, + "paths": null + }, + "matches": [ + "yearly within 9999y" + ], + "counters": {} } ] } \ No newline at end of file