Cells: Fix cross joins in backup related code
What does this MR do and why?
Refactor: Removes unnecessary cross-table-joins which will not be allowed in the future (Cells) when namespaces and users tables are stored in separate databases.
Resolves #417467 (closed)
Why is it safe?
See #417467 (comment 1514678828):
Looks like these namespaces-users joins were added in !40679 (7ffa9e59).
At the time, it was stated that the change was made to avoid N+1 queries. Note that the N+1 that the N+1 test was checking for was the one that was fixed by including
route
. That's the only reason the N+1 test exists. It was also stated that we don't expect theowner
N+1s to be a problem in practice: !40679 (comment 409374466).That's to say, I'm not concerned that this will cause a performance regression. Iterating over
ProjectWiki
is not the common usage.Additionally, backups:
- are performed completely outside of web requests and Sidekiq jobs
- iterate over potentially all projects
- are expected to take a long time in many cases
Therefore I expect that we can remove the users table (the
owner
association) from theincludes
calls, and add an exception for any auto-detected N+1s.
Furthermore, it turns out that the N+1 test doesn't fail on this change.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.