Skip to content

Fix cross joins on group members

Omar Qunsul requested to merge 432604-group-cross-joins-fix into master

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.

cross_joins

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.

  1. !139281 (merged)
  2. !139690 (merged)

Some context regarding .non_invite on members relations

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

  1. Rename preload_user to preload_users. This was a follow up from the previous MR.

  2. The most important part of this MR: we are removing the allow cross join on both users and owners, to prevent them from being used without explicitly allowing the cross joins temporarily if needed.

  3. Cross joins have been solved in many components and tests in the app.

  4. 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 and bots.

  5. Changed many tests to use .to have_user to test whether a user is part of the group or not. See the previous MR.

  6. No known new queries have been added in this MR.

Newly added queries.

  1. 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.

Edited by Omar Qunsul

Merge request reports