zuul
/
zuul
Code Issues Proposed changes

Revert "Dequeue changes if they have no hope of merging." 

This reverts commit dd26978d5d.

The performance of mergability checking is poor and starves zuul from
processing other things. revert the change until we can come up with a
better performing solution to this problem.

Change-Id: I744031159572236f61b3fca45bb22492135ebcb2
Reviewed-on: https://review.openstack.org/22251
Reviewed-by: James E. Blair <corvus@inaugust.com>
Reviewed-by: Jeremy Stanley <fungi@yuggoth.org>
Approved: James E. Blair <corvus@inaugust.com>
Tested-by: James E. Blair <corvus@inaugust.com>
This commit is contained in:
Clark Boylan 2013-02-18 15:26:07 -08:00 committed by James E. Blair
parent 0197624ef7
commit e9ed907690
3 changed files with 4 additions and 145 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

102
tests/test_scheduler.py
View File

@ -1139,13 +1139,6 @@ class testScheduler(unittest.TestCase):
self.waitUntilSettled()
self.fake_jenkins.fakeRelease('.*-merge')
self.waitUntilSettled()
# B will conflict with A, but B can merge if A does not pass tests.
# Check that B has not been dequeued and reported at this point
# as it should remain in the queue.
assert B.data['status'] == 'NEW'
assert B.reported == 1
self.fake_jenkins.fakeRelease('.*-merge')
self.waitUntilSettled()
ref = jobs[-1].parameters['ZUUL_REF']
@ -1161,101 +1154,6 @@ class testScheduler(unittest.TestCase):
assert C.reported == 2
self.assertEmptyQueues()
def test_merge_conflict_dequeing(self):
"Test that unmergable changes are dequeued ASAP"
# D -> C (depends B) -> B (conflict) -> A -> M (conflict)
M = self.fake_gerrit.addFakeChange('org/project', 'master', 'M')
M.addPatchset(['conflict'])
M.setMerged()
self.fake_jenkins.hold_jobs_in_queue = True
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B')
B.addPatchset(['conflict'])
C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C')
C.addPatchset()
C.setDependsOn(B, 2)
D = self.fake_gerrit.addFakeChange('org/project', 'master', 'D')
A.addApproval('CRVW', 2)
B.addApproval('CRVW', 2)
C.addApproval('CRVW', 2)
D.addApproval('CRVW', 2)
self.fake_gerrit.addEvent(A.addApproval('APRV', 1))
self.fake_gerrit.addEvent(B.addApproval('APRV', 1))
self.fake_gerrit.addEvent(C.addApproval('APRV', 1))
self.fake_gerrit.addEvent(D.addApproval('APRV', 1))
self.waitUntilSettled()
self.fake_jenkins.fakeRelease('.*-merge')
self.waitUntilSettled()
self.fake_jenkins.fakeRelease('.*-merge')
self.waitUntilSettled()
# Conflict should be detected at this point in time resulting
# in change B being dequeued and reported.
assert B.data['status'] == 'NEW'
assert B.reported == 2
self.fake_jenkins.fakeRelease('.*-merge')
self.waitUntilSettled()
self.fake_jenkins.fakeRelease('.*-merge')
self.waitUntilSettled()
self.fake_jenkins.hold_jobs_in_queue = False
self.fake_jenkins.fakeRelease()
self.waitUntilSettled()
assert A.data['status'] == 'MERGED'
assert B.data['status'] == 'NEW'
assert C.data['status'] == 'NEW'
assert D.data['status'] == 'MERGED'
assert A.reported == 2
assert B.reported == 2
assert C.reported == 2
assert D.reported == 2
self.assertEmptyQueues()
def test_merge_conflict_at_head_dequeing(self):
"Test that unmergable change at head of queue is dequeued properly"
# C -> B -> A (conflict) -> M (conflict)
M = self.fake_gerrit.addFakeChange('org/project', 'master', 'M')
M.addPatchset(['conflict'])
M.setMerged()
self.fake_jenkins.hold_jobs_in_queue = True
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
A.addPatchset(['conflict'])
B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B')
C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C')
A.addApproval('CRVW', 2)
B.addApproval('CRVW', 2)
C.addApproval('CRVW', 2)
self.fake_gerrit.addEvent(A.addApproval('APRV', 1))
self.fake_gerrit.addEvent(B.addApproval('APRV', 1))
self.fake_gerrit.addEvent(C.addApproval('APRV', 1))
self.waitUntilSettled()
self.fake_jenkins.fakeRelease('.*-merge')
self.waitUntilSettled()
# Conflict should be detected at this point in time resulting
# in change A being dequeued and reported.
assert A.data['status'] == 'NEW'
assert A.reported == 2
self.fake_jenkins.fakeRelease('.*-merge')
self.waitUntilSettled()
self.fake_jenkins.fakeRelease('.*-merge')
self.waitUntilSettled()
self.fake_jenkins.hold_jobs_in_queue = False
self.fake_jenkins.fakeRelease()
self.waitUntilSettled()
assert A.data['status'] == 'NEW'
assert B.data['status'] == 'MERGED'
assert C.data['status'] == 'MERGED'
assert A.reported == 2
assert B.reported == 2
assert C.reported == 2
self.assertEmptyQueues()
def test_post(self):
"Test that post jobs run"
e = {

11
zuul/merger.py
View File

@ -105,9 +105,6 @@ class Repo(object):
self.remote_url))
self.repo.remotes.origin.push('%s:%s' % (local, remote))
def getHeadCommit(self):
return self.repo.head.commit.hexsha
class Merger(object):
log = logging.getLogger("zuul.Merger")
@ -192,16 +189,14 @@ class Merger(object):
# Keep track of the last commit, it's the commit that
# will be passed to jenkins because it's the commit
# for the triggering change
if target_ref:
zuul_ref = change.branch + '/' + target_ref
repo.setZuulRef(zuul_ref, 'HEAD')
commit = repo.getHeadCommit()
zuul_ref = change.branch + '/' + target_ref
commit = repo.setZuulRef(zuul_ref, 'HEAD').hexsha
except:
self.log.exception("Unable to set zuul ref %s for change %s" %
(zuul_ref, change))
return False
if self.push_refs and target_ref:
if self.push_refs:
# Push the results upstream to the zuul ref
for project, branches in projects.items():
repo = self.getRepo(project)

36
zuul/scheduler.py
View File

@ -1122,26 +1122,7 @@ class DependentPipelineManager(BasePipelineManager):
changes))
return changes
def _potentiallyMergable(self, dependent_changes, change):
merges = []
# Merge changes in order against their respective branches.
# Return the list of successful merges. Empty list indicates
# change cannot be merged given the current state of change's
# branch and the dependent changes ahead of it.
for x in xrange(len(dependent_changes)):
merges.append(self.sched.merger.mergeChanges(
dependent_changes[:x] + [change]))
merges.append(self.sched.merger.mergeChanges(
dependent_changes + [change]))
return filter(None, merges)
def _launchJobs(self, change, jobs):
# It is possible that during the processing of changes in _launchJobs
# the change passed in here was removed from the pipelines queue. If
# that is the case ignore the change and move onto the next one.
change_queue = self.pipeline.getQueue(change.project)
if change not in change_queue.queue:
return
self.log.debug("Launching jobs for change %s" % change)
ref = change.current_build_set.ref
if hasattr(change, 'refspec') and not ref:
@ -1149,27 +1130,12 @@ class DependentPipelineManager(BasePipelineManager):
ref = change.current_build_set.ref
dependent_changes = self._getDependentChanges(change)
dependent_changes.reverse()
# Check if this change can merge given the current state of the
# DependentPipeline. If it cannot merge, check if it has any chance
# of merging. If there is no chance at merging remove the change,
# otherwise keep it in hopes that it will merge.
all_changes = dependent_changes + [change]
commit = self.sched.merger.mergeChanges(all_changes, ref)
if not commit:
self.log.info("Unable to merge changes %s" % all_changes)
self.pipeline.setUnableToMerge(change)
# Check if this change can potentially merge given the changes
# from the same project ahead in the queue.
conflictable_changes = filter(
lambda x: x.project == change.project,
dependent_changes)
if not self._potentiallyMergable(conflictable_changes, change):
self.log.info("Change %s cannot be merged, removing from "
"queue." % change)
self.removeChange(change)
self.reportChange(change)
else:
self.possiblyReportChange(change)
self.possiblyReportChange(change)
return
change.current_build_set.commit = commit
#TODO: remove this line after GERRIT_CHANGES is gone