Search: optimize queryset when searching for subprojects (#12588)

With 3 subprojects: from 20 queries (with 4 extra queries per additional
subproject) to 11 (with 1 query per extra subproject).

In .com we have a customer with 80 subprojects, that's generating +320
queries per search :_

Main optimizations:

- get_canonical_custom_domain was changed to be a cached property, so it
can be re-used for each superproject (shared instance) on each
subproject.
- Instead of using `Project.objects.filter(superprojects__parent...)`,
we just use `project.subprojects`, so it's easier to cache the
relationship into `_superprojects`, which is then used by
Project.parent_relationship.

Author: stsewd

readthedocs/core/resolver.py

@@ -199,7 +199,7 @@ def _get_project_domain(self, project, external_version_slug=None, use_canonical
         if external_version_slug:
             domain = self._get_external_subdomain(canonical_project, external_version_slug)
         elif use_canonical_domain and self._use_cname(canonical_project):
-            domain_object = canonical_project.get_canonical_custom_domain()
+            domain_object = canonical_project.canonical_custom_domain
             if domain_object:
                 use_https = domain_object.https
                 domain = domain_object.domain

readthedocs/projects/models.py

@@ -1367,7 +1367,8 @@ def parent_relationship(self):
 
         return self.superprojects.select_related("parent").first()
 
-    def get_canonical_custom_domain(self):
+    @cached_property
+    def canonical_custom_domain(self):
         """Get the canonical custom domain or None."""
         if hasattr(self, "_canonical_domains"):
             # Cached custom domains

readthedocs/proxito/redirects.py

@@ -55,7 +55,7 @@ def canonical_redirect(request, project, redirect_type, external_version_slug=No
         to = parsed_from._replace(scheme="https").geturl()
     elif redirect_type == RedirectType.to_canonical_domain:
         # We need to change the domain and protocol.
-        canonical_domain = project.get_canonical_custom_domain()
+        canonical_domain = project.canonical_custom_domain
         protocol = "https" if canonical_domain.https else "http"
         to = parsed_from._replace(scheme=protocol, netloc=canonical_domain.domain).geturl()
     elif redirect_type == RedirectType.subproject_to_main_domain:

readthedocs/proxito/tests/test_hosting.py

@@ -408,7 +408,7 @@ def test_number_of_queries_versions_with_downloads(self):
                 active=True,
             )
 
-        with self.assertNumQueries(13):
+        with self.assertNumQueries(12):
             r = self.client.get(
                 reverse("proxito_readthedocs_docs_addons"),
                 {

readthedocs/rtd_tests/tests/test_resolver.py

@@ -539,6 +539,8 @@ def test_resolver_public_domain_overrides(self):
             canonical=True,
             https=False,
         )
+        # Purge the cached domain.
+        del self.pip.canonical_custom_domain
         url = Resolver().resolve(project=self.pip)
         self.assertEqual(url, "http://docs.foobar.com/en/latest/")
         url = Resolver().resolve(project=self.pip)

readthedocs/search/api/v2/serializers.py

@@ -12,10 +12,11 @@
 
 from rest_framework import serializers
 
+from readthedocs.builds.models import Version
+from readthedocs.core.resolver import Resolver
 from readthedocs.projects.constants import GENERIC
 from readthedocs.projects.constants import MKDOCS
 from readthedocs.projects.constants import SPHINX_HTMLDIR
-from readthedocs.projects.models import Project
 
 
 # Structures used for storing cached data of a version mostly.
@@ -98,17 +99,22 @@ def __init__(self, *args, projects=None, **kwargs):
         if projects:
             context = kwargs.setdefault("context", {})
             context["projects_data"] = {
-                project.slug: self._build_project_data(project, version.slug)
+                project.slug: self._build_project_data(project, version=version)
                 for project, version in projects
             }
         super().__init__(*args, **kwargs)
 
-    def _build_project_data(self, project, version_slug):
+    def _build_project_data(self, project, version):
         """Build a `ProjectData` object given a project and its version."""
-        url = project.get_docs_url(version_slug=version_slug)
-        project_alias = project.superprojects.values_list("alias", flat=True).first()
+        # NOTE: re-using the resolver doesn't help here,
+        # as this method is called just once per project,
+        # re-using the resolver is useful when resolving the same project multiple times.
+        url = Resolver().resolve_version(project, version)
+        project_alias = None
+        if project.parent_relationship:
+            project_alias = project.parent_relationship.alias
         version_data = VersionData(
-            slug=version_slug,
+            slug=version.slug,
             docs_url=url,
         )
         return ProjectData(
@@ -129,10 +135,15 @@ def _get_project_data(self, obj):
         if project_data:
             return project_data
 
-        project = Project.objects.filter(slug=obj.project).first()
-        if project:
+        version = (
+            Version.objects.filter(project__slug=obj.project, slug=obj.version)
+            .select_related("project")
+            .first()
+        )
+        if version:
+            project = version.project
             projects_data = self.context.setdefault("projects_data", {})
-            projects_data[obj.project] = self._build_project_data(project, obj.version)
+            projects_data[obj.project] = self._build_project_data(project, version=version)
             return projects_data[obj.project]
         return None
 

readthedocs/search/api/v3/executor.py

@@ -128,8 +128,14 @@ def _get_subprojects(self, project, version_slug=None):
         the default version will be used.
         If `version_slug` is None, we will always use the default version.
         """
-        subprojects = Project.objects.filter(superprojects__parent=project)
-        for subproject in subprojects:
+        relationships = project.subprojects.select_related("child")
+        for relationship in relationships:
+            subproject = relationship.child
+            # NOTE: Since we already have the superproject relationship,
+            # we can set it here to avoid an extra query later
+            # when using Project.parent_relationship property.
+            # The superproject instannce is also shared among all subprojects.
+            subproject._superprojects = [relationship]
             version = None
             if version_slug:
                 version = self._get_project_version(

readthedocs/search/api/v3/tests/test_api.py

@@ -362,6 +362,156 @@ class ProxiedSearchAPITest(SearchAPITest):
     def get(self, *args, **kwargs):
         return self.client.get(*args, HTTP_HOST=self.host, **kwargs)
 
+    def test_search_project_number_of_queries(self):
+        # Default version
+        with self.assertNumQueries(11):
+            resp = self.get(self.url, data={"q": "project:project test"})
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+        with self.assertNumQueries(17):
+            resp = self.get(
+                self.url, data={"q": "project:project project:another-project test"}
+            )
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+        # With explicit version
+        with self.assertNumQueries(10):
+            resp = self.get(self.url, data={"q": "project:project/latest test"})
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+        with self.assertNumQueries(16):
+            resp = self.get(
+                self.url, data={"q": "project:project/latest project:another-project/latest test"}
+            )
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+    @mock.patch("readthedocs.search.api.v3.views.tasks.record_search_query.delay", new=mock.MagicMock())
+    def test_search_project_number_of_queries_without_search_recording(self):
+        # Default version
+        with self.assertNumQueries(8):
+            resp = self.get(self.url, data={"q": "project:project test"})
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+        with self.assertNumQueries(12):
+            resp = self.get(
+                self.url, data={"q": "project:project project:another-project test"}
+            )
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+        # With explicit version
+        with self.assertNumQueries(8):
+            resp = self.get(self.url, data={"q": "project:project/latest test"})
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+        with self.assertNumQueries(12):
+            resp = self.get(
+                self.url, data={"q": "project:project/latest project:another-project/latest test"}
+            )
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+    def test_search_subprojects_number_of_queries(self):
+        subproject = get(
+            Project,
+            slug="subproject",
+            users=[self.user],
+            privacy_level=PUBLIC,
+        )
+        subproject.versions.update(built=True, active=True, privacy_level=PUBLIC)
+        self.create_index(subproject.versions.first())
+        self.project.add_subproject(subproject)
+
+        # Search on default version.
+        with self.assertNumQueries(16):
+            resp = self.get(self.url, data={"q": "subprojects:project test"})
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+        # Search on explicit version.
+        with self.assertNumQueries(14):
+            resp = self.get(self.url, data={"q": "subprojects:project/latest test"})
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+        # Add subprojects.
+        for i in range(3):
+            subproject = get(
+                Project,
+                slug=f"subproject-{i}",
+                users=[self.user],
+                privacy_level=PUBLIC,
+            )
+            subproject.versions.update(built=True, active=True, privacy_level=PUBLIC)
+            self.create_index(subproject.versions.first())
+            self.project.add_subproject(subproject)
+
+        # Search on default version.
+        with self.assertNumQueries(26):
+            resp = self.get(self.url, data={"q": "subprojects:project test"})
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+        # Search on explicit version.
+        with self.assertNumQueries(23):
+            resp = self.get(self.url, data={"q": "subprojects:project/latest test"})
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+    @mock.patch("readthedocs.search.api.v3.views.tasks.record_search_query.delay", new=mock.MagicMock())
+    def test_search_subprojects_number_of_queries_without_search_recording(self):
+        subproject = get(
+            Project,
+            slug="subproject",
+            users=[self.user],
+            privacy_level=PUBLIC,
+        )
+        subproject.versions.update(built=True, active=True, privacy_level=PUBLIC)
+        self.create_index(subproject.versions.first())
+        self.project.add_subproject(subproject)
+
+        # Search on default version.
+        with self.assertNumQueries(10):
+            resp = self.get(self.url, data={"q": "subprojects:project test"})
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+        # Search on explicit version.
+        with self.assertNumQueries(10):
+            resp = self.get(self.url, data={"q": "subprojects:project/latest test"})
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+        # Add subprojects.
+        for i in range(3):
+            subproject = get(
+                Project,
+                slug=f"subproject-{i}",
+                users=[self.user],
+                privacy_level=PUBLIC,
+            )
+            subproject.versions.update(built=True, active=True, privacy_level=PUBLIC)
+            self.create_index(subproject.versions.first())
+            self.project.add_subproject(subproject)
+
+        # Search on default version.
+        with self.assertNumQueries(13):
+            resp = self.get(self.url, data={"q": "subprojects:project test"})
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
+        # Search on explicit version.
+        with self.assertNumQueries(13):
+            resp = self.get(self.url, data={"q": "subprojects:project/latest test"})
+            assert resp.status_code == 200
+            assert resp.data["results"]
+
 
 @override_settings(ALLOW_PRIVATE_REPOS=True)
 @override_settings(RTD_ALLOW_ORGANIZATIONS=True)

readthedocs/search/tasks.py

@@ -200,7 +200,7 @@ def record_search_query(project_slug, version_slug, query, total_results, time_s
 
     version = (
         Version.objects.filter(slug=version_slug, project__slug=project_slug)
-        .prefetch_related("project")
+        .select_related("project")
         .first()
     )
     if not version: