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
This commit is contained in:
parent
9a95e715ec
commit
c0acb55ea9
@ -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")
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
Loading…
x
Reference in New Issue
Block a user