replace o(n²) comparisons with id sets and sql aggregation

Author: furlongm

hosts/tasks.py

@@ -41,10 +41,10 @@ def find_all_host_updates():
 def find_all_host_updates_homogenous():
     """ Task to find updates for all hosts where hosts are expected to be homogenous
     """
-    updated_hosts = []
+    updated_host_ids = set()
     ts = get_datetime_now()
     for host in Host.objects.all().iterator():
-        if host not in updated_hosts:
+        if host.id not in updated_host_ids:
             host.find_updates()
             host.updated_at = ts
             host.save()
@@ -56,23 +56,20 @@ def find_all_host_updates_homogenous():
             # and exclude hosts with the current timestamp
             filtered_hosts = filtered_hosts.exclude(updated_at=ts)
 
-            packages = set(host.packages.all())
-            repos = set(host.repos.all())
-            updates = host.updates.all()
+            package_ids = frozenset(host.packages.values_list('id', flat=True))
+            repo_ids = frozenset(host.repos.values_list('id', flat=True))
+            updates = list(host.updates.all())
 
-            phosts = []
             for fhost in filtered_hosts.iterator():
-                frepos = set(fhost.repos.all())
-                if repos != frepos:
+                frepo_ids = frozenset(fhost.repos.values_list('id', flat=True))
+                if repo_ids != frepo_ids:
                     continue
-                fpackages = set(fhost.packages.all())
-                if packages != fpackages:
+                fpackage_ids = frozenset(fhost.packages.values_list('id', flat=True))
+                if package_ids != fpackage_ids:
                     continue
-                phosts.append(fhost)
 
-            for phost in phosts:
-                phost.updates.set(updates)
-                phost.updated_at = ts
-                phost.save()
-                updated_hosts.append(phost)
-                info_message(text=f'Added the same updates to {phost}')
+                fhost.updates.set(updates)
+                fhost.updated_at = ts
+                fhost.save()
+                updated_host_ids.add(fhost.id)
+                info_message(text=f'Added the same updates to {fhost}')

packages/utils.py

@@ -280,21 +280,30 @@ def get_matching_packages(name, epoch, version, release, p_type, arch=None):
 def clean_packageupdates():
     """ Removes PackageUpdate objects that are no longer linked to any hosts
     """
-    package_updates = list(PackageUpdate.objects.all())
-    for update in package_updates:
-        if not update.host_set.exists():
-            text = f'Removing unused PackageUpdate {update}'
+    orphaned = PackageUpdate.objects.filter(host__isnull=True)
+    for update in orphaned:
+        text = f'Removing unused PackageUpdate {update}'
+        info_message(text=text)
+        update.delete()
+
+    duplicate_updates = PackageUpdate.objects.values(
+        'oldpackage', 'newpackage', 'security'
+    ).annotate(count=Count('id'), keep_id=Min('id')).filter(count__gt=1)
+
+    for update in duplicate_updates:
+        extra_updates = PackageUpdate.objects.filter(
+            oldpackage=update['oldpackage'],
+            newpackage=update['newpackage'],
+            security=update['security']
+        ).exclude(id=update['keep_id'])
+        keep_update = PackageUpdate.objects.get(id=update['keep_id'])
+        for extra_update in extra_updates:
+            text = f'Removing duplicate PackageUpdate: {extra_update}'
             info_message(text=text)
-            update.delete()
-        for duplicate in package_updates:
-            if update.oldpackage == duplicate.oldpackage and update.newpackage == duplicate.newpackage and \
-                    update.security == duplicate.security and update.id != duplicate.id:
-                text = f'Removing duplicate PackageUpdate: {update}'
-                info_message(text=text)
-                for host in duplicate.host_set.all():
-                    host.updates.remove(duplicate)
-                    host.updates.add(update)
-                duplicate.delete()
+            for host in extra_update.host_set.all():
+                host.updates.remove(extra_update)
+                host.updates.add(keep_update)
+            extra_update.delete()
 
 
 def clean_packages(remove_duplicates=False):
@@ -316,22 +325,23 @@ def clean_packages(remove_duplicates=False):
         packages.delete()
     if remove_duplicates:
         info_message(text='Checking for duplicate Packages...')
-        for package in Package.objects.all():
-            potential_duplicates = Package.objects.filter(
-                name=package.name,
-                arch=package.arch,
-                epoch=package.epoch,
-                version=package.version,
-                release=package.release,
-                packagetype=package.packagetype,
-                category=package.category,
-            )
-            potential_duplicates = list(potential_duplicates)
-            if len(potential_duplicates) > 1:
-                for dupe in potential_duplicates:
-                    if dupe.id != package.id:
-                        info_message(text=f'Removing duplicate Package {dupe}')
-                        dupe.delete()
+        duplicates = Package.objects.values(
+            'name', 'arch', 'epoch', 'version', 'release', 'packagetype', 'category'
+        ).annotate(count=Count('id'), keep_id=Min('id')).filter(count__gt=1)
+
+        for dup in duplicates:
+            to_delete = Package.objects.filter(
+                name=dup['name'],
+                arch=dup['arch'],
+                epoch=dup['epoch'],
+                version=dup['version'],
+                release=dup['release'],
+                packagetype=dup['packagetype'],
+                category=dup['category']
+            ).exclude(id=dup['keep_id'])
+            for package in to_delete:
+                info_message(text=f'Removing duplicate Package {package}')
+                package.delete()
 
 
 def clean_packagenames():