From dc08e987b471502c7c73bd437ef592b33086131e Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 8 Jun 2023 23:57:57 +0200 Subject: [PATCH 1/5] Deprecation: improve Celery task db query This commit excludes the users that already have an unread notification with this message. This is only to avoid performing db queries for users where our notification backend won't add a new notification anyways. This will improve the performance of this task making it to run faster (hopefully). --- readthedocs/projects/tasks/utils.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/readthedocs/projects/tasks/utils.py b/readthedocs/projects/tasks/utils.py index d802883cfbb..8b9a080b9fe 100644 --- a/readthedocs/projects/tasks/utils.py +++ b/readthedocs/projects/tasks/utils.py @@ -271,6 +271,14 @@ def deprecated_config_file_used_notification(): queryset = User.objects.filter(username__in=users, profile__banned=False).order_by( "id" ) + + # Exclude users that already have an unread notification. + # Our notifications backend already performs de-duplication of notifications automatically. + # However, this is only to speed up things here since this loop takes the most time. + queryset = queryset.exclude( + message__message__startswith="Your project(s)", message__read=False + ) + n_users = queryset.count() for i, user in enumerate(queryset.iterator()): if i % 500 == 0: From a914e7c24302e291578ea92b3af0a9ec11e7b980 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Fri, 9 Jun 2023 01:20:00 +0200 Subject: [PATCH 2/5] Deprecation: improve slow db query Instead of passing 82k slugs on each query to get the user's projects, we perform a set() intersection in Python which is fast. This reduces the time for this query in a pretty noticeable way. --- readthedocs/projects/tasks/utils.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/readthedocs/projects/tasks/utils.py b/readthedocs/projects/tasks/utils.py index 8b9a080b9fe..ef10c97ff0e 100644 --- a/readthedocs/projects/tasks/utils.py +++ b/readthedocs/projects/tasks/utils.py @@ -291,13 +291,15 @@ def deprecated_config_file_used_notification(): ) # All the projects for this user that don't have a configuration file - user_projects = ( - AdminPermission.projects(user, admin=True) - .filter(slug__in=projects) - .only("slug") + # Use set() intersection in Python that's pretty quick and only pass + # these few projects slugs to the db query. + # Otherwise we pass 82k, which makes the query pretty slow. + user_projects = AdminPermission.projects(user, admin=True).values_list( + "slug", flat=True ) + user_projects = list(set(user_projects) & projects) - user_project_slugs = ", ".join([p.slug for p in user_projects[:5]]) + user_project_slugs = ", ".join(user_projects[:5]) if user_projects.count() > 5: user_project_slugs += " and others..." From 8ef92646672da6cd7729d83d61f4d690bc8c1b8b Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Fri, 9 Jun 2023 01:25:50 +0200 Subject: [PATCH 3/5] Deprecation: adapt comment --- readthedocs/projects/tasks/utils.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/readthedocs/projects/tasks/utils.py b/readthedocs/projects/tasks/utils.py index ef10c97ff0e..71555930b75 100644 --- a/readthedocs/projects/tasks/utils.py +++ b/readthedocs/projects/tasks/utils.py @@ -291,9 +291,8 @@ def deprecated_config_file_used_notification(): ) # All the projects for this user that don't have a configuration file - # Use set() intersection in Python that's pretty quick and only pass - # these few projects slugs to the db query. - # Otherwise we pass 82k, which makes the query pretty slow. + # Use set() intersection in Python that's pretty quick since we only need the slugs. + # Otherwise we have to pass 82k slugs to the DB query, which makes it pretty slow. user_projects = AdminPermission.projects(user, admin=True).values_list( "slug", flat=True ) From a933c7019d104c6f36a89c5053b2d5c78d00c849 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Fri, 9 Jun 2023 10:31:35 +0200 Subject: [PATCH 4/5] Minor fix --- readthedocs/projects/tasks/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/projects/tasks/utils.py b/readthedocs/projects/tasks/utils.py index 71555930b75..e2489300df0 100644 --- a/readthedocs/projects/tasks/utils.py +++ b/readthedocs/projects/tasks/utils.py @@ -299,7 +299,7 @@ def deprecated_config_file_used_notification(): user_projects = list(set(user_projects) & projects) user_project_slugs = ", ".join(user_projects[:5]) - if user_projects.count() > 5: + if len(user_projects) > 5: user_project_slugs += " and others..." n_site = DeprecatedConfigFileSiteNotification( From 5b5c5b91f928cb564d2f67f5958ff2ed236064c5 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Fri, 9 Jun 2023 10:56:37 +0200 Subject: [PATCH 5/5] Remove `.exclude` of unread notifications This will interfere with the logic for sending email notifications. We do want to send an email even if the user didn't read the onsite notification. --- readthedocs/projects/tasks/utils.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/readthedocs/projects/tasks/utils.py b/readthedocs/projects/tasks/utils.py index e2489300df0..7bab1f66a2b 100644 --- a/readthedocs/projects/tasks/utils.py +++ b/readthedocs/projects/tasks/utils.py @@ -272,13 +272,6 @@ def deprecated_config_file_used_notification(): "id" ) - # Exclude users that already have an unread notification. - # Our notifications backend already performs de-duplication of notifications automatically. - # However, this is only to speed up things here since this loop takes the most time. - queryset = queryset.exclude( - message__message__startswith="Your project(s)", message__read=False - ) - n_users = queryset.count() for i, user in enumerate(queryset.iterator()): if i % 500 == 0: