Fix cross joins on group members
What does this MR do and why?
Addressing the last part of : #432604 (closed)
As part of the Cells project, we are changing many of the queries that are doing cross database joins to querying both databases main
and main_clusterwide
, while maintaining the current performance of the querying. For more on the topic, you can refer to this documentation.
This MR was extracted from a bigger MR: !138218 (closed)
More context about what's being changed in this MR
This MR addresses the issue of resolving the cross joins on both users
and owners
on Group
class. users and
owners` are called in many places in the app, that is why the scope of this issue big.
This issue started with two previous MRs that are meant to solve this iteratively.
.non_invite
on members
relations
Some context regarding In this and the previous MRs on this issue, we are replacing group.owners
and group.users
that causes cross database query to something that depends on group.group_members
and group.all_owner_members
. But something that was missing from the previous MR that has even caused an issue on GitLab.com that the members
table can have user_id
set to NULL
. And that's only in case when the member
record means an invited user. That's why in this MR you would see a lot of all_owner_members.non_invite
to make sure we are getting the members
records that have user, and that this user is not invited.
Things that are done in this MR
-
Rename
preload_user
topreload_users
. This was a follow up from the previous MR. -
The most important part of this MR: we are removing the allow cross join on both
users
andowners
, to prevent them from being used without explicitly allowing the cross joins temporarily if needed. -
Cross joins have been solved in many components and tests in the app.
-
We explicitly allowed some cross joins that need might more investigation and testing, or in places that haven't been decided yet. For example: Personal Tokens, Tests that call
owners
,users
andbots
. -
Changed many tests to use
.to have_user
to test whether a user is part of the group or not. See the previous MR. -
No known new queries have been added in this MR.
Newly added queries.
group.all_owner_members.non_invite.each_batch
Loading the first batch of owners
on gitlab-org
: https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/25084/commands/79661
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.