Repository: pulp/pulp_rpm
Issue category: Performance Issue

Issue:
Switch to values() instead of select_related
**Version**
all
**Describe the bug**
Some queries use select_related, if we do not need the model instance, we could just use values() that will produce queryset dict results instead.

**To Reproduce**
Steps to reproduce the behavior:

**Expected behavior**
A clear and concise description of what you expected to happen.

**Additional context**
Add any other context about the problem here.  Please provide links to any previous discussions via Discourse or Bugzilla.

During publish metadata generation load just one Artifact checksum type
**Version**
all

**Describe the bug**
There is an overhead which consists in loading all artifact checksum types during publish repodata generation. It's enough to load just one, the one specified in package_checksum_type

**To Reproduce**
Steps to reproduce the behavior:

**Expected behavior**
A clear and concise description of what you expected to happen.

**Additional context**
Add any other context about the problem here.  Please provide links to any previous discussions via Discourse or Bugzilla.



Hints:

Patch:
diff --git a/CHANGES/3225.misc b/CHANGES/3225.misc
new file mode 100644
index 000000000..e1a600d91
--- /dev/null
+++ b/CHANGES/3225.misc
@@ -0,0 +1,1 @@
+Load one Artifact checksum type taken package_checksum_type during publish metadata generation.
diff --git a/CHANGES/3226.misc b/CHANGES/3226.misc
new file mode 100644
index 000000000..c43519e51
--- /dev/null
+++ b/CHANGES/3226.misc
@@ -0,0 +1,1 @@
+ContentArtifact query performance impovement. Use values() instead of select_related().
diff --git a/pulp_rpm/app/tasks/publishing.py b/pulp_rpm/app/tasks/publishing.py
index 862d95a6c..34c1c00d2 100644
--- a/pulp_rpm/app/tasks/publishing.py
+++ b/pulp_rpm/app/tasks/publishing.py
@@ -123,10 +123,8 @@ def publish_artifacts(self, content, prefix=""):
         published_artifacts = []
 
         # Special case for Packages
-        contentartifact_qs = (
-            ContentArtifact.objects.filter(content__in=content)
-            .filter(content__pulp_type=Package.get_pulp_type())
-            .select_related("content__rpm_package__time_build")
+        contentartifact_qs = ContentArtifact.objects.filter(content__in=content).filter(
+            content__pulp_type=Package.get_pulp_type()
         )
 
         rel_path_mapping = defaultdict(list)
@@ -470,8 +468,6 @@ def generate_repo_metadata(
         fil_db = cr.FilelistsSqlite(fil_db_path)
         oth_db = cr.OtherSqlite(oth_db_path)
 
-    packages = Package.objects.filter(pk__in=content)
-
     # We want to support publishing with a different checksum type than the one built-in to the
     # package itself, so we need to get the correct checksums somehow if there is an override.
     # We must also take into consideration that if the package has not been downloaded the only
@@ -482,45 +478,47 @@ def generate_repo_metadata(
     # simple foreign keys and avoid messing with the many-to-many relationship, which doesn't
     # work with select_related() and performs poorly with prefetch_related(). This is fine
     # because we know that Packages should only ever have one artifact per content.
-    contentartifact_qs = (
-        ContentArtifact.objects.filter(content__in=packages.only("pk"))
-        .select_related(
-            # content__rpm_package is a bit of a hack, exploiting the way django sets up model
-            # inheritance, but it works and is unlikely to break. All content artifacts being
-            # accessed here have an associated Package since they originally came from the
-            # Package queryset.
-            "artifact",
-            "content__rpm_package",
-        )
-        .only("artifact", "content__rpm_package__checksum_type", "content__rpm_package__pkgId")
-    )
+    fields = [
+        "content_id",
+        "content__rpm_package__checksum_type",
+        "content__rpm_package__pkgId",
+    ]
+    artifact_checksum = None
+    if package_checksum_type:
+        package_checksum_type = package_checksum_type.lower()
+        artifact_checksum = f"artifact__{package_checksum_type}"
+        fields.append(artifact_checksum)
+
+    contentartifact_qs = ContentArtifact.objects.filter(
+        content__in=content, content__pulp_type=Package.get_pulp_type()
+    ).values(*fields)
 
     pkg_to_hash = {}
     for ca in contentartifact_qs.iterator():
         if package_checksum_type:
-            package_checksum_type = package_checksum_type.lower()
-            pkgid = getattr(ca.artifact, package_checksum_type, None)
+            pkgid = ca.get(artifact_checksum, None)
 
         if not package_checksum_type or not pkgid:
-            if ca.content.rpm_package.checksum_type not in settings.ALLOWED_CONTENT_CHECKSUMS:
+            if ca["content__rpm_package__checksum_type"] not in settings.ALLOWED_CONTENT_CHECKSUMS:
                 raise ValueError(
-                    "Package {} as content unit {} contains forbidden checksum type '{}', "
-                    "thus can't be published. {}".format(
-                        ca.content.rpm_package.nevra,
-                        ca.content.pk,
-                        ca.content.rpm_package.checksum_type,
+                    "Package with pkgId {} as content unit {} contains forbidden checksum type "
+                    "'{}', thus can't be published. {}".format(
+                        ca["content__rpm_package__pkgId"],
+                        ca["content_id"],
+                        ca["content__rpm_package__checksum_type"],
                         ALLOWED_CHECKSUM_ERROR_MSG,
                     )
                 )
-            package_checksum_type = ca.content.rpm_package.checksum_type
-            pkgid = ca.content.rpm_package.pkgId
+            package_checksum_type = ca["content__rpm_package__checksum_type"]
+            pkgid = ca["content__rpm_package__pkgId"]
 
-        pkg_to_hash[ca.content_id] = (package_checksum_type, pkgid)
+        pkg_to_hash[ca["content_id"]] = (package_checksum_type, pkgid)
 
     # TODO: this is meant to be a !! *temporary* !! fix for
     # https://github.com/pulp/pulp_rpm/issues/2407
     pkg_pks_to_ignore = set()
     latest_build_time_by_nevra = defaultdict(list)
+    packages = Package.objects.filter(pk__in=content)
     for pkg in packages.only(
         "pk", "name", "epoch", "version", "release", "arch", "time_build"
     ).iterator():
diff --git a/pulp_rpm/app/tasks/synchronizing.py b/pulp_rpm/app/tasks/synchronizing.py
index 2b170add9..aeebe24b6 100644
--- a/pulp_rpm/app/tasks/synchronizing.py
+++ b/pulp_rpm/app/tasks/synchronizing.py
@@ -174,19 +174,15 @@ def add_metadata_to_publication(publication, version, prefix=""):
     published_artifacts = []
 
     # Handle packages
-    pkg_data = (
-        ContentArtifact.objects.filter(
-            content__in=version.content, content__pulp_type=Package.get_pulp_type()
-        )
-        .select_related("content__rpm_package")
-        .only("pk", "artifact", "content", "content__rpm_package__pkgId")
-    )
+    pkg_data = ContentArtifact.objects.filter(
+        content__in=version.content, content__pulp_type=Package.get_pulp_type()
+    ).values("pk", "content__rpm_package__pkgId")
     for ca in pkg_data.iterator():
         for relative_path in pkgid_to_location_href[str(version.repository.pk)][
-            ca.content.rpm_package.pkgId
+            ca["content__rpm_package__pkgId"]
         ]:
             pa = PublishedArtifact(
-                content_artifact=ca,
+                content_artifact_id=ca["pk"],
                 relative_path=os.path.join(prefix, relative_path),
                 publication=publication,
             )


Test patch: