Fix multiple jobs with provides/requires

To avoid querying the database repeatedly, we cache the results
of the query for completed jobs which provide artifacts needed
for a running job.  We only cached one such result per queue item,
so if a queue item had two jobs with differing requirements, then
we would only use the results associated with the first.

This change updates the cache to be a dictionary keyed by the
requirements used in the search.

Change-Id: Ic707013777303e696cf76120308724ec501979b2
This commit is contained in:
James E. Blair 2019-02-26 16:28:34 -08:00
parent 62e0c13190
commit 08163359f7
3 changed files with 71 additions and 14 deletions

View File

@ -49,11 +49,20 @@
name: image-user
requires: images
- job:
name: library-builder
provides: libraries
- job:
name: library-user
requires: libraries
- project:
name: org/project1
check:
jobs:
- image-builder
- library-builder
gate:
queue: integrated
jobs:
@ -64,6 +73,7 @@
check:
jobs:
- image-user
- library-user
gate:
queue: integrated
jobs:

View File

@ -4966,10 +4966,17 @@ class TestProvidesRequires(ZuulDBTestCase):
{'name': 'image', 'url': 'http://example.com/image'},
]}}
)
self.executor_server.returnData(
'library-builder', A,
{'zuul':
{'artifacts': [
{'name': 'library', 'url': 'http://example.com/library'},
]}}
)
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertEqual(len(self.builds), 1)
self.assertEqual(len(self.builds), 2)
B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
@ -4977,7 +4984,7 @@ class TestProvidesRequires(ZuulDBTestCase):
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertEqual(len(self.builds), 1)
self.assertEqual(len(self.builds), 2)
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
@ -4985,10 +4992,13 @@ class TestProvidesRequires(ZuulDBTestCase):
self.assertHistory([
dict(name='image-builder', result='SUCCESS', changes='1,1'),
dict(name='library-builder', result='SUCCESS', changes='1,1'),
dict(name='image-user', result='SUCCESS', changes='1,1 2,1'),
])
dict(name='library-user', result='SUCCESS', changes='1,1 2,1'),
], ordered=False)
image_user = self.getJobFromHistory('image-user')
self.assertEqual(
self.history[-1].parameters['zuul']['artifacts'],
image_user.parameters['zuul']['artifacts'],
[{
'project': 'org/project1',
'change': '1',
@ -4997,6 +5007,17 @@ class TestProvidesRequires(ZuulDBTestCase):
'url': 'http://example.com/image',
'name': 'image',
}])
library_user = self.getJobFromHistory('library-user')
self.assertEqual(
library_user.parameters['zuul']['artifacts'],
[{
'project': 'org/project1',
'change': '1',
'patchset': '1',
'job': 'library-builder',
'url': 'http://example.com/library',
'name': 'library',
}])
@simple_layout('layouts/provides-requires.yaml')
def test_provides_requires_check_old_success(self):
@ -5008,11 +5029,19 @@ class TestProvidesRequires(ZuulDBTestCase):
{'name': 'image', 'url': 'http://example.com/image'},
]}}
)
self.executor_server.returnData(
'library-builder', A,
{'zuul':
{'artifacts': [
{'name': 'library', 'url': 'http://example.com/library'},
]}}
)
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertHistory([
dict(name='image-builder', result='SUCCESS', changes='1,1'),
])
dict(name='library-builder', result='SUCCESS', changes='1,1'),
], ordered=False)
B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
@ -5022,10 +5051,13 @@ class TestProvidesRequires(ZuulDBTestCase):
self.assertHistory([
dict(name='image-builder', result='SUCCESS', changes='1,1'),
dict(name='library-builder', result='SUCCESS', changes='1,1'),
dict(name='image-user', result='SUCCESS', changes='1,1 2,1'),
])
dict(name='library-user', result='SUCCESS', changes='1,1 2,1'),
], ordered=False)
image_user = self.getJobFromHistory('image-user')
self.assertEqual(
self.history[-1].parameters['zuul']['artifacts'],
image_user.parameters['zuul']['artifacts'],
[{
'project': 'org/project1',
'change': '1',
@ -5034,17 +5066,30 @@ class TestProvidesRequires(ZuulDBTestCase):
'url': 'http://example.com/image',
'name': 'image',
}])
library_user = self.getJobFromHistory('library-user')
self.assertEqual(
library_user.parameters['zuul']['artifacts'],
[{
'project': 'org/project1',
'change': '1',
'patchset': '1',
'job': 'library-builder',
'url': 'http://example.com/library',
'name': 'library',
}])
@simple_layout('layouts/provides-requires.yaml')
def test_provides_requires_check_old_failure(self):
A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A')
self.executor_server.failJob('image-builder', A)
self.executor_server.failJob('library-builder', A)
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertHistory([
dict(name='image-builder', result='FAILURE', changes='1,1'),
])
dict(name='library-builder', result='FAILURE', changes='1,1'),
], ordered=False)
B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
@ -5054,7 +5099,8 @@ class TestProvidesRequires(ZuulDBTestCase):
self.assertHistory([
dict(name='image-builder', result='FAILURE', changes='1,1'),
])
dict(name='library-builder', result='FAILURE', changes='1,1'),
], ordered=False)
self.assertIn('image-user : SKIPPED', B.messages[0])
self.assertIn('not met by build', B.messages[0])

View File

@ -1987,7 +1987,7 @@ class QueueItem(object):
self.layout = None
self.project_pipeline_config = None
self.job_graph = None
self._cached_sql_results = None
self._cached_sql_results = {}
def __repr__(self):
if self.pipeline:
@ -2186,7 +2186,8 @@ class QueueItem(object):
def _getRequirementsResultFromSQL(self, requirements):
# This either returns data or raises an exception
if self._cached_sql_results is None:
requirements_tuple = tuple(sorted(requirements))
if requirements_tuple not in self._cached_sql_results:
sql_driver = self.pipeline.manager.sched.connections.drivers['sql']
conn = sql_driver.tenant_connections.get(self.pipeline.tenant.name)
if conn:
@ -2197,16 +2198,16 @@ class QueueItem(object):
change=self.change.number,
branch=self.change.branch,
patchset=self.change.patchset,
provides=list(requirements))
provides=requirements_tuple)
else:
builds = []
# Just look at the most recent buildset.
# TODO: query for a buildset instead of filtering.
builds = [b for b in builds
if b.buildset.uuid == builds[0].buildset.uuid]
self._cached_sql_results = builds
self._cached_sql_results[requirements_tuple] = builds
builds = self._cached_sql_results
builds = self._cached_sql_results[requirements_tuple]
data = []
if not builds:
return data