Merge lp:~mterry/deja-dup/simple-threshold into lp:deja-dup/24

[Michael Terry]

Some backstory:

Deja Dup needs to make the occasional fresh, full backup. Further, it makes sure to always keep two of these around.

It does this so that it can delete old complete chains (if your backup was just one full backup a year ago and incrementals since, it couldn't delete any of it). And so that if something went wrong with one of your backups, you'll always have a second. Plus, upstream duplicity recommends the practice.

However, users have been complaining that Deja Dup makes a full backup too frequently (typically when they change the schedule to daily, which triggers monthly full backups). And some administrators have requested the ability to tune this value for their users.

This branch does a few things:

1) Allow a user/administrator to set (only via gsettings) the "how many days since the last full backup?" threshold for a new full backup.
2) Set that new setting to 90 (three months, which is the same default for weekly backups as before, but for users with daily backups, this represents a change from one month).
3) Removes UI options to backup at "two weeks" and "monthly" intervals (keeping "daily" and the default "weekly"). The user can still set a custom value in gsettings.
4) Removes UI options to only hold backups for "one month", "two months", and "three months" (keeping "six months", "one year", and the default "forever"). The user can still set a custom value in gsettings.
5) Vastly simplifies the existing 'threshold' logic to just look up in gsettings what the value should be. Previously it calculated it based on the user's delete date (divide by 2) and backup interval (multiply by 12) with clamps of (1 month, 6 months).

The reason for 3 is because I've been trying to encourage shorter backup intervals. This seemed like a good time to drop those options, while I was futzing with these things.

The reason for 4 is because if we're always keeping two full backups, spaced three months apart, the soonest we will delete anything is six months.

And while I was at it, I converted the relevant old-style test to a new-style one.

[lp:~mterry/deja-dup/simple-threshold]

1354. By Michael Terry on 2012-06-21

whoops, forgot to update the new test thresholds to match 90 instead of 84

[Ken VanDine (ken-vandine)]

I'm having trouble figuring out how to run the new style tests, in the tests/scripts directory. It looks like I should run "make check" in the tests/runner directory, but they all fail. Tips?

[Michael Terry (mterry)]

Yeah, make check should work. Maybe they assume something that only happens to be true for my dev machine. Let's chat on IRC about it.

[Ken VanDine (ken-vandine)]

My test failures were due to ~/Downloads being a symlink on my box, tests pass on another box.

Looks good!

review: Approve

=== modified file 'common/CommonUtils.vala'
--- common/CommonUtils.vala	2012-04-30 06:19:13 +0000
+++ common/CommonUtils.vala	2012-06-21 17:50:33 +0000
@@ -33,6 +33,7 @@
 public const string PERIODIC_KEY = "periodic";
 public const string PERIODIC_PERIOD_KEY = "periodic-period";
 public const string DELETE_AFTER_KEY = "delete-after";
+public const string FULL_BACKUP_PERIOD_KEY = "full-backup-period";
 
 public errordomain BackupError {
   BAD_CONFIG,
@@ -501,43 +502,22 @@
 
 public int get_full_backup_threshold()
 {
-  int threshold = 7 * 6; // default to 6 weeks
   // So, there are a few factors affecting how often to make a fresh full
   // backup:
+  //
   // 1) The longer we wait, the more we're filling up the backend with 
   //    iterations on the same crap.
   // 2) The longer we wait, there's a higher risk that some bit will flip
-  //    and the whole backup is toast.
+  //    and the whole incremental chain afterwards is toast.
   // 3) The longer we wait, the less annoying we are, since full backups 
   //    take a long time.
-  // So we try to do them at reasonable times.  But almost nobody should be
-  // going longer than 6 months without a full backup.  Further, we want
-  // to try to keep at least 2 full backups around, so also don't allow a
-  // longer full threshold than half the delete age.
-  // 
-  // 'daily' gets 2 weeks: 1 * 12 => 2 * 7
-  // 'weekly' gets 3 months: 7 * 12
-  // 'biweekly' gets 6 months: 14 * 12
-  // 'monthly' gets 6 months: 28 * 12 => 24 * 7
-  var max = 24 * 7; // 6 months
-  var min = 4 * 7; // 4 weeks
-  var scale = 12;
-  var min_fulls = 2;
-  
+  //
+  // We default to 3 months.
+
   var settings = get_settings();
-  var delete_age = settings.get_int(DELETE_AFTER_KEY);
-  if (delete_age > 0)
-    max = int.max(int.min(delete_age/min_fulls, max), min);
-  
-  var periodic = settings.get_boolean(PERIODIC_KEY);
-  if (periodic) {
-    var period = settings.get_int(PERIODIC_PERIOD_KEY);
-    threshold = period * scale;
-    threshold.clamp(min, max);
-  }
-  else
-    threshold = max;
-  
+  var threshold = settings.get_int(FULL_BACKUP_PERIOD_KEY);
+  if (threshold < 1)
+    threshold = 84; // 3 months
   return threshold;
 }
 

=== modified file 'data/org.gnome.DejaDup.gschema.xml.in'
--- data/org.gnome.DejaDup.gschema.xml.in	2011-08-31 03:50:06 +0000
+++ data/org.gnome.DejaDup.gschema.xml.in	2012-06-21 17:50:33 +0000
@@ -55,6 +55,11 @@
       <_summary>How long to keep backup files</_summary>
       <_description>The number of days to keep backup files on the backup location.  A value of 0 means forever.  This is a minimum number of days; the files may be kept longer.</_description>
     </key>
+    <key name="full-backup-period" type="i">
+      <default>90</default>
+      <_summary>How long to wait between full backups</_summary>
+      <_description>Déjà Dup needs to occasionally make fresh full backups.  This is the number of days to wait between full backups.</_description>
+    </key>
     <key name="backend" type="s">
       <choices>
         <choice value='auto'/>

=== modified file 'tests/Makefile.am'
--- tests/Makefile.am	2012-04-18 14:02:29 +0000
+++ tests/Makefile.am	2012-06-21 17:50:33 +0000
@@ -29,7 +29,6 @@
 	backup/delete \
 	backup/encrypt \
 	backup/exclude \
-	backup/full \
 	backup/mkdir \
 	backup/permissions \
 	backup/quiescent \

=== removed file 'tests/backup/full'
--- tests/backup/full	2011-12-06 15:20:21 +0000
+++ tests/backup/full	1970-01-01 00:00:00 +0000
@@ -1,89 +0,0 @@
-#!/usr/bin/env python
-# -*- Mode: Python; indent-tabs-mode: nil; tab-width: 2; coding: utf-8 -*-
-#
-# This file is part of Déjà Dup.
-# For copyright information, see AUTHORS.
-#
-# Déjà Dup is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 3 of the License, or
-# (at your option) any later version.
-#
-# Déjà Dup is distributed in the hope that it will be useful, but
-# WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-# General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with Déjà Dup.  If not, see <http://www.gnu.org/licenses/>.
-
-# Test whether we correctly switch to full backup mode vs incremental
-
-import sys
-sys.path.insert(0, sys.path[0]+'/..')
-import base
-import ldtp
-
-max_allowed = 24 * 7
-scale = 12
-
-def max_inc():
-  base.setup()
-  base.backup_simple(backend='file', encrypt=False, includes=['data/simple'])
-  # Change date to maximum allowed incremental, confirm next is incremental
-  base.last_date_change('%d days ago' % (max_allowed))
-  base.backup_simple(encrypt=None)
-  assert base.last_type() == 'inc', base.list_manifests()
-
-def max_full():
-  base.setup()
-  base.backup_simple(backend='file', encrypt=False, includes=['data/simple'])
-  # Change date to maximum allowed incremental+1, confirm next is full
-  base.last_date_change('%d days ago' % (max_allowed+1))
-  base.backup_simple(encrypt=None)
-  assert base.last_type() == 'full', base.list_manifests()
-
-def half_inc():
-  base.setup()
-  base.set_settings_value("delete-after", '80')
-  base.backup_simple(backend='file', encrypt=False, includes=['data/simple'])
-  # Change date to maximum allowed incremental, confirm next is inc
-  base.last_date_change('%d days ago' % (40))
-  base.backup_simple(encrypt=None)
-  assert base.last_type() == 'inc', base.list_manifests()
-
-def half_full():
-  base.setup()
-  base.set_settings_value("delete-after", '80')
-  base.backup_simple(backend='file', encrypt=False, includes=['data/simple'])
-  # Change date to maximum allowed incremental+1, confirm next is full
-  base.last_date_change('%d days ago' % (41))
-  base.backup_simple(encrypt=None)
-  assert base.last_type() == 'full', base.list_manifests()
-
-def period_inc():
-  base.setup()
-  base.set_settings_value("periodic", 'true')
-  base.set_settings_value("periodic-period", '7')
-  base.backup_simple(backend='file', encrypt=False, includes=['data/simple'])
-  # Change date to maximum allowed incremental, confirm next is inc
-  base.last_date_change('%d days ago' % (7*scale))
-  base.backup_simple(encrypt=None)
-  assert base.last_type() == 'inc', base.list_manifests()
-
-def period_full():
-  base.setup()
-  base.set_settings_value("periodic", 'true')
-  base.set_settings_value("periodic-period", '7')
-  base.backup_simple(backend='file', encrypt=False, includes=['data/simple'])
-  # Change date to maximum allowed incremental+1, confirm next is full
-  base.last_date_change('%d days ago' % ((7*scale)+1))
-  base.backup_simple(encrypt=None)
-  assert base.last_type() == 'full', base.list_manifests()
-
-base.run(max_inc)
-base.run(max_full)
-base.run(half_inc)
-base.run(half_full) # optimism!
-base.run(period_inc)
-base.run(period_full)

=== added file 'tests/scripts/threshold-full.test'
--- tests/scripts/threshold-full.test	1970-01-01 00:00:00 +0000
+++ tests/scripts/threshold-full.test	2012-06-21 17:50:33 +0000
@@ -0,0 +1,15 @@
+# Confirm that we are doing the right thing with full backup thresholds.
+# Specifically, that when we are right over the threshold, we do a full backup.
+
+[Operation]
+Type=backup
+IsFull=true
+
+[Duplicity]
+Runs=status;dry;backup;
+
+[Duplicity status]
+#echo "INFO 3"
+#echo "chain-complete"
+#echo " full $(date --utc --date='91 days ago' +%Y%m%dT%H%M%SZ) 1 enc"
+OutputScript=true

=== added file 'tests/scripts/threshold-inc.test'
--- tests/scripts/threshold-inc.test	1970-01-01 00:00:00 +0000
+++ tests/scripts/threshold-inc.test	2012-06-21 17:50:33 +0000
@@ -0,0 +1,16 @@
+# Confirm that we are doing the right thing with full backup thresholds.
+# Specifically, that when we are right under the threshold, we still do an
+# incremental.
+
+[Operation]
+Type=backup
+IsFull=false
+
+[Duplicity]
+Runs=status;dry;backup;
+
+[Duplicity status]
+#echo "INFO 3"
+#echo "chain-complete"
+#echo " full $(date --utc --date='90 days ago' +%Y%m%dT%H%M%SZ) 1 enc"
+OutputScript=true

=== modified file 'widgets/ConfigDelete.vala'
--- widgets/ConfigDelete.vala	2011-10-27 15:38:04 +0000
+++ widgets/ConfigDelete.vala	2012-06-21 17:50:33 +0000
@@ -23,9 +23,6 @@
 
 public class ConfigDelete : ConfigChoice
 {
-  public static const int MONTHLY = 28;
-  public static const int BIMONTHLY = 28*2;
-  public static const int TRIMONTHLY = 28*3;
   public static const int SEMIANNUALLY = 365/2;
   public static const int ANNUALLY = 365;
   public static int FOREVER = int.MAX;
@@ -40,9 +37,6 @@
     Gtk.TreeIter iter;
     int i = 0;
     
-    store.insert_with_values(out iter, i++, 0, _("At least a month"), 1, MONTHLY);
-    store.insert_with_values(out iter, i++, 0, _("At least two months"), 1, BIMONTHLY);
-    store.insert_with_values(out iter, i++, 0, _("At least three months"), 1, TRIMONTHLY);
     store.insert_with_values(out iter, i++, 0, _("At least six months"), 1, SEMIANNUALLY);
     store.insert_with_values(out iter, i++, 0, _("At least a year"), 1, ANNUALLY);
     store.insert_with_values(out iter, i++, 0, _("Forever"), 1, FOREVER);

=== modified file 'widgets/ConfigLabelPolicy.vala'
--- widgets/ConfigLabelPolicy.vala	2011-10-31 13:15:46 +0000
+++ widgets/ConfigLabelPolicy.vala	2012-06-21 17:50:33 +0000
@@ -60,13 +60,7 @@
     if (delete_after <= 0)
       delete_after = ConfigDelete.FOREVER;
     
-    if (delete_after == ConfigDelete.MONTHLY)
-      policy = _("Old backups will be kept for at least a month or until the backup location is low on space.");
-    else if (delete_after == ConfigDelete.BIMONTHLY)
-      policy = _("Old backups will be kept for at least two months or until the backup location is low on space.");
-    else if (delete_after == ConfigDelete.TRIMONTHLY)
-      policy = _("Old backups will be kept for at least three months or until the backup location is low on space.");
-    else if (delete_after == ConfigDelete.SEMIANNUALLY)
+    if (delete_after == ConfigDelete.SEMIANNUALLY)
       policy = _("Old backups will be kept for at least six months or until the backup location is low on space.");
     else if (delete_after == ConfigDelete.ANNUALLY)
       policy = _("Old backups will be kept for at least a year or until the backup location is low on space.");

=== modified file 'widgets/ConfigPeriod.vala'
--- widgets/ConfigPeriod.vala	2011-10-07 14:43:17 +0000
+++ widgets/ConfigPeriod.vala	2012-06-21 17:50:33 +0000
@@ -35,8 +35,6 @@
     
     store.insert_with_values(out iter, i++, 0, _("Daily"), 1, 1);
     store.insert_with_values(out iter, i++, 0, _("Weekly"), 1, 7);
-    store.insert_with_values(out iter, i++, 0, _("Every 2 weeks"), 1, 14);
-    store.insert_with_values(out iter, i++, 0, _("Monthly"), 1, 28);
     
     store.set_sort_column_id(1, Gtk.SortType.ASCENDING);