Merge lp:~jtv/launchpad/bug-452107 into lp:launchpad

[Jeroen T. Vermeulen]

[Jeroen T. Vermeulen (jtv)]

= Bug 452107 =

We have a script that exports the latest translations for a product
series to a bzr branch. Another script allows users to import their
translations from a bzr branch. There's nothing stopping a user from
setting up both on the same product series and the same branch, creating
a full-duplex exchange of translations between the branch and Rosetta.

To avoid all sorts of race conditions, the export-to-bzr script checks
that there are no pending BranchJobs tasked with putting changes coming
in from the branch onto the translations import queue. If there are
pending jobs of this kind, it skips the export.

Alas! BranchJobs have no garbage collection or status monitoring of any
kind. So if a job dies in a way that circumvents its status change to
"failed" (e.g. because of a server reset or a script kill), the zombie
job sticks around in uncompleted state and haunts us forever.

This branch copes with that by assuming that a RosettaUploadJob (the
type of BranchJob in question) will always be completed, one way or the
other, within one day. Any jobs older than that are ignored.

I implemented this by adding a "since" parameter to the method that
finds unfinished jobs. It still defaults to an unlimited search because
the method may conceivably be used in the future to find zombie jobs
instead of the pending ones that aren't zombies.

Test: {{{./bin/test -vv -t translations.*to.branch}}}

Staging Q/A is a pain in the arse:

 * Pick a project that you have edit rights for. Ensure that it's set
   up to use Launchpad Translations.

 * Register a branch for the project on staging. Populate it by pushing
   a real branch to it. Set the branch as the development branch for a
   translatable release series of your product.

 * Set up translations export for the product series to your branch.

 * Set up translations import from bzr for the product series (of both
   template and translations).

 * Add a .pot file to your branch, commit, and push.

 * Wait for the rosetta-branches script to run on staging. (Make sure
   this has been set up; mthaddon started on it 2009-10-19 but I haven't
   checked up at the time of writing). This can take up to an hour.

 * Verify that your template appears on the project's import queue.
   Wait for it to be imported.

 * Translate something in the UI.

 * Have cronscripts/translations-export-to-branch.py run on the staging
   codehosting server. Never mind the horrible failures for the other
   branches; they're not available on this server.

 * Check that this committed your UI translation to your branch.

So far we've verified that I haven't broken exports, but not the actual
fix yet. Have a LOSA execute this SQL against the staging db to
simulate zombification of the job that just imported your template:

    UPDATE Job
    SET status = 1
    FROM Product, ProductSeries, BranchJob
    WHERE
        Job.id = BranchJob.job AND
        ProductSeries.branch = BranchJob.branch AND
        Product.id = ProductSeries.product AND
        Product.name = '<your-product-name>' AND
        ProductSeries.name = '<your-productseries-name>' AND
        BranchJob.job_type = 3;

This should update 1 record....

[Brad Crittenden (bac)]

This fix looks good Jeroen. Are there plans to do some garbage collection on those zombies?

review: Approve (code)

[Jeroen T. Vermeulen (jtv)]

> This fix looks good Jeroen. Are there plans to do some garbage collection on
> those zombies?

I believe there are; but that's purely a Code problem and we couldn't wait for it.

Separately from that, I believe Henning has requested that "running" jobs be cleared out during rollouts, since at that point we know that no jobs are really still running.

Thanks for the review!

[Jonathan Lange (jml)]

On Wed, Oct 21, 2009 at 6:10 AM, Jeroen T. Vermeulen <email address hidden> wrote:
>> This fix looks good Jeroen.  Are there plans to do some garbage collection on
>> those zombies?
>
> I believe there are; but that's purely a Code problem and we couldn't wait for it.
>

It'd still be nice to mention the bug number in your doctest, so you
know to fix it when the code bug is fixed.

jml

=== modified file 'lib/lp/code/interfaces/branchjob.py'
--- lib/lp/code/interfaces/branchjob.py	2009-08-13 07:10:18 +0000
+++ lib/lp/code/interfaces/branchjob.py	2009-10-20 06:50:31 +0000
@@ -158,11 +158,13 @@
         """Iterate through ready IRosettaUploadJobs."""
 
 
-    def findUnfinishedJobs(branch):
+    def findUnfinishedJobs(branch, since=None):
         """Find any `IRosettaUploadJob`s for `branch` that haven't run yet.
 
-        Returns ready jobs, but also ones in any other state except
-        "complete" or "failed."
+        :param branch: Branch to find unfinished jobs for.
+        :param since: Optional cutoff date: ignore jobs older than this.
+        :return: Any jobs for `branch` (and newer than `since`, if
+            given) whose status is neither "complete" nor "failed."
         """
 
 

=== modified file 'lib/lp/code/model/branchjob.py'
--- lib/lp/code/model/branchjob.py	2009-10-06 20:13:13 +0000
+++ lib/lp/code/model/branchjob.py	2009-10-20 06:50:31 +0000
@@ -529,8 +529,8 @@
         """
         store = Store.of(self.branch)
         conditions = [
-            BranchMergeProposal.target_branch==self.branch.id,
-            BranchMergeProposal.source_branch==Branch.id,
+            BranchMergeProposal.target_branch == self.branch.id,
+            BranchMergeProposal.source_branch == Branch.id,
             Branch.last_scanned_id.is_in(revision_ids)]
         if not include_superseded:
             conditions.append(
@@ -843,15 +843,18 @@
         return (RosettaUploadJob(job) for job in jobs)
 
     @staticmethod
-    def findUnfinishedJobs(branch):
+    def findUnfinishedJobs(branch, since=None):
         """See `IRosettaUploadJobSource`."""
         store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
-        jobs = store.using(BranchJob, Job).find((BranchJob), And(
+        match = And(
             Job.id == BranchJob.jobID,
             BranchJob.branch == branch,
             BranchJob.job_type == BranchJobType.ROSETTA_UPLOAD,
             Job._status != JobStatus.COMPLETED,
-            Job._status != JobStatus.FAILED))
+            Job._status != JobStatus.FAILED)
+        if since is not None:
+            match = And(match, Job.date_created > since)
+        jobs = store.using(BranchJob, Job).find((BranchJob), match)
         return jobs
 
 

=== modified file 'lib/lp/translations/doc/translations-export-to-branch.txt'
--- lib/lp/translations/doc/translations-export-to-branch.txt	2009-09-24 08:28:54 +0000
+++ lib/lp/translations/doc/translations-export-to-branch.txt	2009-10-20 06:50:31 +0000
@@ -257,3 +257,20 @@
     has pending translations changes.  Not committing.
     INFO Processed 1 item(s); 1 failure(s).
 
+There is one problem with detecting this race condition.  Jobs are never
+cleaned up.  So if the job failed for whatever reason, an unfinished job
+will stick around forever.
+
+To avoid blocking on such a job forever, the check ignores jobs that are
+old enough that they must have completed one way or another.
+
+    >>> job.date_created -= timedelta(days=7)
+    >>> transaction.commit()
+    >>> script.main()
+    INFO Exporting to translations branches.
+    INFO Exporting Gazblachko trunk series.
+    DEBUG ...
+    INFO Writing file 'po/main/nl.po':
+    INFO ...
+    INFO Unlock.
+    INFO Processed 1 item(s); 0 failure(s).

=== modified file 'lib/lp/translations/scripts/translations_to_branch.py'
--- lib/lp/translations/scripts/translations_to_branch.py	2009-10-02 09:48:30 +0000
+++ lib/lp/translations/scripts/translations_to_branch.py	2009-10-20 06:50:31 +0000
@@ -63,8 +63,12 @@
                 "Translations export for %s was just disabled." % (
                     source.title))
 
+        branch = source.translations_branch
         jobsource = getUtility(IRosettaUploadJobSource)
-        if jobsource.findUnfinishedJobs(source.translations_branch).any():
+        unfinished_jobs = jobsource.findUnfinishedJobs(
+            branch, since=datetime.now(UTC) - timedelta(days=1))
+
+        if unfinished_jobs.any():
             raise ConcurrentUpdateError(
                 "Translations branch for %s has pending translations "
                 "changes.  Not committing." % source.title)