From f3b04125a6795ade278791af636eb114d499d985 Mon Sep 17 00:00:00 2001 From: Dirk Julich Date: Tue, 7 Jul 2026 16:52:11 +0200 Subject: [PATCH] =?UTF-8?q?AAP-81860=20=E2=80=94=20Eliminate=20LEFT=20OUTE?= =?UTF-8?q?R=20JOINs=20in=20ActivityStream=20RBAC=20query=20(#16533)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * AAP-81860 — Eliminate LEFT OUTER JOINs in ActivityStream RBAC query ActivityStreamAccess.filtered_queryset() used field-traversal Q objects (e.g. Q(host__inventory__in=...)) across 18 M2M relationships, which Django translates into LEFT OUTER JOINs. This forces PostgreSQL to join all intermediary tables before filtering, making it the #2 DB consumer at 483s in a 10-minute Scale Lab window. Replace all M2M field traversals with pk__in subqueries using .through intermediary tables, following the same pattern proven in AAP-81082 (28% improvement for unified job RBAC). This changes the SQL from LEFT OUTER JOIN to IN (SELECT ...) semi-joins, allowing PostgreSQL to skip the unconditional joins. Also: - Remove .distinct() (no longer needed without M2M JOINs) - Remove unnecessary if-truthy checks that evaluated subqueries as boolean before including them (5 extra DB roundtrips per request) - Migrate accessible_pk_qs(user, 'read_role') to access_ids_qs(user, 'view') for direct DAB RBAC API usage * Use .exists() instead of queryset truth-test for auditing_orgs guard Avoids evaluating the full queryset when checking whether the user has auditing orgs — .exists() issues a cheaper SELECT 1 … LIMIT 1 instead. * Restore .exists() guards for resource-type Q branches Re-add conditional guards around inventory, credential, project, job template, workflow job template, and team Q branches. Uses .exists() (cheap SELECT 1 LIMIT 1) instead of the old queryset truth-test to avoid unnecessary query evaluation while still preventing generation of an overly complex query when the user has no access to a given resource type. --------- Co-authored-by: Claude Opus 4.6 --- awx/main/access.py | 93 ++++++++++++++++++++++++++++------------------ 1 file changed, 56 insertions(+), 37 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 05c67d06c4..4ecc0c1fd1 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -2714,54 +2714,73 @@ class ActivityStreamAccess(BaseAccess): # 'job_template', 'job', 'project', 'project_update', 'workflow_job', # 'inventory_source', 'workflow_job_template' - q = Q(user=self.user) - inventory_set = Inventory.accessible_pk_qs(self.user, 'read_role') - if inventory_set: + AS = ActivityStream + + q = Q(pk__in=AS.user.through.objects.filter(user=self.user).values('activitystream_id')) + + inventory_set = Inventory.access_ids_qs(self.user, 'view') + if inventory_set.exists(): q |= ( - Q(ad_hoc_command__inventory__in=inventory_set) - | Q(inventory__in=inventory_set) - | Q(host__inventory__in=inventory_set) - | Q(group__inventory__in=inventory_set) - | Q(inventory_source__inventory__in=inventory_set) - | Q(inventory_update__inventory_source__inventory__in=inventory_set) + Q(pk__in=AS.ad_hoc_command.through.objects.filter(adhoccommand__inventory__in=inventory_set).values('activitystream_id')) + | Q(pk__in=AS.inventory.through.objects.filter(inventory__in=inventory_set).values('activitystream_id')) + | Q(pk__in=AS.host.through.objects.filter(host__inventory__in=inventory_set).values('activitystream_id')) + | Q(pk__in=AS.group.through.objects.filter(group__inventory__in=inventory_set).values('activitystream_id')) + | Q(pk__in=AS.inventory_source.through.objects.filter(inventorysource__inventory__in=inventory_set).values('activitystream_id')) + | Q( + pk__in=AS.inventory_update.through.objects.filter(inventoryupdate__inventory_source__inventory__in=inventory_set).values( + 'activitystream_id' + ) + ) ) - credential_set = Credential.accessible_pk_qs(self.user, 'read_role') - if credential_set: - q |= Q(credential__in=credential_set) + credential_set = Credential.access_ids_qs(self.user, 'view') + if credential_set.exists(): + q |= Q(pk__in=AS.credential.through.objects.filter(credential__in=credential_set).values('activitystream_id')) auditing_orgs = (Organization.access_qs(self.user, 'change') | Organization.access_qs(self.user, 'audit')).distinct().values_list('id', flat=True) - if auditing_orgs: + if auditing_orgs.exists(): q |= ( - Q(user__in=auditing_orgs.values('member_role__members')) - | Q(organization__in=auditing_orgs) - | Q(notification_template__organization__in=auditing_orgs) - | Q(notification__notification_template__organization__in=auditing_orgs) - | Q(label__organization__in=auditing_orgs) - | Q(role__in=Role.visible_roles(self.user) if auditing_orgs else []) + Q(pk__in=AS.user.through.objects.filter(user__in=auditing_orgs.values('member_role__members')).values('activitystream_id')) + | Q(pk__in=AS.organization.through.objects.filter(organization__in=auditing_orgs).values('activitystream_id')) + | Q(pk__in=AS.notification_template.through.objects.filter(notificationtemplate__organization__in=auditing_orgs).values('activitystream_id')) + | Q( + pk__in=AS.notification.through.objects.filter(notification__notification_template__organization__in=auditing_orgs).values( + 'activitystream_id' + ) + ) + | Q(pk__in=AS.label.through.objects.filter(label__organization__in=auditing_orgs).values('activitystream_id')) + | Q(pk__in=AS.role.through.objects.filter(role__in=Role.visible_roles(self.user)).values('activitystream_id')) ) - project_set = Project.accessible_pk_qs(self.user, 'read_role') - if project_set: - q |= Q(project__in=project_set) | Q(project_update__project__in=project_set) - - jt_set = JobTemplate.accessible_pk_qs(self.user, 'read_role') - if jt_set: - q |= Q(job_template__in=jt_set) | Q(job__job_template__in=jt_set) - - wfjt_set = WorkflowJobTemplate.accessible_pk_qs(self.user, 'read_role') - if wfjt_set: - q |= ( - Q(workflow_job_template__in=wfjt_set) - | Q(workflow_job_template_node__workflow_job_template__in=wfjt_set) - | Q(workflow_job__workflow_job_template__in=wfjt_set) + project_set = Project.access_ids_qs(self.user, 'view') + if project_set.exists(): + q |= Q(pk__in=AS.project.through.objects.filter(project__in=project_set).values('activitystream_id')) | Q( + pk__in=AS.project_update.through.objects.filter(projectupdate__project__in=project_set).values('activitystream_id') ) - team_set = Team.accessible_pk_qs(self.user, 'read_role') - if team_set: - q |= Q(team__in=team_set) + jt_set = JobTemplate.access_ids_qs(self.user, 'view') + if jt_set.exists(): + q |= Q(pk__in=AS.job_template.through.objects.filter(jobtemplate__in=jt_set).values('activitystream_id')) | Q( + pk__in=AS.job.through.objects.filter(job__job_template__in=jt_set).values('activitystream_id') + ) - return qs.filter(q).distinct() + wfjt_set = WorkflowJobTemplate.access_ids_qs(self.user, 'view') + if wfjt_set.exists(): + q |= ( + Q(pk__in=AS.workflow_job_template.through.objects.filter(workflowjobtemplate__in=wfjt_set).values('activitystream_id')) + | Q( + pk__in=AS.workflow_job_template_node.through.objects.filter(workflowjobtemplatenode__workflow_job_template__in=wfjt_set).values( + 'activitystream_id' + ) + ) + | Q(pk__in=AS.workflow_job.through.objects.filter(workflowjob__workflow_job_template__in=wfjt_set).values('activitystream_id')) + ) + + team_set = Team.access_ids_qs(self.user, 'view') + if team_set.exists(): + q |= Q(pk__in=AS.team.through.objects.filter(team__in=team_set).values('activitystream_id')) + + return qs.filter(q) def can_add(self, data): return False