perf(organization-teams): fix n+1 org query #82109
Conversation
github-actions
bot
added
the
Scope: Backend
src/sentry/models/projectteam.py
Outdated
| orgs = {} | ||
| for team in teams: | ||
| if team.organization_id not in orgs: | ||
| orgs[team.organization_id] = team.organization |
There was a problem hiding this comment.
Since we're here, let's make this a bulk query to fetch all the orgs at once? You can use prefetch_related_objects to do this:
Then there's no need for the orgs dict at all. Just iterate through all the project teams and do set_cached_field_value with the org directly
Nm, we still need the orgs dict because project_teams doesn't have it cached
There was a problem hiding this comment.
there will only ever be one org as where this is called asserts that, but i'll make this change
There was a problem hiding this comment.
Yeah, it's simpler to just stick the prefetch in so that if someone uses this elsewhere it works
src/sentry/models/projectteam.py
Outdated
| @@ -21,9 +21,11 @@ def get_for_teams_with_org_cache(self, teams: Sequence["Team"]) -> QuerySet["Pro | |||
| .order_by("project__name", "project__slug") | |||
| .select_related("project") | |||
| ) | |||
|
|
|||
| # TODO(dcramer): we should query in bulk for ones we're missing here | |||
There was a problem hiding this comment.
Can remove this comment after we bulk fetch
There was a problem hiding this comment.
how feels to delete a # TODO(dcramer):
src/sentry/models/projectteam.py
Outdated
| prefetch_related_objects(project_teams, "project__organization") | ||
| orgs = {pt.project.organization_id: pt.project.organization for pt in project_teams} |
There was a problem hiding this comment.
Hmm, actually I was thinking you'd do this on the teams. But doing it on the project_teams actually does the caching you want in the next loop. So actually, you can just delete everything after line 24, since all that set_cached_field_value does is populate project_team.project.organization.
Reading this even more... We could also just do .select_related("project", "project__organization") and return that list of project teams. No idea why it wasn't done this way when originally implemented...
There was a problem hiding this comment.
yeah this makes sense. simplified.
❌ 1 Tests Failed:
View the top 1 failed tests by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
the way this is currently written it will make the same sql query over and over again 🤦🏼
this fixes it so we simply do once for each org_id. (there should never be more than one org_id in a set of teams being queried, but will leave that for future refactors.