From b56f56a486890512fd4b5773bf2b5ff299d3b4eb Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Fri, 13 Oct 2017 14:51:35 +0200 Subject: [PATCH] Use dogpile.cache to cache the default_buildroot_groups result --- module_build_service/builder/base.py | 20 +++++- .../scheduler/handlers/modules.py | 2 + module_build_service/utils.py | 28 ++++++++ tests/test_builder/test_base.py | 71 +++++++++++++++++++ 4 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 tests/test_builder/test_base.py diff --git a/module_build_service/builder/base.py b/module_build_service/builder/base.py index 88104e0e..e7bc40dc 100644 --- a/module_build_service/builder/base.py +++ b/module_build_service/builder/base.py @@ -29,6 +29,7 @@ # TODO: Ensure the RPM %dist tag is set according to the policy. import six +import dogpile.cache from abc import ABCMeta, abstractmethod from requests.exceptions import ConnectionError @@ -36,6 +37,7 @@ from module_build_service import conf, log, db from module_build_service import pdc, models import module_build_service.scm import module_build_service.utils +from module_build_service.utils import create_dogpile_key_generator_func """ @@ -61,7 +63,6 @@ Koji workflow """ - class GenericBuilder(six.with_metaclass(ABCMeta)): """ External Api for builders @@ -92,6 +93,14 @@ class GenericBuilder(six.with_metaclass(ABCMeta)): backend = "generic" backends = {} + # Create region to cache the default_buildroot_groups results. + # We are skipping the caching based on the first two arguments of + # default_buildroot_groups, because they are "self" and db.session + # instance which are different each call we call that method. + default_buildroot_groups_cache = dogpile.cache.make_region( + function_key_generator=create_dogpile_key_generator_func(2)).configure( + 'dogpile.cache.memory') + @classmethod def register_backend_class(cls, backend_class): GenericBuilder.backends[backend_class.backend] = backend_class @@ -274,8 +283,17 @@ class GenericBuilder(six.with_metaclass(ABCMeta)): """ raise NotImplementedError() + @classmethod + def clear_cache(cls, module_build): + """ + Clears the per module build default_buildroot_groups cache. + """ + cls.default_buildroot_groups_cache.delete( + "default_buildroot_groups_" + str(module_build.id)) + @classmethod @module_build_service.utils.retry(wait_on=(ConnectionError)) + @default_buildroot_groups_cache.cache_on_arguments() def default_buildroot_groups(cls, session, module): local_modules = models.ModuleBuild.local_modules(db.session) local_modules = {m.name + "-" + m.stream: m for m in local_modules} diff --git a/module_build_service/scheduler/handlers/modules.py b/module_build_service/scheduler/handlers/modules.py index f91763ff..21979097 100644 --- a/module_build_service/scheduler/handlers/modules.py +++ b/module_build_service/scheduler/handlers/modules.py @@ -96,6 +96,7 @@ def failed(config, session, msg): build.transition(config, state="failed") session.commit() build_logs.stop(build.id) + module_build_service.builder.GenericBuilder.clear_cache(build) def done(config, session, msg): @@ -123,6 +124,7 @@ def done(config, session, msg): session.commit() build_logs.stop(build.id) + module_build_service.builder.GenericBuilder.clear_cache(build) def init(config, session, msg): diff --git a/module_build_service/utils.py b/module_build_service/utils.py index 7a28b20d..89fc1be5 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -1375,3 +1375,31 @@ def get_rpm_release_from_mmd(mmd): dist_str = '.'.join([mmd.name, mmd.stream, str(mmd.version)]) dist_hash = hashlib.sha1(dist_str).hexdigest()[:8] return conf.default_dist_tag_prefix + dist_hash + +def create_dogpile_key_generator_func(skip_first_n_args=0): + """ + Creates dogpile key_generator function with additional features: + + - when models.ModuleBuild is an argument of method cached by dogpile-cache, + the ModuleBuild.id is used as a key. Therefore it is possible to cache + data per particular module build, while normally, it would be per + ModuleBuild.__str__() output, which contains also batch and other data + which changes during the build of a module. + - it is able to skip first N arguments of a cached method. This is useful + when the db.session or PDCClient instance is part of cached method call, + and the caching should work no matter what session instance is passed + to cached method argument. + """ + def key_generator(namespace, fn): + fname = fn.__name__ + def generate_key(*arg, **kwarg): + key_template = fname + "_" + for s in arg[skip_first_n_args:]: + if type(s) == models.ModuleBuild: + key_template += str(s.id) + else: + key_template += str(s) + "_" + return key_template + + return generate_key + return key_generator diff --git a/tests/test_builder/test_base.py b/tests/test_builder/test_base.py new file mode 100644 index 00000000..789bf94f --- /dev/null +++ b/tests/test_builder/test_base.py @@ -0,0 +1,71 @@ +# Copyright (c) 2017 Red Hat, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +# Written by Jan Kaluza + +import unittest +import koji + +import module_build_service.models +import module_build_service.builder + +from tests import conf, init_data, db + +from module_build_service.builder import GenericBuilder +from mock import patch + + +class TestGenericBuilder(unittest.TestCase): + + def setUp(self): + init_data() + self.module = module_build_service.models.ModuleBuild.query.filter_by(id=1).one() + + @patch('module_build_service.pdc.resolve_profiles') + def test_default_buildroot_groups_cache(self, resolve_profiles): + pdc_groups = { + "buildroot": [], + "srpm-buildroot": [] + } + resolve_profiles.return_value = pdc_groups + + expected_groups = { + "build": [], + "srpm-build": [] + } + + # Call default_buildroot_groups, the result should be cached. + ret = GenericBuilder.default_buildroot_groups(db.session, self.module) + self.assertEqual(ret, expected_groups) + resolve_profiles.assert_called_once() + resolve_profiles.reset_mock() + + # Now try calling it again to verify resolve_profiles is not called, + # because it is cached. + ret = GenericBuilder.default_buildroot_groups(db.session, self.module) + self.assertEqual(ret, expected_groups) + resolve_profiles.assert_not_called() + resolve_profiles.reset_mock() + + # And now try clearing the cache and call it again. + GenericBuilder.clear_cache(self.module) + ret = GenericBuilder.default_buildroot_groups(db.session, self.module) + self.assertEqual(ret, expected_groups) + resolve_profiles.assert_called_once()