From 929f90344ef8c145694fe5860615dbd49dc43b68 Mon Sep 17 00:00:00 2001 From: Pauline Middelink Date: Thu, 11 May 2017 22:26:44 +0200 Subject: [PATCH] Change backup policy to be inclusive, meaning all given policies are evaluated for each snapshot, thereby making sure that each keep-* is able to retain its most recent snapshot. Thereby insuring that weeklies keep Sundays around and monthlies keep the last day of the month around. Added testcase to make sure when multiple --keep-tags are given, ALL of them need to match. --- doc/manual.rst | 11 ++ src/restic/snapshot_filter.go | 147 ++++++------------- src/restic/snapshot_filter_test.go | 81 +++++----- src/restic/testdata/policy_keep_snapshots_0 | 13 +- src/restic/testdata/policy_keep_snapshots_10 | 12 +- src/restic/testdata/policy_keep_snapshots_13 | 17 +-- src/restic/testdata/policy_keep_snapshots_15 | 20 +-- src/restic/testdata/policy_keep_snapshots_17 | 22 +-- src/restic/testdata/policy_keep_snapshots_18 | 11 +- src/restic/testdata/policy_keep_snapshots_19 | 11 ++ src/restic/testdata/policy_keep_snapshots_3 | 13 +- src/restic/testdata/policy_keep_snapshots_4 | 13 +- src/restic/testdata/policy_keep_snapshots_9 | 22 +-- 13 files changed, 154 insertions(+), 239 deletions(-) create mode 100644 src/restic/testdata/policy_keep_snapshots_19 diff --git a/doc/manual.rst b/doc/manual.rst index cdf166c7a..10cd1282b 100644 --- a/doc/manual.rst +++ b/doc/manual.rst @@ -788,6 +788,9 @@ All the ``--keep-*`` options above only count hours/days/weeks/months/years which have a snapshot, so those without a snapshot are ignored. +All snapshots are evaluated counted against all matching keep-* counts. A +single snapshot on 30-09-2017 (Sun) will count as a daily, weekly and monthly. + Let's explain this with an example: Suppose you have only made a backup on each Sunday for 12 weeks. Then ``forget --keep-daily 4`` will keep the last four snapshots for the last four Sundays, but remove the rest. @@ -796,6 +799,14 @@ is a safety feature: it prevents restic from removing many snapshots when no new ones are created. If it was implemented otherwise, running ``forget --keep-daily 4`` on a Friday would remove all snapshots! +Another example: Suppose you make daily backups for 100 years. Then +``forget --keep-daily 7 --keep-weekly 5 --keep-monthly 12 --keep-yearly 75`` +will keep the most recent 7 daily snapshots, then 4 (remember, 7 dailies +already include a week!) last-day-of-the-weeks and 11 or 12 +last-day-of-the-months. (11 or 12 depends if the 5 weeklies cross a month). +And ofcourse 75 last-day-of-the-year snapshots. All other snapshots are +removed. + Autocompletion -------------- diff --git a/src/restic/snapshot_filter.go b/src/restic/snapshot_filter.go index 559150a5e..f56334428 100644 --- a/src/restic/snapshot_filter.go +++ b/src/restic/snapshot_filter.go @@ -1,7 +1,6 @@ package restic import ( - "fmt" "reflect" "sort" "time" @@ -52,18 +51,6 @@ func (e ExpirePolicy) Empty() bool { return reflect.DeepEqual(e, empty) } -// filter is used to split a list of snapshots into those to keep and those to -// remove according to a policy. -type filter struct { - Unprocessed Snapshots - Remove Snapshots - Keep Snapshots -} - -func (f filter) String() string { - return fmt.Sprintf("", len(f.Unprocessed), len(f.Keep), len(f.Remove)) -} - // ymdh returns an integer in the form YYYYMMDDHH. func ymdh(d time.Time) int { return d.Year()*1000000 + int(d.Month())*10000 + d.Day()*100 + d.Hour() @@ -90,84 +77,16 @@ func y(d time.Time) int { return d.Year() } -// apply moves snapshots from Unprocess to either Keep or Remove. It sorts the -// snapshots into buckets according to the return value of fn, and then moves -// the newest snapshot in each bucket to Keep and all others to Remove. When -// max snapshots were found, processing stops. -func (f *filter) apply(fn func(time.Time) int, max int) { - if max == 0 || len(f.Unprocessed) == 0 { - return - } +var a int - sameBucket := Snapshots{} - lastBucket := fn(f.Unprocessed[0].Time) - - for len(f.Unprocessed) > 0 { - cur := f.Unprocessed[0] - - bucket := fn(cur.Time) - - // if the snapshots are from a new bucket, forget all but the first - // (=last in time) snapshot from the previous bucket. - if bucket != lastBucket { - f.Keep = append(f.Keep, sameBucket[0]) - f.Remove = append(f.Remove, sameBucket[1:]...) - - sameBucket = Snapshots{} - lastBucket = bucket - max-- - - if max == 0 { - return - } - } - - // collect all snapshots for the current bucket - sameBucket = append(sameBucket, cur) - f.Unprocessed = f.Unprocessed[1:] - } - - // if we have leftovers, process them too. - if len(sameBucket) > 0 { - f.Keep = append(f.Keep, sameBucket[0]) - f.Remove = append(f.Remove, sameBucket[1:]...) - } +// always retuns a unique number for d. +func always(d time.Time) int { + a++ + return a } -// keepTags marks the snapshots which have all tags as to be kept. -func (f *filter) keepTags(tags []string) { - if len(tags) == 0 { - return - } - - unprocessed := f.Unprocessed[:0] - for _, sn := range f.Unprocessed { - if sn.HasTags(tags) { - f.Keep = append(f.Keep, sn) - continue - } - unprocessed = append(unprocessed, sn) - } - f.Unprocessed = unprocessed -} - -// keepLast marks the last n snapshots as to be kept. -func (f *filter) keepLast(n int) { - if n > len(f.Unprocessed) { - n = len(f.Unprocessed) - } - - f.Keep = append(f.Keep, f.Unprocessed[:n]...) - f.Unprocessed = f.Unprocessed[n:] -} - -// finish moves all remaining snapshots to remove. -func (f *filter) finish() { - f.Remove = append(f.Remove, f.Unprocessed...) -} - -// ApplyPolicy runs returns the snapshots from s that are to be deleted according -// to the policy p. s is sorted in the process. +// ApplyPolicy returns the snapshots from list that are to be kept and removed +// according to the policy p. list is sorted in the process. func ApplyPolicy(list Snapshots, p ExpirePolicy) (keep, remove Snapshots) { sort.Sort(list) @@ -179,20 +98,46 @@ func ApplyPolicy(list Snapshots, p ExpirePolicy) (keep, remove Snapshots) { return list, remove } - f := filter{ - Unprocessed: list, - Remove: Snapshots{}, - Keep: Snapshots{}, + var buckets = [6]struct { + Count int + bucker func(d time.Time) int + Last int + }{ + {p.Last, always, -1}, + {p.Hourly, ymdh, -1}, + {p.Daily, ymd, -1}, + {p.Weekly, yw, -1}, + {p.Monthly, ym, -1}, + {p.Yearly, y, -1}, } - f.keepTags(p.Tags) - f.keepLast(p.Last) - f.apply(ymdh, p.Hourly) - f.apply(ymd, p.Daily) - f.apply(yw, p.Weekly) - f.apply(ym, p.Monthly) - f.apply(y, p.Yearly) - f.finish() + for _, cur := range list { + var keep_snap bool - return f.Keep, f.Remove + // Tags are handled specially as they are not counted. + if len(p.Tags) > 0 { + if cur.HasTags(p.Tags) { + keep_snap = true + } + } + // Now update the other buckets and see if they have some counts left. + for i, b := range buckets { + if b.Count > 0 { + val := b.bucker(cur.Time) + if val != b.Last { + keep_snap = true + buckets[i].Last = val + buckets[i].Count-- + } + } + } + + if keep_snap { + keep = append(keep, cur) + } else { + remove = append(remove, cur) + } + } + + return keep, remove } diff --git a/src/restic/snapshot_filter_test.go b/src/restic/snapshot_filter_test.go index c31eeab57..d8bbd4ec8 100644 --- a/src/restic/snapshot_filter_test.go +++ b/src/restic/snapshot_filter_test.go @@ -77,8 +77,8 @@ var testExpireSnapshots = restic.Snapshots{ {Time: parseTimeUTC("2014-11-10 10:20:30"), Tags: []string{"foo"}}, {Time: parseTimeUTC("2014-11-12 10:20:30"), Tags: []string{"foo"}}, {Time: parseTimeUTC("2014-11-13 10:20:30"), Tags: []string{"foo"}}, - {Time: parseTimeUTC("2014-11-13 10:20:30")}, - {Time: parseTimeUTC("2014-11-15 10:20:30")}, + {Time: parseTimeUTC("2014-11-13 10:20:30"), Tags: []string{"bar"}}, + {Time: parseTimeUTC("2014-11-15 10:20:30"), Tags: []string{"foo", "bar"}}, {Time: parseTimeUTC("2014-11-18 10:20:30")}, {Time: parseTimeUTC("2014-11-20 10:20:30")}, {Time: parseTimeUTC("2014-11-21 10:20:30")}, @@ -164,57 +164,58 @@ var expireTests = []restic.ExpirePolicy{ {Yearly: 10}, {Daily: 7, Weekly: 2, Monthly: 3, Yearly: 10}, {Tags: []string{"foo"}}, + {Tags: []string{"foo", "bar"}}, } func TestApplyPolicy(t *testing.T) { for i, p := range expireTests { - keep, remove := restic.ApplyPolicy(testExpireSnapshots, p) + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + keep, remove := restic.ApplyPolicy(testExpireSnapshots, p) - t.Logf("test %d: returned keep %v, remove %v (of %v) expired snapshots for policy %v", - i, len(keep), len(remove), len(testExpireSnapshots), p) + t.Logf("test %d: returned keep %v, remove %v (of %v) expired snapshots for policy %v", + i, len(keep), len(remove), len(testExpireSnapshots), p) - if len(keep)+len(remove) != len(testExpireSnapshots) { - t.Errorf("test %d: len(keep)+len(remove) = %d != len(testExpireSnapshots) = %d", - i, len(keep)+len(remove), len(testExpireSnapshots)) - } + if len(keep)+len(remove) != len(testExpireSnapshots) { + t.Errorf("test %d: len(keep)+len(remove) = %d != len(testExpireSnapshots) = %d", + i, len(keep)+len(remove), len(testExpireSnapshots)) + } - if p.Sum() > 0 && len(keep) > p.Sum() { - t.Errorf("not enough snapshots removed: policy allows %v snapshots to remain, but ended up with %v", - p.Sum(), len(keep)) - } + if p.Sum() > 0 && len(keep) > p.Sum() { + t.Errorf("not enough snapshots removed: policy allows %v snapshots to remain, but ended up with %v", + p.Sum(), len(keep)) + } - for _, sn := range keep { - t.Logf("test %d: keep snapshot at %v %s\n", i, sn.Time, sn.Tags) - } - for _, sn := range remove { - t.Logf("test %d: forget snapshot at %v %s\n", i, sn.Time, sn.Tags) - } + for _, sn := range keep { + t.Logf("test %d: keep snapshot at %v %s\n", i, sn.Time, sn.Tags) + } + for _, sn := range remove { + t.Logf("test %d: forget snapshot at %v %s\n", i, sn.Time, sn.Tags) + } - goldenFilename := filepath.Join("testdata", fmt.Sprintf("policy_keep_snapshots_%d", i)) + goldenFilename := filepath.Join("testdata", fmt.Sprintf("policy_keep_snapshots_%d", i)) - if *updateGoldenFiles { - buf, err := json.MarshalIndent(keep, "", " ") + if *updateGoldenFiles { + buf, err := json.MarshalIndent(keep, "", " ") + if err != nil { + t.Fatalf("error marshaling result: %v", err) + } + + if err = ioutil.WriteFile(goldenFilename, buf, 0644); err != nil { + t.Fatalf("unable to update golden file: %v", err) + } + } + + buf, err := ioutil.ReadFile(goldenFilename) if err != nil { - t.Fatalf("error marshaling result: %v", err) + t.Fatalf("error loading golden file %v: %v", goldenFilename, err) } - if err = ioutil.WriteFile(goldenFilename, buf, 0644); err != nil { - t.Fatalf("unable to update golden file: %v", err) + var want restic.Snapshots + err = json.Unmarshal(buf, &want) + + if !reflect.DeepEqual(keep, want) { + t.Fatalf("test %v: wrong result, want:\n %v\ngot:\n %v", i, want, keep) } - } - - buf, err := ioutil.ReadFile(goldenFilename) - if err != nil { - t.Errorf("error loading golden file %v: %v", goldenFilename, err) - continue - } - - var want restic.Snapshots - err = json.Unmarshal(buf, &want) - - if !reflect.DeepEqual(keep, want) { - t.Errorf("test %v: wrong result, want:\n %v\ngot:\n %v", i, want, keep) - continue - } + }) } } diff --git a/src/restic/testdata/policy_keep_snapshots_0 b/src/restic/testdata/policy_keep_snapshots_0 index 261d70719..952f8c02e 100644 --- a/src/restic/testdata/policy_keep_snapshots_0 +++ b/src/restic/testdata/policy_keep_snapshots_0 @@ -317,12 +317,19 @@ { "time": "2014-11-15T10:20:30Z", "tree": null, - "paths": null + "paths": null, + "tags": [ + "foo", + "bar" + ] }, { "time": "2014-11-13T10:20:30Z", "tree": null, - "paths": null + "paths": null, + "tags": [ + "bar" + ] }, { "time": "2014-11-13T10:20:30Z", @@ -536,4 +543,4 @@ "tree": null, "paths": null } -] \ No newline at end of file +] diff --git a/src/restic/testdata/policy_keep_snapshots_10 b/src/restic/testdata/policy_keep_snapshots_10 index 72ae755ce..e2d61b68b 100644 --- a/src/restic/testdata/policy_keep_snapshots_10 +++ b/src/restic/testdata/policy_keep_snapshots_10 @@ -9,11 +9,6 @@ "tree": null, "paths": null }, - { - "time": "2016-01-12T21:02:03Z", - "tree": null, - "paths": null - }, { "time": "2016-01-09T21:02:03Z", "tree": null, @@ -53,10 +48,5 @@ "time": "2016-01-01T07:08:03Z", "tree": null, "paths": null - }, - { - "time": "2015-11-22T10:20:30Z", - "tree": null, - "paths": null } -] \ No newline at end of file +] diff --git a/src/restic/testdata/policy_keep_snapshots_13 b/src/restic/testdata/policy_keep_snapshots_13 index 93a52ad8c..abf4ed809 100644 --- a/src/restic/testdata/policy_keep_snapshots_13 +++ b/src/restic/testdata/policy_keep_snapshots_13 @@ -14,24 +14,9 @@ "tree": null, "paths": null }, - { - "time": "2016-01-08T20:02:03Z", - "tree": null, - "paths": null - }, { "time": "2016-01-03T07:02:03Z", "tree": null, "paths": null - }, - { - "time": "2015-11-22T10:20:30Z", - "tree": null, - "paths": null - }, - { - "time": "2015-11-15T10:20:30Z", - "tree": null, - "paths": null } -] \ No newline at end of file +] diff --git a/src/restic/testdata/policy_keep_snapshots_15 b/src/restic/testdata/policy_keep_snapshots_15 index 973e951ef..51bed0e6e 100644 --- a/src/restic/testdata/policy_keep_snapshots_15 +++ b/src/restic/testdata/policy_keep_snapshots_15 @@ -9,16 +9,6 @@ "tree": null, "paths": null }, - { - "time": "2016-01-09T21:02:03Z", - "tree": null, - "paths": null - }, - { - "time": "2016-01-03T07:02:03Z", - "tree": null, - "paths": null - }, { "time": "2015-11-22T10:20:30Z", "tree": null, @@ -43,13 +33,5 @@ "time": "2014-11-22T10:20:30Z", "tree": null, "paths": null - }, - { - "time": "2014-10-22T10:20:30Z", - "tree": null, - "paths": null, - "tags": [ - "foo" - ] } -] \ No newline at end of file +] diff --git a/src/restic/testdata/policy_keep_snapshots_17 b/src/restic/testdata/policy_keep_snapshots_17 index 553c18d89..8eb4ce6f8 100644 --- a/src/restic/testdata/policy_keep_snapshots_17 +++ b/src/restic/testdata/policy_keep_snapshots_17 @@ -34,16 +34,6 @@ "tree": null, "paths": null }, - { - "time": "2016-01-04T16:23:03Z", - "tree": null, - "paths": null - }, - { - "time": "2016-01-03T07:02:03Z", - "tree": null, - "paths": null - }, { "time": "2015-11-22T10:20:30Z", "tree": null, @@ -54,19 +44,9 @@ "tree": null, "paths": null }, - { - "time": "2015-09-22T10:20:30Z", - "tree": null, - "paths": null - }, - { - "time": "2015-08-22T10:20:30Z", - "tree": null, - "paths": null - }, { "time": "2014-11-22T10:20:30Z", "tree": null, "paths": null } -] \ No newline at end of file +] diff --git a/src/restic/testdata/policy_keep_snapshots_18 b/src/restic/testdata/policy_keep_snapshots_18 index 5898b0950..caf2b746e 100644 --- a/src/restic/testdata/policy_keep_snapshots_18 +++ b/src/restic/testdata/policy_keep_snapshots_18 @@ -1,4 +1,13 @@ [ + { + "time": "2014-11-15T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo", + "bar" + ] + }, { "time": "2014-11-13T10:20:30Z", "tree": null, @@ -111,4 +120,4 @@ "foo" ] } -] \ No newline at end of file +] diff --git a/src/restic/testdata/policy_keep_snapshots_19 b/src/restic/testdata/policy_keep_snapshots_19 new file mode 100644 index 000000000..177e8b400 --- /dev/null +++ b/src/restic/testdata/policy_keep_snapshots_19 @@ -0,0 +1,11 @@ +[ + { + "time": "2014-11-15T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo", + "bar" + ] + } +] diff --git a/src/restic/testdata/policy_keep_snapshots_3 b/src/restic/testdata/policy_keep_snapshots_3 index 261d70719..952f8c02e 100644 --- a/src/restic/testdata/policy_keep_snapshots_3 +++ b/src/restic/testdata/policy_keep_snapshots_3 @@ -317,12 +317,19 @@ { "time": "2014-11-15T10:20:30Z", "tree": null, - "paths": null + "paths": null, + "tags": [ + "foo", + "bar" + ] }, { "time": "2014-11-13T10:20:30Z", "tree": null, - "paths": null + "paths": null, + "tags": [ + "bar" + ] }, { "time": "2014-11-13T10:20:30Z", @@ -536,4 +543,4 @@ "tree": null, "paths": null } -] \ No newline at end of file +] diff --git a/src/restic/testdata/policy_keep_snapshots_4 b/src/restic/testdata/policy_keep_snapshots_4 index 261d70719..952f8c02e 100644 --- a/src/restic/testdata/policy_keep_snapshots_4 +++ b/src/restic/testdata/policy_keep_snapshots_4 @@ -317,12 +317,19 @@ { "time": "2014-11-15T10:20:30Z", "tree": null, - "paths": null + "paths": null, + "tags": [ + "foo", + "bar" + ] }, { "time": "2014-11-13T10:20:30Z", "tree": null, - "paths": null + "paths": null, + "tags": [ + "bar" + ] }, { "time": "2014-11-13T10:20:30Z", @@ -536,4 +543,4 @@ "tree": null, "paths": null } -] \ No newline at end of file +] diff --git a/src/restic/testdata/policy_keep_snapshots_9 b/src/restic/testdata/policy_keep_snapshots_9 index 0b577ae74..776816b08 100644 --- a/src/restic/testdata/policy_keep_snapshots_9 +++ b/src/restic/testdata/policy_keep_snapshots_9 @@ -28,25 +28,5 @@ "time": "2016-01-07T10:02:03Z", "tree": null, "paths": null - }, - { - "time": "2016-01-06T08:02:03Z", - "tree": null, - "paths": null - }, - { - "time": "2016-01-05T09:02:03Z", - "tree": null, - "paths": null - }, - { - "time": "2016-01-04T16:23:03Z", - "tree": null, - "paths": null - }, - { - "time": "2016-01-03T07:02:03Z", - "tree": null, - "paths": null } -] \ No newline at end of file +]