diff options
author | James E. Blair <jeblair@hp.com> | 2014-08-16 08:17:02 -0700 |
---|---|---|
committer | James E. Blair <jeblair@hp.com> | 2014-08-16 08:17:02 -0700 |
commit | c0acb55ea9ee508dc433862d782e609de19dc428 (patch) | |
tree | 9cbae13fd63e3275100ab6121de0ceb4ad506e5e | |
parent | 9a95e715eca6d483e7531c5cb47cfc44bf171ba9 (diff) | |
download | zuul-c0acb55ea9ee508dc433862d782e609de19dc428.tar.gz |
Maintain the trigger cache after reconfiguring
A previous change removed the trigger cache cleanup. When Zuul
is live-reconfigured, it creates new Project objects and
attaches them to all changes currently in pipelines. Because
the cache contained changes outside of pipelines, those changes
were not being updated with new Project objects.
This restores the maintainCache method and uses it during the
reconfigure event to clear the cache of all changes not currently
in pipelines. This corrects the error introduced by the previous
change as well as gives Zuul an opportunity to release unused
memory (as now the cache will at least be emptied on each
reconfiguration).
The project-change-merged test is updated to explicitly test this
situation.
Change-Id: I67736fca08f2e14ab733bd9f143820da19839ef9
-rw-r--r-- | tests/test_zuultrigger.py | 31 | ||||
-rw-r--r-- | zuul/scheduler.py | 7 | ||||
-rw-r--r-- | zuul/trigger/gerrit.py | 8 |
3 files changed, 38 insertions, 8 deletions
diff --git a/tests/test_zuultrigger.py b/tests/test_zuultrigger.py index 9e9bc61d2..9a90a982e 100644 --- a/tests/test_zuultrigger.py +++ b/tests/test_zuultrigger.py @@ -84,11 +84,17 @@ class TestZuulTrigger(ZuulTestCase): # A, B, C; B conflicts with A, but C does not. # When A is merged, B and C should be checked for conflicts, # and B should receive a -1. + # D and E are used to repeat the test in the second part, but + # are defined here to that they end up in the trigger cache. A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B') C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C') + D = self.fake_gerrit.addFakeChange('org/project', 'master', 'D') + E = self.fake_gerrit.addFakeChange('org/project', 'master', 'E') A.addPatchset(['conflict']) B.addPatchset(['conflict']) + D.addPatchset(['conflict2']) + E.addPatchset(['conflict2']) A.addApproval('CRVW', 2) self.fake_gerrit.addEvent(A.addApproval('APRV', 1)) self.waitUntilSettled() @@ -98,8 +104,33 @@ class TestZuulTrigger(ZuulTestCase): self.assertEqual(A.reported, 2) self.assertEqual(B.reported, 1) self.assertEqual(C.reported, 0) + self.assertEqual(D.reported, 0) + self.assertEqual(E.reported, 0) self.assertEqual(B.messages[0], "Merge Failed.\n\nThis change was unable to be automatically " "merged with the current state of the repository. Please rebase " "your change and upload a new patchset.") self.assertEqual(self.fake_gerrit.queries[0], "project:org/project status:open") + + # Reconfigure and run the test again. This is a regression + # check to make sure that we don't end up with a stale trigger + # cache that has references to projects from the old + # configuration. + self.sched.reconfigure(self.config) + + D.addApproval('CRVW', 2) + self.fake_gerrit.addEvent(D.addApproval('APRV', 1)) + self.waitUntilSettled() + + self.assertEqual(len(self.history), 2) + self.assertEqual(self.history[1].name, 'project-gate') + self.assertEqual(A.reported, 2) + self.assertEqual(B.reported, 1) + self.assertEqual(C.reported, 0) + self.assertEqual(D.reported, 2) + self.assertEqual(E.reported, 1) + self.assertEqual(E.messages[0], + "Merge Failed.\n\nThis change was unable to be automatically " + "merged with the current state of the repository. Please rebase " + "your change and upload a new patchset.") + self.assertEqual(self.fake_gerrit.queries[1], "project:org/project status:open") diff --git a/zuul/scheduler.py b/zuul/scheduler.py index a2e07cd0f..d6c51e27d 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -183,7 +183,6 @@ class Scheduler(threading.Thread): self.triggers = dict() self.reporters = dict() self.config = None - self._maintain_trigger_cache = False self.trigger_event_queue = Queue.Queue() self.result_event_queue = Queue.Queue() @@ -667,6 +666,7 @@ class Scheduler(threading.Thread): "Exception while canceling build %s " "for change %s" % (build, item.change)) self.layout = layout + self.maintainTriggerCache() for trigger in self.triggers.values(): trigger.postConfig() if statsd: @@ -784,10 +784,6 @@ class Scheduler(threading.Thread): while pipeline.manager.processQueue(): pass - if self._maintain_trigger_cache: - self.maintainTriggerCache() - self._maintain_trigger_cache = False - except Exception: self.log.exception("Exception in run handler:") # There may still be more events to process @@ -1171,7 +1167,6 @@ class BasePipelineManager(object): self.log.debug("Removing change %s from queue" % item.change) change_queue = self.pipeline.getQueue(item.change.project) change_queue.dequeueItem(item) - self.sched._maintain_trigger_cache = True def removeChange(self, change): # Remove a change from the queue, probably because it has been diff --git a/zuul/trigger/gerrit.py b/zuul/trigger/gerrit.py index 4d4deb8b6..69664886e 100644 --- a/zuul/trigger/gerrit.py +++ b/zuul/trigger/gerrit.py @@ -280,8 +280,12 @@ class Gerrit(object): # This lets the user supply a list of change objects that are # still in use. Anything in our cache that isn't in the supplied # list should be safe to remove from the cache. - # TODO(jeblair): consider removing this feature - return + remove = [] + for key, change in self._change_cache.items(): + if change not in relevant: + remove.append(key) + for key in remove: + del self._change_cache[key] def postConfig(self): pass |