launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/speedup-archive-enable into lp:launchpad.

Commit message:
Make Archive.enable only update rows that don't already have the desired value.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1500973 in Launchpad itself: "lp times out trying to enable a non-virtualized PPA for test rebuilds"
  https://bugs.launchpad.net/launchpad/+bug/1500973

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/speedup-archive-enable/+merge/272828

Make Archive.enable only update rows that don't already have the desired value.

I don't know of a way to specifically test the effect of the optimisation in the test suite, but regression testing should be sufficient, and EXPLAIN (ANALYZE ON, BUFFERS ON) on dogfood suggests that this should help a good bit.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/speedup-archive-enable into lp:launchpad.

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2015-09-25 14:16:21 +0000
+++ lib/lp/soyuz/model/archive.py	2015-09-29 17:49:19 +0000
@@ -2037,8 +2037,11 @@
                 AND BuildQueue.build_farm_job =
                     BinaryPackageBuild.build_farm_job
                 -- Build is in state BuildStatus.NEEDSBUILD (0)
-                AND BinaryPackageBuild.status = %s;
-            """, params=(status.value, self.id, BuildStatus.NEEDSBUILD.value))
+                AND BinaryPackageBuild.status = %s
+                AND BuildQueue.status != %s;
+            """, params=(
+                status.value, self.id, BuildStatus.NEEDSBUILD.value,
+                status.value))
 
     def _recalculateBuildVirtualization(self):
         """Update virtualized columns for this archive."""
@@ -2054,22 +2057,31 @@
         if self.require_virtualized:
             # We can avoid the Processor join in this case.
             value = True
+            match = False
             proc_tables = []
             proc_clauses = []
             # BulkUpdate doesn't support an empty list of values.
-            store.find(BinaryPackageBuild, *bpb_clauses).set(virtualized=value)
+            bpb_rows = store.find(
+                BinaryPackageBuild,
+                BinaryPackageBuild.virtualized == match, *bpb_clauses)
+            bpb_rows.set(virtualized=value)
         else:
             value = Not(Processor.supports_nonvirtualized)
+            match = Processor.supports_nonvirtualized
             proc_tables = [Processor]
             proc_clauses = [BinaryPackageBuild.processor_id == Processor.id]
             store.execute(BulkUpdate(
                 {BinaryPackageBuild.virtualized: value},
                 table=BinaryPackageBuild, values=proc_tables,
-                where=And(*(bpb_clauses + proc_clauses))))
+                where=And(
+                    BinaryPackageBuild.virtualized == match,
+                    *(bpb_clauses + proc_clauses))))
         store.execute(BulkUpdate(
             {BuildQueue.virtualized: value},
             table=BuildQueue, values=([BinaryPackageBuild] + proc_tables),
-            where=And(*(bq_clauses + proc_clauses))))
+            where=And(
+                BuildQueue.virtualized == match,
+                *(bq_clauses + proc_clauses))))
         store.invalidate()
 
     def enable(self):