Skip to content

Resolving group and project membership check cross joins

Omar Qunsul requested to merge 432604-group-membership-check into master

What does this MR do and why?

Addressing 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)

This MR addresses mainly the views and specs of solving cross joins on Group#members and Group#owners.

To summarize the changes in this MR:

(Changed Query) Checking whether a user a direct owner of a group

This is changed on Group#owned_by?

https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/24872/commands/79012

(Changed Query) Checking whether a user is one of the direct members of a group

This was common in many of the tests. By doing something like group.users it 1) causes a cross join between members and users, which we are solving, and 2) it loads ALL the users from the users table for no necessary reason. Instead, by replacing that with group.has_user? (new method), which is using exists? that uses a very efficient query on the members table only.

This method is not used in normal code yet, but mainly used for tests as of now.

https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/24872/commands/79013

(Changed Query) Counting the number of group members

Instead of group.users.count, we can also use group.group_members.count instead.

https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/24872/commands/79014

Group Membership check in tests

Adding a new matcher expect(group).to have_user(user) to make it easy to see if a user is a direct member of a group without cross join query or not.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #432604 (closed)

Edited by Omar Qunsul

Merge request reports

Loading