Merge pull request #4366 from gautammenghani/master

Issue #3624: Preserve oldest snapshot when keep-* values are not sati…

changelog/unreleased/issue-3624

@@ -0,0 +1,9 @@
+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
+https://forum.restic.net/t/keeping-yearly-snapshots-policy-when-backup-began-during-the-year/4670/2

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,7 +257,9 @@ 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 {
+				// 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
 					buckets[i].Last = val
@@ -275,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

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
+      }
     }
   ]
 }

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
+      }
     }
   ]
 }

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": {}
     }
   ]
 }

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}
     }
   ]
 }