Attachment 1446278 Details for Bug 1573892 – tested patch

[patch] tested patch

bz1573892-improvement-and-serializers.patch (text/plain), 11.12 KB, created by Pavel Moravec on 2018-05-31 12:55:39 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Pavel Moravec

Created: 2018-05-31 12:55:39 UTC

Size: 11.12 KB

patch

obsolete

>From 6a40090685f8295d42419a1641351998fa75e709 Mon Sep 17 00:00:00 2001
>From: Tatiana Tereshchenko <ttereshc@redhat.com>
>Date: Fri, 11 May 2018 13:17:07 +0200
>Subject: [PATCH 1/3] Use aggregation to identify unique errata pkglists
>
>To improve both performance and memory consumption of celery
>workers during applicability regeneration.
>Serializer for Errata now deals wit unique pkglists as well.
>
>closes #3172
>https://pulp.plan.io/issues/3172
>---
> plugins/pulp_rpm/plugins/db/models.py     | 20 ++++++++++++++++++++
> plugins/pulp_rpm/plugins/profilers/yum.py | 22 +++++++++++-----------
> plugins/pulp_rpm/plugins/serializers.py   |  7 ++++---
> 3 files changed, 35 insertions(+), 14 deletions(-)
>
>diff --git a/plugins/pulp_rpm/plugins/db/models.py b/plugins/pulp_rpm/plugins/db/models.py
>index b9e7e98d..303939d0 100644
>--- a/plugins/pulp_rpm/plugins/db/models.py
>+++ b/plugins/pulp_rpm/plugins/db/models.py
>@@ -1163,6 +1163,26 @@ def rpm_search_dicts(self):
>                     ret.append(unit_key)
>         return ret
> 
>+    @classmethod
>+    def get_unique_pkglists(cls, errata_id):
>+        """
>+        Generate a list of unique pkglists for a specified erratum.
>+
>+        Those pkglists contain only information about packages.
>+
>+        :param errata_id: The erratum to generate a unique set of pkglists for
>+        :type  errata_id: str
>+        :return: unique pkglists for a specified erratum
>+        :rtype: list of dicts
>+        """
>+        match_stage = {'$match': {'errata_id': errata_id}}
>+        group_stage = {'$group': {'_id': '$errata_id',
>+                                  'pkglists': {'$addToSet': '$collections.packages'}}}
>+        pkglists = ErratumPkglist.objects.aggregate(match_stage, group_stage,
>+                                                    allowDiskUse=True,
>+                                                    batchSize=5).next()['pkglists']
>+        return pkglists
>+
>     def create_legacy_metadata_dict(self):
>         """
>         Generate metadata dict and add erratum pkglist to it since it is stored
>diff --git a/plugins/pulp_rpm/plugins/profilers/yum.py b/plugins/pulp_rpm/plugins/profilers/yum.py
>index 033849cc..fa53d6c5 100644
>--- a/plugins/pulp_rpm/plugins/profilers/yum.py
>+++ b/plugins/pulp_rpm/plugins/profilers/yum.py
>@@ -5,9 +5,9 @@
> from pulp.server.db.model.criteria import UnitAssociationCriteria
> 
> from pulp_rpm.common.ids import TYPE_ID_ERRATA, TYPE_ID_RPM
>+from pulp_rpm.plugins.db import models
> from pulp_rpm.yum_plugin import util
> 
>-
> _logger = util.getLogger(__name__)
> 
> NVREA_KEYS = ['name', 'version', 'release', 'epoch', 'arch']
>@@ -309,24 +309,24 @@ def _is_errata_applicable(errata, profile_lookup_table, available_rpm_nevras):
>         :param profile_lookup_table: lookup table of a unit profile keyed by "name arch"
>         :type profile_lookup_table: dict
> 
>+        :param available_rpm_nevras: NEVRA of packages available in a repo
>+        :type available_rpm_nevras: list of tuples
>+
>         :return: true if applicable, false otherwise
>         :rtype: boolean
>         """
>         # Get rpms from errata
>-        errata_rpms = YumProfiler._get_rpms_from_errata(errata)
>+        pkglists = models.Errata.get_unique_pkglists(errata.unit_key.get('errata_id'))
> 
>         # RHBZ #1171280: ensure we are only checking applicability against RPMs
>         # we have access to in the repo. This is to prevent a RHEL6 machine
>         # from finding RHEL7 packages, for example.
>-        available_errata_rpms = []
>-        for errata_rpm in errata_rpms:
>-            if YumProfiler._create_nevra(errata_rpm) in available_rpm_nevras:
>-                available_errata_rpms.append(errata_rpm)
>-
>-        # Check if any rpm from errata is applicable to the consumer
>-        for errata_rpm in available_errata_rpms:
>-            if YumProfiler._is_rpm_applicable(errata_rpm, profile_lookup_table):
>-                return True
>+        for pkglist in pkglists:
>+            for collection in pkglist:
>+                for errata_rpm in collection:
>+                    if YumProfiler._create_nevra(errata_rpm) in available_rpm_nevras and \
>+                       YumProfiler._is_rpm_applicable(errata_rpm, profile_lookup_table):
>+                        return True
> 
>         # Return false if none of the errata rpms are applicable
>         return False
>diff --git a/plugins/pulp_rpm/plugins/serializers.py b/plugins/pulp_rpm/plugins/serializers.py
>index 8efef802..a8754d91 100644
>--- a/plugins/pulp_rpm/plugins/serializers.py
>+++ b/plugins/pulp_rpm/plugins/serializers.py
>@@ -54,14 +54,15 @@ def serialize(self, unit):
>         Convert a single unit to it's dictionary form.
> 
>         Add to errratum unit its pkglist.
>+        Duplicated pkglists are eliminated.
> 
>         :param unit: The object to be converted
>         :type unit: object
>         """
>-        from pulp_rpm.plugins.db.models import ErratumPkglist
>+        from pulp_rpm.plugins.db import models
> 
>-        pkglists = ErratumPkglist.objects(errata_id=unit.get('errata_id'))
>-        unit['pkglist'] = [coll for pkglist in pkglists for coll in pkglist['collections']]
>+        pkglists = models.Errata.get_unique_pkglists(unit.get('errata_id'))
>+        unit['pkglist'] = [coll for pkglist in pkglists for coll in pkglist]
>         return super(Errata, self).serialize(unit)
> 
> 
>
>From 8c852a12f24205f727d31854d36c0fea29c3ed73 Mon Sep 17 00:00:00 2001
>From: Tatiana Tereshchenko <ttereshc@redhat.com>
>Date: Tue, 15 May 2018 20:58:20 +0200
>Subject: [PATCH 2/3] Use aggregation for recursive copy of errata + API
> response fix
>
>---
> plugins/pulp_rpm/plugins/db/models.py   |  8 ++++----
> plugins/pulp_rpm/plugins/serializers.py | 13 ++++++++++++-
> 2 files changed, 16 insertions(+), 5 deletions(-)
>
>diff --git a/plugins/pulp_rpm/plugins/db/models.py b/plugins/pulp_rpm/plugins/db/models.py
>index 303939d0..4d49ff2c 100644
>--- a/plugins/pulp_rpm/plugins/db/models.py
>+++ b/plugins/pulp_rpm/plugins/db/models.py
>@@ -1137,10 +1137,10 @@ class Errata(UnitMixin, ContentUnit):
>     @property
>     def rpm_search_dicts(self):
>         ret = []
>-        pkglists = ErratumPkglist.objects(errata_id=self.errata_id)
>+        pkglists = Errata.get_unique_pkglists(errata_id=self.errata_id)
>         for pkglist in pkglists:
>-            for collection in pkglist['collections']:
>-                for package in collection.get('packages', []):
>+            for collection in pkglist:
>+                for package in collection:
>                     if len(package.get('sum') or []) == 2:
>                         checksum = package['sum'][1]
>                         checksumtype = server_util.sanitize_checksum_type(package['sum'][0])
>@@ -1173,7 +1173,7 @@ def get_unique_pkglists(cls, errata_id):
>         :param errata_id: The erratum to generate a unique set of pkglists for
>         :type  errata_id: str
>         :return: unique pkglists for a specified erratum
>-        :rtype: list of dicts
>+        :rtype: list of lists of dicts
>         """
>         match_stage = {'$match': {'errata_id': errata_id}}
>         group_stage = {'$group': {'_id': '$errata_id',
>diff --git a/plugins/pulp_rpm/plugins/serializers.py b/plugins/pulp_rpm/plugins/serializers.py
>index a8754d91..6330e23e 100644
>--- a/plugins/pulp_rpm/plugins/serializers.py
>+++ b/plugins/pulp_rpm/plugins/serializers.py
>@@ -62,7 +62,18 @@ def serialize(self, unit):
>         from pulp_rpm.plugins.db import models
> 
>         pkglists = models.Errata.get_unique_pkglists(unit.get('errata_id'))
>-        unit['pkglist'] = [coll for pkglist in pkglists for coll in pkglist]
>+        unit['pkglist'] = []
>+        coll_num = 0
>+        for pkglist in pkglists:
>+            for coll in pkglist:
>+                # To preserve the original format of a pkglist the 'short' and 'name'
>+                # keys are added. 'short' can be an empty string, collection 'name'
>+                # should be unique within an erratum.
>+                unit['pkglist'].append({'packages': coll,
>+                                        'short': '',
>+                                        'name': 'collection-%s' % coll_num})
>+                coll_num += 1
>+
>         return super(Errata, self).serialize(unit)
> 
> 
>From a679f26eb7d7fe2ec006bf4e6db1d0841715a107 Mon Sep 17 00:00:00 2001
>From: Tatiana Tereshchenko <ttereshc@redhat.com>
>Date: Sat, 26 May 2018 15:57:37 +0200
>Subject: [PATCH] Respect search criteria in the erratum serializer
>
>Serializer should not add a pkglist to an erratum if it was excluded from
>fields in search criteria.
>Serializer handles the case when errata_id is absent in fields in search
>criteria while pkglist is not.
>
>re #3172
>https://pulp.plan.io/issues/3172
>---
> plugins/pulp_rpm/plugins/serializers.py | 36 +++++++++++++++++++++------------
> 1 file changed, 23 insertions(+), 13 deletions(-)
>
>diff --git a/plugins/pulp_rpm/plugins/serializers.py b/plugins/pulp_rpm/plugins/serializers.py
>index 6330e23e..e75fd7c0 100644
>--- a/plugins/pulp_rpm/plugins/serializers.py
>+++ b/plugins/pulp_rpm/plugins/serializers.py
>@@ -53,7 +53,7 @@ def serialize(self, unit):
>         """
>         Convert a single unit to it's dictionary form.
> 
>-        Add to errratum unit its pkglist.
>+        Add to errratum unit its pkglist if needed.
>         Duplicated pkglists are eliminated.
> 
>         :param unit: The object to be converted
>@@ -61,18 +61,28 @@ def serialize(self, unit):
>         """
>         from pulp_rpm.plugins.db import models
> 
>-        pkglists = models.Errata.get_unique_pkglists(unit.get('errata_id'))
>-        unit['pkglist'] = []
>-        coll_num = 0
>-        for pkglist in pkglists:
>-            for coll in pkglist:
>-                # To preserve the original format of a pkglist the 'short' and 'name'
>-                # keys are added. 'short' can be an empty string, collection 'name'
>-                # should be unique within an erratum.
>-                unit['pkglist'].append({'packages': coll,
>-                                        'short': '',
>-                                        'name': 'collection-%s' % coll_num})
>-                coll_num += 1
>+        # If pkglist field is absent, it's on purpose, e.g. not specified in the fields during
>+        # search. So it should not be added during serialization.
>+        # If pkglist field is present, it's always emtpy => it should be filled in.
>+        if 'pkglist' in unit:
>+            errata_id = unit.get('errata_id')
>+
>+            # If fields in search criteria don't include errata_id
>+            if errata_id is None:
>+                erratum_obj = models.Errata.objects.only('errata_id').get(id=unit.get('_id'))
>+                errata_id = erratum_obj.errata_id
>+
>+            pkglists = models.Errata.get_unique_pkglists(errata_id)
>+            coll_num = 0
>+            for pkglist in pkglists:
>+                for coll in pkglist:
>+                    # To preserve the original format of a pkglist the 'short' and 'name'
>+                    # keys are added. 'short' can be an empty string, collection 'name'
>+                    # should be unique within an erratum.
>+                    unit['pkglist'].append({'packages': coll,
>+                                            'short': '',
>+                                            'name': 'collection-%s' % coll_num})
>+                    coll_num += 1
> 
>         return super(Errata, self).serialize(unit)
>
>

Actions: View | Diff

Attachments on bug 1573892: 1446278 | 1475668