[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

[Britney] RfC/RfR: Rewrite age requirement implementation



Hi,

I have created a patch to make Britney track package ageing with higher
resolution.  The patch is attached along with the patch for the test suite.

Please review the patch(es) and let me know if you have any concerns
with it.

 * If I do not hear objections about the proposal, I intend to merge it
   in a week.

I see the following advantages of the patch:

 * With this patch ageing, the age-requirement becomes a lower bound
   as expected.
   - The original "Dates" format is lossy.  By timing your upload, you
     can change the age requirement by +/- half a day.
   - Note: This imprecision cannot be fixed retroactively, so the patch
     only fixes this for future uploads.

 * We can trivially add a pretty good estimate on when the age
   requirement will be met.
   - Notably it will be easier for people to see if they are looking at
     an outdated report.

 * We no longer have "boring" Britney runs, since ageing can be
   satisfied in any regular/cronned Britney run.
   - Out of band runs


A couple of implementation details, which can certainly be changed:

 * To offset non-determinism in "when" Britney checks the clock, the
   implementation lets packages satisfy the age requirement if they are
   within 5 minutes of the deadline.
   - We could also add an option to seed "date_now".  Might make sense
     for test runs (so you can replay an earlier run or guessimate a
     future run)

 * The new "age-information" file is stored in a "state-dir".  Don't
   care if that is changed (be it dir or/and filename).

 * The patch makes Britney prune unused entries from the
   "age-information" file (i.e. entries not in testing nor in unstable)
   - Can extract that into a separate patch.

 * I am not particular attached to using seconds - it was just easy to
   work with.


Thanks,
~Niels

From 826aadd5ff0e5ce91d8a668f4b5afb3ee1b1aea4 Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Sat, 16 Jan 2016 10:28:24 +0000
Subject: [PATCH] britney: Track aging with second resolution

Signed-off-by: Niels Thykier <niels@thykier.net>
---
 britney.conf            |  2 ++
 britney.py              | 74 +++++++++++++++++++++++++++++++++++--------------
 britney_nobreakall.conf |  2 ++
 excuse.py               | 52 ++++++++++++++++++++++++++++------
 4 files changed, 100 insertions(+), 30 deletions(-)

diff --git a/britney.conf b/britney.conf
index f2502d1..79c1820 100644
--- a/britney.conf
+++ b/britney.conf
@@ -13,6 +13,8 @@ EXCUSES_YAML_OUTPUT = /srv/release.debian.org/britney/var/data-b2/output/excuses
 UPGRADE_OUTPUT    = /srv/release.debian.org/britney/var/data-b2/output/output.txt
 HEIDI_OUTPUT      = /srv/release.debian.org/britney/var/data-b2/output/HeidiResult
 
+STATE_DIR         = /srv/release.debian.org/britney/var/data-b2/state-cache
+
 # List of release architectures
 ARCHITECTURES     = i386 amd64 arm64 armel armhf mips mipsel powerpc ppc64el s390x
 
diff --git a/britney.py b/britney.py
index 9c7580a..aad33b5 100755
--- a/britney.py
+++ b/britney.py
@@ -189,6 +189,7 @@ import optparse
 import apt_pkg
 
 from collections import defaultdict
+from datetime import datetime, timedelta, timezone
 from functools import reduce
 from itertools import chain, product
 from operator import attrgetter
@@ -235,8 +236,7 @@ class Britney(object):
         This method initializes and populates the data lists, which contain all
         the information needed by the other methods of the class.
         """
-        # britney's "day" begins at 3pm
-        self.date_now = int(((time.time() / (60*60)) - 15) / 24)
+        self.date_now = datetime.utcnow()
 
         # parse the command line arguments
         self.__parse_arguments()
@@ -325,7 +325,7 @@ class Britney(object):
         self.normalize_bugs()
 
         # read additional data
-        self.dates = self.read_dates(self.options.testing)
+        self.dates = self.read_dates(self.options.state_dir, self.options.testing)
         self.urgencies = self.read_urgencies(self.options.testing)
         self.excuses = []
 
@@ -779,7 +779,7 @@ class Britney(object):
             if maxvert is None:
                 self.bugs['testing'][pkg] = []
 
-    def read_dates(self, basedir):
+    def read_dates(self, state_dir, legacy_dir):
         """Read the upload date for the packages from the specified directory
         
         The upload dates are read from the `Dates' file within the directory
@@ -794,28 +794,55 @@ class Britney(object):
         name and the value is a tuple with two items, the version and the date.
         """
         dates = {}
-        filename = os.path.join(basedir, "Dates")
-        self.__log("Loading upload data from %s" % filename)
-        for line in open(filename, encoding='ascii'):
-            l = line.split()
-            if len(l) != 3: continue
-            try:
-                dates[l[0]] = (l[1], int(l[2]))
-            except ValueError:
-                self.__log("Dates, unable to parse \"%s\"" % line, type="E")
+        filename = os.path.join(state_dir, 'age-information')
+        legacy_filename = os.path.join(legacy_dir, "Dates")
+        if os.path.exists(filename):
+            self.__log("Loading age-information from %s" % filename)
+            with open(filename, encoding='utf-8') as fd:
+                for line in fd:
+                    l = line.split()
+                    if len(l) != 3:
+                        continue
+                    try:
+                        dates[l[0]] = (l[1], datetime.utcfromtimestamp(int(l[2])))
+                    except ValueError:
+                        self.__log("age-information, unable to parse \"%s\"" % line, type="E")
+        else:
+            self.__log("Loading legacy age-information from %s" % legacy_filename)
+            # The 15:01 time is to offset "Britney's day".  Note that even then the
+            # conversion is imperfect as the original data can silently be "shewed" by
+            # half a day in either direction
+            date_epoch = datetime(1970, 1, 1, 15, 1)
+            for line in open(legacy_filename, encoding='ascii'):
+                l = line.split()
+                if len(l) != 3:
+                    continue
+                try:
+                    dates[l[0]] = (l[1], date_epoch + timedelta(days=int(l[2])))
+                except ValueError:
+                    self.__log("Dates, unable to parse \"%s\"" % line, type="E")
         return dates
 
-    def write_dates(self, basedir, dates):
+    def write_dates(self, state_dir, legacy_dir, dates, sources):
         """Write the upload date for the packages to the specified directory
 
         For a more detailed explanation of the format, please check the method
         read_dates.
         """
-        filename = os.path.join(basedir, "Dates")
-        self.__log("Writing upload data to %s" % filename)
+        filename = os.path.join(state_dir, "age-information")
+        legacy_filename = os.path.join(legacy_dir, "Dates")
+        self.__log("Writing age-information data to %s" % filename)
+        sources_t = sources['testing']
+        sources_u = sources['unstable']
         with open(filename, 'w', encoding='utf-8') as f:
             for pkg in sorted(dates):
-                f.write("%s %s %d\n" % ((pkg,) + dates[pkg]))
+                if pkg not in sources_t and pkg not in sources_u:
+                    continue
+                version, seen_time = dates[pkg]
+                f.write("%s %s %d\n" % (pkg,version, seen_time.replace(tzinfo=timezone.utc).timestamp()))
+        if os.path.exists(legacy_filename):
+            self.__log("Removing legacy age information file %s" % legacy_filename)
+            os.remove(legacy_filename)
 
 
     def read_urgencies(self, basedir):
@@ -1365,7 +1392,10 @@ class Britney(object):
             elif not same_source(self.dates[src][0], source_u[VERSION]):
                 self.dates[src] = (source_u[VERSION], self.date_now)
 
-            days_old = self.date_now - self.dates[src][1]
+            # Add 5 minutes of fuzz in favor of the package
+            first_seen = self.dates[src][1]
+            time_delta = self.date_now - first_seen + timedelta(minutes=5)
+            days_old = time_delta.days
             min_days = self.MINDAYS[urgency]
 
             for age_days_hint in [ x for x in self.hints.search('age-days', package=src) if \
@@ -1374,8 +1404,10 @@ class Britney(object):
                     int(age_days_hint.days), age_days_hint.user))
                 min_days = int(age_days_hint.days)
 
+            earliest_migration_time = first_seen + timedelta(days=min_days)
+            excuse.set_age_info(first_seen, earliest_migration_time=earliest_migration_time)
             excuse.setdaysold(days_old, min_days)
-            if days_old < min_days:
+            if self.date_now < earliest_migration_time:
                 urgent_hints = [ x for x in self.hints.search('urgent', package=src) if \
                    same_source(source_u[VERSION], x.version) ]
                 if urgent_hints:
@@ -2629,9 +2661,9 @@ class Britney(object):
 
             # write dates
             try:
-                self.write_dates(self.options.outputdir, self.dates)
+                self.write_dates(self.options.state_dir, self.options.outputdir, self.dates, self.sources)
             except AttributeError:
-                self.write_dates(self.options.testing, self.dates)
+                self.write_dates(self.options.state_dir, self.options.testing, self.dates, self.sources)
 
             # write HeidiResult
             self.__log("Writing Heidi results to %s" % self.options.heidi_output)
diff --git a/britney_nobreakall.conf b/britney_nobreakall.conf
index 9f0c22e..44ae2f4 100644
--- a/britney_nobreakall.conf
+++ b/britney_nobreakall.conf
@@ -13,6 +13,8 @@ EXCUSES_YAML_OUTPUT = /srv/release.debian.org/britney/var/data-b2/output/excuses
 UPGRADE_OUTPUT    = /srv/release.debian.org/britney/var/data-b2/output/output.txt
 HEIDI_OUTPUT      = /srv/release.debian.org/britney/var/data-b2/output/HeidiResult
 
+STATE_DIR         = /srv/release.debian.org/britney/var/data-b2/state-cache
+
 # List of release architectures
 ARCHITECTURES     = i386 amd64 arm64 armel armhf mips mipsel powerpc ppc64el s390x
 
diff --git a/excuse.py b/excuse.py
index 8942bef..8596cbe 100644
--- a/excuse.py
+++ b/excuse.py
@@ -15,6 +15,8 @@
 # GNU General Public License for more details.
 
 import re
+from datetime import timezone
+
 
 class Excuse(object):
     """Excuse class
@@ -45,6 +47,8 @@ class Excuse(object):
         self.urgency = None
         self.daysold = None
         self.mindays = None
+        self.first_seen = None
+        self.earliest_migration_time = None
         self.section = None
         self._is_valid = False
         self._dontinvalidate = False
@@ -61,7 +65,7 @@ class Excuse(object):
         self.htmlline = []
 
     def sortkey(self):
-        if self.daysold == None:
+        if self.daysold is None:
             return (-1, self.name)
         return (self.daysold, self.name)
 
@@ -116,6 +120,14 @@ class Excuse(object):
         """Invalidate dependency"""
         if name not in self.invalid_deps: self.invalid_deps.append(name)
 
+    def set_age_info(self, first_seen, earliest_migration_time=None):
+        """Set age information about this excuse"""
+        self.first_seen = first_seen
+        if earliest_migration_time is not None:
+            self.earliest_migration_time = earliest_migration_time
+        else:
+            self.earliest_migration_time = None
+
     def setdaysold(self, daysold, mindays):
         """Set the number of days from the upload and the minimum number of days for the update"""
         self.daysold = daysold
@@ -129,6 +141,28 @@ class Excuse(object):
         """Add a note in HTML"""
         self.htmlline.append(note)
 
+    def _html_format_age(self):
+        res = ''
+
+        if self.earliest_migration_time is not None:
+            if self.earliest_migration_time <= self.first_seen:
+                res += "<li>Age requirements: satisfied (since: %s UTC)\n" % (self.earliest_migration_time)
+            else:
+                res += "<li>Age requirements: Not met (will be met: %s UTC):\n" % (self.earliest_migration_time)
+        else:
+            # No age requirements
+            res += "<li>Age information:\n"
+
+        if self.daysold is not None:
+            # Print this as well as some tool is probably depending on it
+            if self.daysold < self.mindays:
+                res = res + ("<li>Too young, only %d of %d days old\n" %
+                             (self.daysold, self.mindays))
+            else:
+                res = res + ("<li>%d days old (needed %d days)\n" %
+                             (self.daysold, self.mindays))
+        return res
+
     def html(self):
         """Render the excuse in HTML"""
         res = "<a id=\"%s\" name=\"%s\">%s</a> (%s to %s)\n<ul>\n" % \
@@ -137,13 +171,8 @@ class Excuse(object):
             res = res + "<li>Maintainer: %s\n" % (self.maint)
         if self.section and self.section.find("/") > -1:
             res = res + "<li>Section: %s\n" % (self.section)
-        if self.daysold != None:
-            if self.daysold < self.mindays:
-                res = res + ("<li>Too young, only %d of %d days old\n" %
-                (self.daysold, self.mindays))
-            else:
-                res = res + ("<li>%d days old (needed %d days)\n" %
-                (self.daysold, self.mindays))
+        if self.first_seen is not None or self.daysold is not None:
+            res += self._html_format_age()
         for x in self.htmlline:
             res = res + "<li>" + x + "\n"
         lastdep = ""
@@ -183,7 +212,7 @@ class Excuse(object):
             res.append("Maintainer: %s" % maint)
         if self.section and self.section.find("/") > -1:
             res.append("Section: %s" % (self.section))
-        if self.daysold != None:
+        if self.daysold is not None:
             if self.daysold < self.mindays:
                 res.append(("Too young, only %d of %d days old" %
                 (self.daysold, self.mindays)))
@@ -217,6 +246,11 @@ class Excuse(object):
         excusedata["new-version"] = self.ver[1]
         excusedata["age"] = self.daysold
         excusedata["age-needed"] = self.mindays
+        if self.first_seen is not None:
+            excusedata["age-information"] = {
+                "first_seen_timestamp": int(self.first_seen.timestamp()),
+                "earliest_migration_timestamp": int(self.earliest_migration_time.timestamp()),
+            }
         excusedata["new-bugs"] = sorted(self.newbugs)
         excusedata["old-bugs"] = sorted(self.oldbugs)
         if self.forced:
-- 
2.7.0.rc3

From 164d61bbdc6726e09f54649396ac2f77d6628af6 Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Sat, 16 Jan 2016 10:29:41 +0000
Subject: [PATCH] Handle aging as seconds instead of days

Signed-off-by: Niels Thykier <niels@thykier.net>
---
 perl-lib/BritneyTest.pm                            | 24 ++++++++++++++--------
 perl-lib/TestLib.pm                                |  6 +++---
 t/migration-chain/hooks/post-setup                 |  3 ++-
 .../hooks/post-setup                               | 11 +++++-----
 .../hooks/post-setup                               |  3 ++-
 .../hooks/post-setup                               | 11 +++++-----
 6 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/perl-lib/BritneyTest.pm b/perl-lib/BritneyTest.pm
index 70be37e..fbf1933 100644
--- a/perl-lib/BritneyTest.pm
+++ b/perl-lib/BritneyTest.pm
@@ -62,14 +62,18 @@ sub setup {
     my ($self) = @_;
     my $rundir = $self->rundir;
     my $testdir = $self->testdir;
-    my $outputdir;
+    my ($outputdir, $state_dir);
     mkdir $rundir, 0777 or croak "mkdir $rundir: $!";
     system ('rsync', '-a', "$testdir/", "$rundir/") == 0 or
         croak "rsync failed: " . (($?>>8) & 0xff);
     $outputdir = "$rundir/var/data/output";
+    $state_dir = "${outputdir}/state-dir";
     unless (-d $outputdir ) {
         system ('mkdir', '-p', $outputdir) == 0 or croak "mkdir -p $outputdir failed";
     }
+    if (not -d $state_dir and not mkdir($state_dir)) {
+        croak("Cannot mkdir(${state_dir}): $!");
+    }
 
     $self->_read_test_data;
 
@@ -104,13 +108,14 @@ sub setup {
             symlink "../../../hints", $hintlink or croak "symlink $hintlink -> $rundir/hints: $!";
         }
         unless ( -f "$datatdir/Urgency" or -f "$datatdir/Dates" ) {
-            $self->_generate_urgency_dates ($datatdir, "$rundir/var/data/unstable/Sources");
+            $self->_generate_urgency_dates($state_dir, $datatdir,
+                                           "$rundir/var/data/unstable/Sources");
         }
     }
 
 
-    $self->_gen_britney_conf ("$rundir/britney.conf", $self->{'testdata'},
-                              "$rundir/var/data", "$outputdir");
+    $self->_gen_britney_conf("$rundir/britney.conf", $self->{'testdata'},
+                             "$rundir/var/data", "$outputdir");
 
     $self->_hook ('post-setup');
 }
@@ -262,7 +267,7 @@ sub _read_test_data {
 }
 
 sub _generate_urgency_dates {
-    my ($self, $datatdir, $sidsources) = @_;
+    my ($self, $state_dir, $datatdir, $sidsources) = @_;
     my $urgen = "$datatdir/Test-urgency.in";
     my $dates = {};
     my $urgencies = {};
@@ -284,8 +289,7 @@ sub _generate_urgency_dates {
 
     if ( -f $urgen ) {
         # Load the urgency generation hints.
-        # Britney's day begins at 3pm.
-        my $bnow = int (((time / (60 * 60)) - 15) / 24);
+        my $bnow = int(time);
         open $fd, '<', $urgen or croak "opening $urgen: $!";
         while ( my $line = <$fd> ) {
             chomp $line;
@@ -301,7 +305,7 @@ sub _generate_urgency_dates {
             if ($date eq '*') {
                 $date = 1;
             } elsif ($date =~ m/^age=(\d+)$/o) {
-                $date = $bnow - $1;
+                $date = $bnow - ($1 * 24 * 3600);
             } elsif ($date !~ m/^\d+$/o) {
                 croak "Date for $srcver is not an int ($urgen: $.)";
             }
@@ -311,7 +315,7 @@ sub _generate_urgency_dates {
         close $fd;
     }
 
-    TestLib::gen_dates_urgencies ($datatdir, \@sources, $dates, $urgencies);
+    TestLib::gen_dates_urgencies($state_dir, $datatdir, \@sources, $dates, $urgencies);
 }
 
 sub _gen_britney_conf {
@@ -340,6 +344,8 @@ UPGRADE_OUTPUT      = $outputdir/output.txt
 HEIDI_OUTPUT        = $outputdir/HeidiResult
 EXCUSES_YAML_OUTPUT = $outputdir/excuses.yaml
 
+STATE_DIR           = $outputdir/state-dir
+
 # List of release architectures
 ARCHITECTURES     = $archs
 
diff --git a/perl-lib/TestLib.pm b/perl-lib/TestLib.pm
index 40d28df..542e786 100644
--- a/perl-lib/TestLib.pm
+++ b/perl-lib/TestLib.pm
@@ -23,11 +23,11 @@ use Carp qw(croak);
 #  - if an entry is not present, it is assumed to be "low"
 #  - if $urgencies is undefined, all urgencies are assumed to be "low"
 sub gen_dates_urgencies {
-    my ($testingdir, $spkgs, $dates, $urgencies) = @_;
+    my ($state_dir, $testingdir, $spkgs, $dates, $urgencies) = @_;
     $dates = {} unless $dates;
     $urgencies = {} unless $urgencies;
 
-    open my $df, '>', "$testingdir/Dates" or croak "opening $testingdir/Dates: $!";
+    open my $df, '>', "$state_dir/age-information" or croak "opening $state_dir/age-information: $!";
     open my $uf, '>', "$testingdir/Urgency" or croak "opening $testingdir/Urgency: $!";
 
     foreach my $s (@$spkgs) {
@@ -38,7 +38,7 @@ sub gen_dates_urgencies {
         print $uf "$pkg $ver $urgen\n";
     }
 
-    close $df or croak "closing $testingdir/Dates: $!";
+    close $df or croak "closing $state_dir/age-information: $!";
     close $uf or croak "closing $testingdir/Urgency: $!";
 }
 
diff --git a/t/migration-chain/hooks/post-setup b/t/migration-chain/hooks/post-setup
index 2a89f3d..fe597dc 100755
--- a/t/migration-chain/hooks/post-setup
+++ b/t/migration-chain/hooks/post-setup
@@ -12,7 +12,8 @@ my ($rundir) = @ARGV;
 my $pkgs = [map { ["src-$_", '1.0-1'] } 0..N-1];
 push @$pkgs, ['src-root', '1.0-1'];
 
-TestLib::gen_dates_urgencies ("$rundir/var/data/testing", $pkgs);
+TestLib::gen_dates_urgencies("$rundir/var/data/output/state-dir",
+                             "$rundir/var/data/testing", $pkgs);
 
 open my $pkgs, '>', "$rundir/var/data/unstable/Packages_i386" or
     die "open Packages_i386: $!";
diff --git a/t/tree-circle-dependencies-huge-graph-new-young/hooks/post-setup b/t/tree-circle-dependencies-huge-graph-new-young/hooks/post-setup
index e050436..d6f40ab 100755
--- a/t/tree-circle-dependencies-huge-graph-new-young/hooks/post-setup
+++ b/t/tree-circle-dependencies-huge-graph-new-young/hooks/post-setup
@@ -23,9 +23,8 @@ use TestLib;
 
 my ($rundir) = @ARGV;
 {
-    my @pkgs = ();
-    my $today = int(time / (3600 * 24));
-    my %dates = ();
+    my (@pkgs, %dates);
+    my $today = int(time);
     open my $ext, '>', "$rundir/expected" or die "opening expected: $!";
     for my $i (0..DEPTH-1) {
         for my $j (0..WIDTH-1) {
@@ -38,10 +37,12 @@ my ($rundir) = @ARGV;
 
     for my $i (0..NOT_READY-1) {
         push @pkgs, ["young-$i", "1.0-2"];
-        $dates{"young-$i/1.0-2"} = $today;
+        $dates{"young-$i/1.0-2"} = $today + 48 * 3600;
     }
 
-    TestLib::gen_dates_urgencies ("$rundir/var/data/testing", \@pkgs, \%dates);
+    TestLib::gen_dates_urgencies("$rundir/var/data/output/state-dir",
+                                 "$rundir/var/data/testing", \@pkgs,
+                                 \%dates);
 }
 
 
diff --git a/t/tree-circle-dependencies-huge-graph-no-hint/hooks/post-setup b/t/tree-circle-dependencies-huge-graph-no-hint/hooks/post-setup
index 83e3680..b4ee46c 100755
--- a/t/tree-circle-dependencies-huge-graph-no-hint/hooks/post-setup
+++ b/t/tree-circle-dependencies-huge-graph-no-hint/hooks/post-setup
@@ -31,7 +31,8 @@ my ($rundir) = @ARGV;
         }
     }
     close $ext or die "closing expected: $!";
-    TestLib::gen_dates_urgencies ("$rundir/var/data/testing", \@pkgs);
+    TestLib::gen_dates_urgencies("$rundir/var/data/output/state-dir",
+                                 "$rundir/var/data/testing", \@pkgs);
 }
 
 
diff --git a/t/tree-circle-dependencies-huge-graph-old-young/hooks/post-setup b/t/tree-circle-dependencies-huge-graph-old-young/hooks/post-setup
index 7d32576..c34e7be 100755
--- a/t/tree-circle-dependencies-huge-graph-old-young/hooks/post-setup
+++ b/t/tree-circle-dependencies-huge-graph-old-young/hooks/post-setup
@@ -23,9 +23,8 @@ use TestLib;
 
 my ($rundir) = @ARGV;
 {
-    my @pkgs = ();
-    my $today = int(time / (3600 * 24));
-    my %dates = ();
+    my (@pkgs, %dates);
+    my $today = int(time);
     open my $ext, '>', "$rundir/expected" or die "opening expected: $!";
     for my $i (0..DEPTH-1) {
         for my $j (0..WIDTH-1) {
@@ -37,13 +36,15 @@ my ($rundir) = @ARGV;
 
     for my $i (0..NOT_READY-1) {
         push @pkgs, ["young-$i", "1.0-2"];
-        $dates{"young-$i/1.0-2"} = $today;
+        $dates{"young-$i/1.0-2"} = $today + 48 * 3600;
         print $ext "young-$i 1.0-1 i386\n";
         print $ext "young-$i 1.0-1 source\n";
     }
 
     close $ext or die "closing expected: $!";
-    TestLib::gen_dates_urgencies ("$rundir/var/data/testing", \@pkgs, \%dates);
+    TestLib::gen_dates_urgencies("$rundir/var/data/output/state-dir",
+                                 "$rundir/var/data/testing", \@pkgs,
+                                 \%dates);
 }
 
 
-- 
2.7.0.rc3

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: