From e70343d849786dc3c20079e65bd4b37cd3fe2114 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 24 Apr 2015 16:22:19 -0400 Subject: [PATCH 1/2] Faster cache lookup by removing a join with the ImagePlacementTable, removing the extra loop to add the locations and filtering the images looked up by the base image --- buildman/jobutil/buildjob.py | 4 +++- data/model/legacy.py | 15 +++++++++++++++ test/test_imagetree.py | 21 +++++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/buildman/jobutil/buildjob.py b/buildman/jobutil/buildjob.py index 3c00a3bc3..91c98f6b0 100644 --- a/buildman/jobutil/buildjob.py +++ b/buildman/jobutil/buildjob.py @@ -104,7 +104,9 @@ class BuildJob(object): return None # Build an in-memory tree of the full heirarchy of images in the repository. - all_images = model.get_repository_images(repo_namespace, repo_name) + all_images = model.get_repository_images_directly(repo_build.repository, + with_ancestor=base_image) + all_tags = model.list_repository_tags(repo_namespace, repo_name) tree = ImageTree(all_images, all_tags, base_filter=base_image.id) diff --git a/data/model/legacy.py b/data/model/legacy.py index 813fe0e67..38dea3ecc 100644 --- a/data/model/legacy.py +++ b/data/model/legacy.py @@ -1751,6 +1751,21 @@ def get_matching_repository_images(namespace_name, repository_name, docker_image return _get_repository_images_base(namespace_name, repository_name, modify_query) + +def get_repository_images_directly(repository, with_ancestor=None): + query = (Image + .select(Image, ImageStorage) + .join(ImageStorage) + .where(Image.repository == repository)) + + if with_ancestor: + ancestors_string = '%s%s/' % (with_ancestor.ancestors, with_ancestor.id) + query = query.where((Image.ancestors ** (ancestors_string + '%')) | + (Image.id == with_ancestor.id)) + + return query + + def get_repository_images(namespace_name, repository_name): return _get_repository_images_base(namespace_name, repository_name, lambda q: q) diff --git a/test/test_imagetree.py b/test/test_imagetree.py index e257792c4..f1622e868 100644 --- a/test/test_imagetree.py +++ b/test/test_imagetree.py @@ -91,6 +91,27 @@ class TestImageTree(unittest.TestCase): self.assertEquals('staging', tree.tag_containing_image(result[0])) + def test_longest_path_simple_repo_direct_lookup(self): + repository = model.get_repository(NAMESPACE, SIMPLE_REPO) + all_images = list(model.get_repository_images(NAMESPACE, SIMPLE_REPO)) + all_tags = list(model.list_repository_tags(NAMESPACE, SIMPLE_REPO)) + + base_image = self._get_base_image(all_images) + tag_image = all_tags[0].image + + def checker(index, image): + return True + + filtered_images = model.get_repository_images_directly(repository, with_ancestor=base_image) + self.assertEquals(set([f.id for f in filtered_images]), set([a.id for a in all_images])) + + tree = ImageTree(filtered_images, all_tags) + + ancestors = tag_image.ancestors.split('/')[2:-1] # Skip the first image. + result = tree.find_longest_path(base_image.id, checker) + self.assertEquals(3, len(result)) + self.assertEquals('latest', tree.tag_containing_image(result[-1])) + if __name__ == '__main__': unittest.main() From 31260d50f5ad343eff417bdfe660951c80b9b015 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 24 Apr 2015 16:37:37 -0400 Subject: [PATCH 2/2] Rename the new images method to a slightly better name --- buildman/jobutil/buildjob.py | 4 ++-- data/model/legacy.py | 2 +- test/test_imagetree.py | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/buildman/jobutil/buildjob.py b/buildman/jobutil/buildjob.py index 91c98f6b0..a8a5e2b80 100644 --- a/buildman/jobutil/buildjob.py +++ b/buildman/jobutil/buildjob.py @@ -104,8 +104,8 @@ class BuildJob(object): return None # Build an in-memory tree of the full heirarchy of images in the repository. - all_images = model.get_repository_images_directly(repo_build.repository, - with_ancestor=base_image) + all_images = model.get_repository_images_without_placements(repo_build.repository, + with_ancestor=base_image) all_tags = model.list_repository_tags(repo_namespace, repo_name) tree = ImageTree(all_images, all_tags, base_filter=base_image.id) diff --git a/data/model/legacy.py b/data/model/legacy.py index 38dea3ecc..014df99df 100644 --- a/data/model/legacy.py +++ b/data/model/legacy.py @@ -1752,7 +1752,7 @@ def get_matching_repository_images(namespace_name, repository_name, docker_image return _get_repository_images_base(namespace_name, repository_name, modify_query) -def get_repository_images_directly(repository, with_ancestor=None): +def get_repository_images_without_placements(repository, with_ancestor=None): query = (Image .select(Image, ImageStorage) .join(ImageStorage) diff --git a/test/test_imagetree.py b/test/test_imagetree.py index f1622e868..9d86da7ba 100644 --- a/test/test_imagetree.py +++ b/test/test_imagetree.py @@ -102,7 +102,8 @@ class TestImageTree(unittest.TestCase): def checker(index, image): return True - filtered_images = model.get_repository_images_directly(repository, with_ancestor=base_image) + filtered_images = model.get_repository_images_without_placements(repository, + with_ancestor=base_image) self.assertEquals(set([f.id for f in filtered_images]), set([a.id for a in all_images])) tree = ImageTree(filtered_images, all_tags)