From fdf1f2cd06c747eb9ef4f882bb38ff1c5a591bf0 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Wed, 25 Sep 2024 16:51:43 -0700 Subject: [PATCH 1/5] Revert "Prefetch build and project on version list (#11616)" This reverts commit e5f8092025637d5a730b63b91d52c075bd4ef660. --- readthedocs/builds/models.py | 16 ---------------- readthedocs/builds/querysets.py | 26 +------------------------- readthedocs/projects/views/public.py | 6 +----- 3 files changed, 2 insertions(+), 46 deletions(-) diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 98ad201105c..25940a8bb99 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -207,9 +207,6 @@ class Meta: unique_together = [("project", "slug")] ordering = ["-verbose_name"] - # Property used for prefetching version related fields - LATEST_BUILD_CACHE = "_latest_build" - def __str__(self): return self.verbose_name @@ -294,19 +291,6 @@ def vcs_url(self): @property def last_build(self): - # TODO deprecated in favor of `latest_build`, which matches naming on - # the Project model - return self.latest_build - - @property - def latest_build(self): - # Check if there is `_latest_build` prefetch in the Queryset. - # Used for database optimization. - if hasattr(self, self.LATEST_BUILD_CACHE): - if latest_build := getattr(self, self.LATEST_BUILD_CACHE): - return latest_build[0] - return None - return self.builds.order_by("-date").first() @property diff --git a/readthedocs/builds/querysets.py b/readthedocs/builds/querysets.py index 8c304419986..006185cca76 100644 --- a/readthedocs/builds/querysets.py +++ b/readthedocs/builds/querysets.py @@ -3,7 +3,7 @@ import structlog from django.db import models -from django.db.models import OuterRef, Prefetch, Q, Subquery +from django.db.models import Q from django.utils import timezone from readthedocs.builds.constants import ( @@ -141,30 +141,6 @@ def for_reindex(self): .distinct() ) - def prefetch_subquery(self): - """ - Prefetch related objects via subquery for each version. - - .. note:: - - This should come after any filtering. - """ - from readthedocs.builds.models import Build - - # Prefetch the latest build for each project. - subquery_builds = Subquery( - Build.internal.filter(version=OuterRef("version_id")) - .order_by("-date") - .values_list("id", flat=True)[:1] - ) - prefetch_builds = Prefetch( - "builds", - Build.internal.filter(pk__in=subquery_builds), - to_attr=self.model.LATEST_BUILD_CACHE, - ) - - return self.prefetch_related(prefetch_builds) - class VersionQuerySet(SettingsOverrideObject): _default_class = VersionQuerySetBase diff --git a/readthedocs/projects/views/public.py b/readthedocs/projects/views/public.py index b8481e72954..c9fb0d3d808 100644 --- a/readthedocs/projects/views/public.py +++ b/readthedocs/projects/views/public.py @@ -123,11 +123,7 @@ def get_context_data(self, **kwargs): queryset=versions, project=project, ) - versions = ( - self.get_filtered_queryset() - .prefetch_related("project") - .prefetch_subquery() - ) + versions = self.get_filtered_queryset() context["versions"] = versions protocol = "http" From aae57181ada00a0a4975679ff10babdb290e0f3d Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Wed, 25 Sep 2024 16:55:01 -0700 Subject: [PATCH 2/5] Revert "Add support for successful build prefetch (#11613)" This reverts commit a6e1fdefcc874f5b0f69f0baa770f556eeb376b1. --- readthedocs/projects/models.py | 8 -------- readthedocs/projects/querysets.py | 24 ++++-------------------- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index f09e98673a3..b3f9a9b9aba 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -575,7 +575,6 @@ class Project(models.Model): # Property used for storing the latest build for a project when prefetching LATEST_BUILD_CACHE = "_latest_build" - LATEST_SUCCESSFUL_BUILD_CACHE = "_latest_successful_build" class Meta: ordering = ("slug",) @@ -921,13 +920,6 @@ def conf_dir(self, version=LATEST): @property def has_good_build(self): - # Check if there is `_latest_successful_build` attribute in the Queryset. - # Used for database optimization. - if hasattr(self, self.LATEST_SUCCESSFUL_BUILD_CACHE): - if build_successful := getattr(self, self.LATEST_SUCCESSFUL_BUILD_CACHE): - return build_successful[0] - return None - # Check if there is `_good_build` annotation in the Queryset. # Used for Database optimization. if hasattr(self, "_good_build"): diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index baa17db7e76..5725af100e1 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -132,33 +132,17 @@ def prefetch_latest_build(self): from readthedocs.builds.models import Build # Prefetch the latest build for each project. - subquery_build_latest = Subquery( + subquery = Subquery( Build.internal.filter(project=OuterRef("project_id")) .order_by("-date") .values_list("id", flat=True)[:1] ) - prefetch_build_latest = Prefetch( + latest_build = Prefetch( "builds", - Build.internal.filter(pk__in=subquery_build_latest), + Build.internal.filter(pk__in=subquery), to_attr=self.model.LATEST_BUILD_CACHE, ) - - # Prefetch the latest successful build for each project. - subquery_build_successful = Subquery( - Build.internal.filter(project=OuterRef("project_id")) - .order_by("-date") - .values_list("id", flat=True)[:1] - ) - prefetch_build_successful = Prefetch( - "builds", - Build.internal.filter(pk__in=subquery_build_successful), - to_attr=self.model.LATEST_SUCCESSFUL_BUILD_CACHE, - ) - - return self.prefetch_related( - prefetch_build_latest, - prefetch_build_successful, - ) + return self.prefetch_related(latest_build) # Aliases From 9bbbcdea845cb64740725ad1508c7172e4d550aa Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Wed, 25 Sep 2024 22:21:47 -0700 Subject: [PATCH 3/5] Don't use project prefetching for now --- readthedocs/projects/querysets.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index 5725af100e1..93c251ab725 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -148,7 +148,10 @@ def prefetch_latest_build(self): def dashboard(self, user): """Get the projects for this user including the latest build.""" - return self.for_user(user).prefetch_latest_build() + # Prefetching seems to cause some inconsistent performance issues, + # disabling for now. For more background, see: + # https://github.com/readthedocs/readthedocs.org/pull/11621 + return self.for_user(user) def api(self, user=None): return self.public(user) From 48bbae6327a8ab33a145ec0c0708d2e41f782406 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Wed, 25 Sep 2024 22:24:34 -0700 Subject: [PATCH 4/5] Still rename model method, without prefetch --- readthedocs/builds/models.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 25940a8bb99..118847917fe 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -291,6 +291,10 @@ def vcs_url(self): @property def last_build(self): + return self.latest_build + + @property + def latest_build(self): return self.builds.order_by("-date").first() @property From ad0263222b99f0288a9597b4990f8fd824ae5cd2 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Wed, 25 Sep 2024 22:30:10 -0700 Subject: [PATCH 5/5] Add comment back --- readthedocs/builds/models.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 118847917fe..f4514bee815 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -291,6 +291,8 @@ def vcs_url(self): @property def last_build(self): + # TODO deprecated in favor of `latest_build`, which matches naming on + # the Project model return self.latest_build @property