Merge ubuntu-archive-tools:optimize-phased-updater into ubuntu-archive-tools:main

[Steve Langasek]

This is a 30% reduction locally in the runtime of the phased-updater script; for a script that runs for 2 hours, that seems worth doing.

I do note one behavior difference here, which is that we are now only ever looking at the first record returned by getPublishedBinaries(), so if this ever returns a mix of records with p-u-p and without (such as with UEFI signed binaries?), we should instead loop over the records and break as soon as we find one that's non-null. That would reduce the runtime savings (especially as the majority of SRUs are going to be older kernel SRUs that have been fully phased and superseded), but might still be worthwhile.

[Steve Langasek (vorlon)]

poked around with the API and confirmed that binary payloads for signing do not show up in the output of getPublishedBinaries(); so as long as the assumption holds that p-u-p is the same for all binaries from a source, this implementation should be safe, and a lot faster.

[Steve Langasek (vorlon)]

I've done a test run of the script here btw and the html output looks equivalent to https://people.canonical.com/~ubuntu-archive/phased-updates.html

[Brian Murray (brian-murray)]

I was testing this today and ran it locally and then waited for the server to finish running its version of the report. Strangely, the one I ran locally show gce-compute-image-packages as being phased to 40% while the report on ubuntu-archive-team showed it as being phased to 30%. Additionally, checking in Launchpad right now show both binary packages as being phased at 30%. So I'm a bit concerned something is awry with this change and think it bears further investigation.

review: Needs Information

Unmerged commits

c2dd209... by Steve Langasek on 2022-07-16

Optimize the inner loop of phased-updater by not querying for extra binaries

Currently the code walks all of the binary publications for each SRU source,
but immediately afterwards there is a comment that "the p-u-p is currently
the same for all binary packages". This causes us to spend quite a lot of
time on API queries for other binaries, only to ignore the results.

Refactor the code to always only look at the first record returned from
getPublishedBinaries(), which has the same result *unless*
getPublishedBinaries() can return a mix of records with p-u-p and without;
and saves about 30% of the runtime of the script.

diff --git a/phased-updater b/phased-updater
index e457e33..e072bc8 100755
--- a/phased-updater
+++ b/phased-updater
@@ -612,10 +612,11 @@ def main():
 
     releases = []
     for series in ubuntu.series:
-        if series.active:
-            if series.status == 'Active Development':
-                continue
-            releases.append(series)
+        # XXX - trusty and xenial ESM
+        if not series.active or series.status == 'Active Development' \
+           or series.name in ('trusty', 'xenial'):
+            continue
+        releases.append(series)
     releases.reverse()
     issues = {}
     for release in releases:
@@ -624,9 +625,6 @@ def main():
         rname = release.name
         rvers = release.version
         issues[rname] = OrderedDict()
-        # XXX - trusty and xenial ESM
-        if rname in ('trusty', 'xenial'):
-            continue
         pub_sources = archive.getPublishedSources(
             created_since_date=cdate,
             order_by_date=True,
@@ -644,33 +642,31 @@ def main():
                 continue
             version = pub_source.source_package_version
             spph_link = pub_source.self_link
-            pbs = None
             try:
-                pbs = [pb for pb in pub_source.getPublishedBinaries()
-                       if pb.phased_update_percentage is not None]
+                # the p-u-p is currently the same for all binary packages
+                pb = pub_source.getPublishedBinaries()[0]
+                last_pup = pb.phased_update_percentage
             # workaround for LP: #1695113
             except lazr.restfulclient.errors.ServerError as e:
                 if 'HTTP Error 503' in str(e):
                     logging.info('Skipping 503 Error for %s' % src_pkg)
                     pass
-            if not pbs:
+            # no binary publications
+            except IndexError:
+                continue
+            if last_pup is None:
                 continue
-            if pbs:
-                # the p-u-p is currently the same for all binary packages
-                last_pup = pbs[0].phased_update_percentage
-            else:
-                last_pup = None
             max_pup = 0
             if last_pup == 0:
                 for allpb in archive.getPublishedBinaries(
                         exact_match=True, pocket='Updates',
-                        binary_name=pbs[0].binary_package_name):
+                        binary_name=pb.binary_package_name):
                     if allpb.distro_arch_series.distroseries == release:
                         if allpb.phased_update_percentage > 0:
                             max_pup = allpb.phased_update_percentage
                             break
-                if pbs[0].creator_link:
-                    creator = pbs[0].creator_link.split('/')[-1]
+                if pb.creator_link:
+                    creator = pb.creator_link.split('/')[-1]
                 else:
                     creator = '~ubuntu-archive-robot'
                 # the phasing was manually set to 0 by someone
@@ -707,19 +703,17 @@ def main():
                 if spph_link not in issues[rname]:
                     issues[rname][spph_link] = {}
                 issues[rname][spph_link]['rate'] = rate_increase
-            if pbs:
-                if spph_link not in issues[rname]:
-                    issues[rname][spph_link] = {}
-                # phasing has stopped so check what the max value was
-                if last_pup == 0:
-                    issues[rname][spph_link]['max_pup'] = max_pup
-                issues[rname][spph_link]['pup'] = last_pup
+            if spph_link not in issues[rname]:
+                issues[rname][spph_link] = {}
+            # phasing has stopped so check what the max value was
+            if last_pup == 0:
+                issues[rname][spph_link]['max_pup'] = max_pup
+            issues[rname][spph_link]['pup'] = last_pup
             suite = rname + '-updates'
             if spph_link not in issues[rname]:
                 continue
             elif ('rate' not in issues[rname][spph_link] and
-                  'buckets' not in issues[rname][spph_link] and
-                  pbs):
+                  'buckets' not in issues[rname][spph_link]):
                 # there is not an error so increment the phasing
                 current_pup = issues[rname][spph_link]['pup']
                 # if this is an update that is restarting we want to start at
@@ -734,7 +728,7 @@ def main():
                 if not options.no_act:
                     set_pup(current_pup, new_pup, release, suite, src_pkg)
                 issues[rname][spph_link]['pup'] = new_pup
-            elif pbs:
+            else:
                 # there is an error and pup is not None so stop the phasing
                 current_pup = issues[rname][spph_link]['pup']
                 if 'max_pup' in issues[rname][spph_link]: