Skip to content

Use shared code for team member check

Brian Williams requested to merge bwill/use-shared-code-for-member-check into master

What does this MR do and why?

Describe in detail what your merge request does and why.

It was pointed out at !109245 (comment 1273068446) that there is some existing code (Gitlab::Com.gitlab_com_group_member?) to check if a user is a team member by querying gitlab-com. This MR changes Gitlab::Com.gitlab_com_group_member? to take a user, and calls it directly for the feature flag check.

Additionally, this MR uses the more optimal of the two database queries. This MR does not introduce any new database queries, but I have prepared it for database review anyway since it is better to be safe than sorry.

Before: 4 seconds https://console.postgres.ai/shared/a8bcf2cf-ad73-45df-a443-3680cff8d883

Before
SELECT
    members.user_id
FROM
    (
        SELECT
            DISTINCT ON ( user_id, invite_email )
            *
        FROM
            members
        WHERE
            members.type = 'GroupMember' AND
            members.source_type = 'Namespace' AND
            members.requested_at IS NULL AND
            members.source_id IN (
                SELECT
                    namespaces.id
                FROM
                    (
                        SELECT
                            namespaces.*
                        FROM
                            namespaces
                        WHERE
                            namespaces.type = 'Group' AND
                            namespaces.id = 6543
                    ) AS namespaces
                WHERE
                    namespaces.type = 'Group'
            )
        ORDER BY
            user_id,
            invite_email,
            access_level DESC,
            expires_at DESC,
            created_at ASC
    ) AS members
WHERE
    members.type = 'GroupMember';
 Subquery Scan on members  (cost=537.65..539.41 rows=1 width=4) (actual time=4100.032..4101.336 rows=2227 loops=1)
   Filter: ((members.type)::text = 'GroupMember'::text)
   Rows Removed by Filter: 0
   Buffers: shared hit=12 read=1970 dirtied=31
   I/O Timings: read=4033.152 write=0.000
   ->  Unique  (cost=537.65..538.31 rows=88 width=1147) (actual time=4100.028..4100.930 rows=2227 loops=1)
         Buffers: shared hit=12 read=1970 dirtied=31
         I/O Timings: read=4033.152 write=0.000
         ->  Sort  (cost=537.65..537.87 rows=88 width=1147) (actual time=4100.026..4100.254 rows=2227 loops=1)
               Sort Key: members_1.user_id, members_1.invite_email, members_1.access_level DESC, members_1.expires_at DESC, members_1.created_at
               Sort Method: quicksort  Memory: 378kB
               Buffers: shared hit=12 read=1970 dirtied=31
               I/O Timings: read=4033.152 write=0.000
               ->  Nested Loop  (cost=1.13..534.81 rows=88 width=1147) (actual time=53.651..4088.570 rows=2227 loops=1)
                     Buffers: shared read=1970 dirtied=31
                     I/O Timings: read=4033.152 write=0.000
                     ->  Index Scan using namespaces_pkey on public.namespaces  (cost=0.56..3.58 rows=1 width=4) (actual time=23.647..23.649 rows=1 loops=1)
                           Index Cond: (namespaces.id = 6543)
                           Filter: ((namespaces.type)::text = 'Group'::text)
                           Rows Removed by Filter: 0
                           Buffers: shared read=5
                           I/O Timings: read=23.586 write=0.000
                     ->  Index Scan using index_members_on_source_id_and_source_type on public.members members_1  (cost=0.56..530.34 rows=88 width=58) (actual time=29.995..4061.049 rows=2227 loops=1)
                           Index Cond: ((members_1.source_id = 6543) AND ((members_1.source_type)::text = 'Namespace'::text))
                           Filter: ((members_1.requested_at IS NULL) AND ((members_1.type)::text = 'GroupMember'::text))
                           Rows Removed by Filter: 0
                           Buffers: shared read=1965 dirtied=31
                           I/O Timings: read=4009.567 write=0.000
Time: 4.107 s
  - planning: 4.968 ms
  - execution: 4.102 s
    - I/O read: 4.033 s
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 12 (~96.00 KiB) from the buffer pool
  - reads: 1970 (~15.40 MiB) from the OS file cache, including disk I/O
  - dirtied: 31 (~248.00 KiB)
  - writes: 0

After: 17 ms https://console.postgres.ai/shared/15e0d4f7-2156-4fc3-949e-7529a3936040

After
SELECT
    "members"."user_id"
FROM
    "members"
WHERE
    "members"."type" = 'GroupMember'
    AND "members"."source_id" = 22
    AND "members"."source_type" = 'Namespace'
    AND "members"."requested_at" IS NULL
    AND "members"."access_level" != 5
 Index Scan using index_members_on_source_id_and_source_type on public.members  (cost=0.56..3.59 rows=1 width=4) (actual time=9.012..9.013 rows=0 loops=1)
   Index Cond: ((members.source_id = 22) AND ((members.source_type)::text = 'Namespace'::text))
   Filter: ((members.requested_at IS NULL) AND (members.access_level <> 5) AND ((members.type)::text = 'GroupMember'::text))
   Rows Removed by Filter: 0
   Buffers: shared read=4
   I/O Timings: read=8.967 write=0.000
Time: 12.377 ms
  - planning: 3.323 ms
  - execution: 9.054 ms
    - I/O read: 8.967 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 0 from the buffer pool
  - reads: 4 (~32.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

Note that the new query does not do permission or visibility checks. This is ok, as check is specific to gitlab-com, which is publicly and likely always will be.

MR acceptance checklist

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

Edited by Brian Williams

Merge request reports

Loading