Colin Watson has proposed merging lp:~cjwatson/launchpad/pcj-queue-ordering
into lp:launchpad.
Commit message:
Prioritise mass-sync copies below others, and allow others to jump the queue.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1064274 in Launchpad itself: "PCJ runner has no prioritisation
facilities"
https://bugs.launchpad.net/launchpad/+bug/1064274
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/pcj-queue-ordering/+merge/130108
== Summary ==
The PackageCopyJob queue is strictly first-come-first-served. Package-copy
requests can be starved for hours by a long mass-sync queue, of the kind that
occurs when auto-syncing packages from Debian into Ubuntu at the start of an
Ubuntu release cycle.
== Proposed fix ==
We went back and forth on this quite a bit on IRC (initially trying to increase
capacity with more runners, only to find that all the obvious paths to that are
blocked for one reason or another) and agreed that a sensible approach would be
to allow single copies to pre-empt mass copies. We generally don't care very
much about the response time to mass copies, but people copying individual
packages between PPAs expect something close to an interactive response time.
== Pre-implementation notes ==
Simply ordering the queue by copy_policy is the obvious approach, but it
occurred to me that we also need to run the query more than once or else it'll
simply pick up 3000 mass-sync copies right at the start and we have the same
problem. William Grant suggested that it would probably be reasonable to just
select the first job each time round the loop, so I've done that. Technically
I suppose this has something like O(n^2 log n) complexity, but the numbers
aren't such that I would expect this to be a serious problem in practice and at
the moment the query step at the start of a PCJ run doesn't take enough time to
show up in logs; if we turn out to have a problem then we could mitigate it by
working in batches of (say) ten at a time.
== LOC Rationale ==
+40, but this should be the last substantial addition before we can get rid of
synchronous PPA copies and delayed copies.
== Tests ==
bin/test -vvct lp.soyuz.tests.test_packagecopyjob
== Demo and Q/A ==
Copy a reasonably substantial number of packages (say, 10) from Ubuntu to a
dogfood PPA, including binaries, using Archive.copyPackages. Run the PCJ
processor. While it's running, copy another package between PPAs using the web
UI or Archive.copyPackage. Verify in the logs that the single copy jumps the
queue.
--
https://code.launchpad.net/~cjwatson/launchpad/pcj-queue-ordering/+merge/130108
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~cjwatson/launchpad/pcj-queue-ordering into lp:launchpad.
=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py 2012-10-04 05:50:31 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py 2012-10-17 13:01:44 +0000
@@ -206,13 +206,26 @@
@classmethod
def iterReady(cls):
- """Iterate through all ready PackageCopyJobs."""
- jobs = IStore(PackageCopyJob).find(
- PackageCopyJob,
- And(PackageCopyJob.job_type == cls.class_job_type,
- PackageCopyJob.job == Job.id,
- Job.id.is_in(Job.ready_jobs)))
- return (cls(job) for job in jobs)
+ """Iterate through all ready PackageCopyJobs.
+
+ Even though it's slower, we repeat the query each time in order that
+ very long queues of mass syncs can be pre-empted by other jobs.
+ """
+ seen = set()
+ while True:
+ jobs = IStore(PackageCopyJob).find(
+ PackageCopyJob,
+ And(PackageCopyJob.job_type == cls.class_job_type,
+ PackageCopyJob.job == Job.id,
+ Job.id.is_in(Job.ready_jobs)))
+ jobs.order_by(PackageCopyJob.copy_policy)
+ for job in jobs:
+ if job.id not in seen:
+ seen.add(job.id)
+ yield cls(job)
+ break
+ else:
+ break
def getOopsVars(self):
"""See `IRunnableJob`."""
=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py 2012-10-04 07:02:08 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py 2012-10-17 13:01:44 +0000
@@ -478,6 +478,33 @@
emails = pop_notifications()
self.assertEqual(len(emails), 1)
+ def test_iterReady_orders_by_copy_policy(self):
+ # iterReady prioritises mass-sync copies below anything else.
+ self.makeJob(copy_policy=PackageCopyPolicy.MASS_SYNC)
+ self.makeJob()
+ self.makeJob(copy_policy=PackageCopyPolicy.MASS_SYNC)
+ ready_jobs = list(getUtility(IPlainPackageCopyJobSource).iterReady())
+ self.assertEqual([
+ PackageCopyPolicy.INSECURE,
+ PackageCopyPolicy.MASS_SYNC,
+ PackageCopyPolicy.MASS_SYNC,
+ ], [job.copy_policy for job in ready_jobs])
+
+ def test_iterReady_preempt(self):
+ # Ordinary ("insecure") copy jobs that arrive in the middle of a
+ # long mass-sync run take precedence immediately.
+ for i in range(2):
+ self.makeJob(copy_policy=PackageCopyPolicy.MASS_SYNC)
+ iterator = getUtility(IPlainPackageCopyJobSource).iterReady()
+ self.assertEqual(
+ PackageCopyPolicy.MASS_SYNC, next(iterator).copy_policy)
+ self.makeJob()
+ self.assertEqual(
+ PackageCopyPolicy.INSECURE, next(iterator).copy_policy)
+ self.assertEqual(
+ PackageCopyPolicy.MASS_SYNC, next(iterator).copy_policy)
+ self.assertRaises(StopIteration, next, iterator)
+
def test_getOopsVars(self):
distroseries = self.factory.makeDistroSeries()
archive1 = self.factory.makeArchive(distroseries.distribution)
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
