Launchpad itself

Merge lp:~cjwatson/launchpad/process-job-source-groups-locking into lp:launchpad

  1. process-job-source-groups-locking
  2. Merge into devel
Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: no longer in the source branch.
Merged at revision: 15713
Proposed branch: lp:~cjwatson/launchpad/process-job-source-groups-locking
Merge into: lp:launchpad
Diff against target: 53 lines (+9/-9)
2 files modified
cronscripts/process-job-source-groups.py (+3/-1)
lib/lp/registry/tests/test_process_job_sources_cronjob.py (+6/-8)
To merge this branch: bzr merge lp:~cjwatson/launchpad/process-job-source-groups-locking
Reviewer Review Type Date Requested Status
Benji York (community) code Approve
Review via email: mp+117234@code.launchpad.net

Commit message

Stop process-job-source-groups taking its own lock.

Description of the change

== Summary ==

Bug 839659: we can't run process-job-source-groups on overlapping schedules for multiple groups (MAIN and FREQUENT) because it takes a lock whose name doesn't depend on the groups being run.

== Proposed fix ==

It really doesn't need its own lock, as it just runs process-job-source which locks anyway, so drop it.

== Tests ==

bin/test -vvct test_process_job_sources_cronjob

== Demo and Q/A ==

Run 'cronscripts/process-job-source-groups.py MAIN' and 'cronscripts/process-job-source-groups.py FREQUENT' in parallel on e.g. dogfood. Neither should block on the other, and both should process pending jobs.

== Lint ==

Just the usual false positive for scripts.

./cronscripts/process-job-source-groups.py
      10: '_pythonpath' imported but unused

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

Looks good.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'cronscripts/process-job-source-groups.py'
--- cronscripts/process-job-source-groups.py 2012-01-01 03:14:54 +0000
+++ cronscripts/process-job-source-groups.py 2012-07-30 10:32:06 +0000
@@ -113,4 +113,6 @@
113113
114if __name__ == '__main__':114if __name__ == '__main__':
115 script = ProcessJobSourceGroups()115 script = ProcessJobSourceGroups()
116 script.lock_and_run()116 # We do not need to take a lock here; all the interesting work is done
117 # by process-job-source.py, which takes its own per-job-source locks.
118 script.run()
117119
=== modified file 'lib/lp/registry/tests/test_process_job_sources_cronjob.py'
--- lib/lp/registry/tests/test_process_job_sources_cronjob.py 2012-06-13 01:03:45 +0000
+++ lib/lp/registry/tests/test_process_job_sources_cronjob.py 2012-07-30 10:32:06 +0000
@@ -102,14 +102,15 @@
102 self.assertIn('Group: MAIN\n I', output)102 self.assertIn('Group: MAIN\n I', output)
103103
104 def test_empty_queue(self):104 def test_empty_queue(self):
105 # The script should just create a lockfile, launch a child for105 # The script should just launch a child for each job source class,
106 # each job source class, and then exit if no jobs are in the queue.106 # and then exit if no jobs are in the queue. It should not create
107 # its own lockfile.
107 returncode, output, error = run_script(self.script, ['MAIN'])108 returncode, output, error = run_script(self.script, ['MAIN'])
108 expected = (109 expected = (
109 '.*Creating lockfile:.*launchpad-processjobsourcegroups.lock'
110 '.*Creating lockfile:.*launchpad-process-job-'110 '.*Creating lockfile:.*launchpad-process-job-'
111 'source-IMembershipNotificationJobSource.lock.*')111 'source-IMembershipNotificationJobSource.lock.*')
112 self.assertTextMatchesExpressionIgnoreWhitespace(expected, error)112 self.assertTextMatchesExpressionIgnoreWhitespace(expected, error)
113 self.assertNotIn("launchpad-processjobsourcegroups.lock", error)
113114
114 def test_processed(self):115 def test_processed(self):
115 # The script should output the number of jobs that have been116 # The script should output the number of jobs that have been
@@ -137,8 +138,7 @@
137 for source in self.getJobSources("MAIN"):138 for source in self.getJobSources("MAIN"):
138 args.extend(("--exclude", source))139 args.extend(("--exclude", source))
139 returncode, output, error = run_script(self.script, args)140 returncode, output, error = run_script(self.script, args)
140 expected = "INFO Creating lockfile: ...\n"141 self.assertEqual("", error)
141 self.assertThat(error, DocTestMatches(expected))
142142
143 def test_exclude_non_existing_group(self):143 def test_exclude_non_existing_group(self):
144 # If a job source specified by --exclude does not exist the script144 # If a job source specified by --exclude does not exist the script
@@ -148,7 +148,5 @@
148 args.extend(("--exclude", source))148 args.extend(("--exclude", source))
149 args.extend(("--exclude", "BobbyDazzler"))149 args.extend(("--exclude", "BobbyDazzler"))
150 returncode, output, error = run_script(self.script, args)150 returncode, output, error = run_script(self.script, args)
151 expected = (151 expected = "INFO 'BobbyDazzler' is not in MAIN\n"
152 "INFO Creating lockfile: ...\n"
153 "INFO 'BobbyDazzler' is not in MAIN\n")
154 self.assertThat(error, DocTestMatches(expected))152 self.assertThat(error, DocTestMatches(expected))