Merge lp:~mwhudson/launchpad/puller-scheduler-aaaaaaaaaaaargh into lp:launchpad

[Michael Hudson-Doyle]

[Michael Hudson-Doyle (mwhudson)]

Hi Tim,

Testing this damn puller stuff on staging, it's still not quite right: if we ask for work and don't get anything, we should only stop the source if no jobs are running, otherwise we get the problem we've always had with the puller, where a long running job stops all pulling for a while.

I've tested this patch on staging and it works nicely (while a mirror branch was in the process of being pulled I pushed up a hosted branch and it was pulled within seconds).

Cheers,
mwh

[Tim Penhey (thumper)]

On Fri, 07 Aug 2009 12:50:18 Michael Hudson wrote:
> I've tested this patch on staging and it works nicely (while a mirror
> branch was in the process of being pulled I pushed up a hosted branch and
> it was pulled within seconds).

We used to have one puller for mirrored branches and one for hosted. Has this
changed now?

[Michael Hudson-Doyle (mwhudson)]

Tim Penhey wrote:
> On Fri, 07 Aug 2009 12:50:18 Michael Hudson wrote:
>> I've tested this patch on staging and it works nicely (while a mirror
>> branch was in the process of being pulled I pushed up a hosted branch and
>> it was pulled within seconds).
>
> We used to have one puller for mirrored branches and one for hosted. Has this
> changed now?

Yes. We could probably separate them out again, but I don't really
think there's much point.

Cheers,
mwh

[Tim Penhey (thumper)]

On Fri, 07 Aug 2009 13:21:07 Michael Hudson wrote:
> Tim Penhey wrote:
> > On Fri, 07 Aug 2009 12:50:18 Michael Hudson wrote:
> >> I've tested this patch on staging and it works nicely (while a mirror
> >> branch was in the process of being pulled I pushed up a hosted branch
> >> and it was pulled within seconds).
> >
> > We used to have one puller for mirrored branches and one for hosted. Has
> > this changed now?
>
> Yes. We could probably separate them out again, but I don't really
> think there's much point.
>
> Cheers,
> mwh

 merge approved

review: Approve

=== modified file 'lib/canonical/twistedsupport/task.py'
--- lib/canonical/twistedsupport/task.py	2009-08-06 10:47:18 +0000
+++ lib/canonical/twistedsupport/task.py	2009-08-07 00:33:44 +0000
@@ -246,7 +246,8 @@
         find any jobs, if we actually start any jobs then the exit condition
         in _taskEnded will always be reached before this one.
         """
-        self._stop()
+        if self._worker_count == 0:
+            self._stop()
 
     def taskProductionFailed(self, reason):
         """See `ITaskConsumer`.

=== modified file 'lib/canonical/twistedsupport/tests/test_task.py'
--- lib/canonical/twistedsupport/tests/test_task.py	2009-08-06 10:47:18 +0000
+++ lib/canonical/twistedsupport/tests/test_task.py	2009-08-07 00:33:44 +0000
@@ -509,6 +509,16 @@
         consumer.noTasksFound()
         self.assertEqual([], task_log)
 
+    def test_source_not_stopped_if_no_tasks_found_and_job_running(self):
+        # If no tasks are found while a job is running, we do not stop the
+        # source.
+        consumer = self.makeConsumer()
+        log = []
+        consumer.consume(LoggingSource(log))
+        consumer.taskStarted(self._neverEndingTask)
+        consumer.noTasksFound()
+        self.assertEqual(0, log.count('stop'))
+
     def test_source_stopped_when_tasks_done(self):
         # When no more tasks are running, we stop the task source.
         consumer = self.makeConsumer()