Skip to content

Draft: [SPIKE] Enhanced filtering based on project/group

Bishwa Hang Rai requested to merge 428880-spike-enhance-filter into master

What does this MR do and why?

This is a SPIKE MR to investigate and gather feedback to add group/project level filtering to existing AddOn eligible user list.

  1. Resolver: ee/app/graphql/resolvers/gitlab_subscriptions/add_on_eligible_users_resolver.rb
  2. Finder: ee/app/finders/gitlab_subscriptions/add_on_eligible_users_finder.rb

Related previous MR: !129203 (merged)

Screenshots or screen recordings

How to set up and validate locally

GraphQL queries

  1. Filter by group(sub-group):
by_group 
{
  namespace(fullPath: "<GROUP-FULL-PATH>") {
    addOnEligibleUsers(addOnType: CODE_SUGGESTIONS, filterByGroup: "gid://gitlab/Group/<SUB-GROUP-ID>", search: "test") {
      nodes {
        id
        username
        name
        publicEmail
        avatarUrl
        webUrl
        lastActivityOn
        lastLoginAt
        addOnAssignments(
          addOnPurchaseIds: ["gid://gitlab/GitlabSubscriptions::AddOnPurchase/<ADD-ON-PURCHASE-ID>"]
        ) {
          nodes {
            addOnPurchase {
              name
            }
          }
        }
      }
    }
  }
}
  1. Filter by project:
by_project 
{
  namespace(fullPath: "<GROUP-FULL-PATH>") {
    addOnEligibleUsers(addOnType: CODE_SUGGESTIONS, filterByProject: "gid://gitlab/Project/<PROJECT-ID>") {
      nodes {
        id
        username
        name
        publicEmail
        avatarUrl
        webUrl
        lastActivityOn
        lastLoginAt
        addOnAssignments(
          addOnPurchaseIds: ["gid://gitlab/GitlabSubscriptions::AddOnPurchase/<ADD-ON-PURCHASE-ID>"]
        ) {
          nodes {
            addOnPurchase {
              name
            }
          }
        }
      }
    }

Create seed records and test query

  1. Create a new group. Add yourself (logged in user) as owner.
  2. Also add few members as guest and developer to the group
  3. Create a sub-group and add few members as developer/guest
  4. Create a project for the sub-group and group, and add few members as developer/guest
  5. Enable the hamilton_seat_management FF
    • Feature.enable(:hamilton_seat_management)
  6. Create an add on purchase for that group
    • GitlabSubscriptions::AddOnPurchase.create!(add_on: GitlabSubscriptions::AddOn.first, expires_on: 1.year.from_now, namespace_id: <GROUP-ID>, quantity: 10, purchase_xid: 'A-12345')
  7. Visit http://gdk.test:3000/-/graphql-explorer, and add the query specified above

SQL Performance Analysis

Recap on existing implementation

To recap the existing SQL query for the AddOnEligibleUsersFinder, please visit the previous MR: !129203 (merged) Relavant comment discussion: !129203 (comment 1559493910) And query plan:

  1. First run: https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/22786/commands/73487
  2. Second run: https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/22786/commands/73488

SPIKE

This SPIKE MR refactors the existing query to add filtering based by group or project, that will be passed in as query argument.

If the list if filtered by a group/project, we will only show eligible members that is:

  1. direct members of the group/project
  2. inherited members of the ancestors of group/project
  3. invited members to the self and ancestors

If we have a tree structure something like below, with Germany and France as root-group:

Germany {direct: ['user-1', 'user-2']}
  |
   - Munich {direct: 'user-3' , inherited: ['user-1', 'user-2']}
           |
           Project-X {direct: 'user-4' , inherited: ['user-1', 'user-2', 'user-3']}
   - Berlin {direct: 'user-5', inherited: ['user-1', 'user-2'], invited: {Paris: {'user-a', 'user-b'}}


France {direct: ['user-a']}
  |
   - Paris {direct: 'user-b' , inherited: ['user-a']}

Considering we are querying for eligible users, inside the scope of root-group Germany:

  1. If we filterByGroup: 'Munich', then the eligible users list would be: [user-1, user-2, user-3]
  2. If we filterByProject: 'Project-X', then the eligible users list would be: [user-1, user-2, user-3, user-4]
  3. If we filterByGroup: 'Berlin', then the eligible users list would be: [user-1, user-2, user-5, user-a, user-b]

Final SQL and query plan is shown in this comment: !135586 (comment 1639421306)

For historical log on how we started: 👇

Filter by Group

SQL 
SELECT
    "users"."id",
    "users"."email",
    "users"."encrypted_password",
    "users"."reset_password_token",
    "users"."reset_password_sent_at",
    "users"."remember_created_at",
    "users"."sign_in_count",
    "users"."current_sign_in_at",
    "users"."last_sign_in_at",
    "users"."current_sign_in_ip",
    "users"."last_sign_in_ip",
    "users"."created_at",
    "users"."updated_at",
    "users"."name",
    "users"."admin",
    "users"."projects_limit",
    "users"."failed_attempts",
    "users"."locked_at",
    "users"."username",
    "users"."can_create_group",
    "users"."can_create_team",
    "users"."state",
    "users"."color_scheme_id",
    "users"."password_expires_at",
    "users"."created_by_id",
    "users"."last_credential_check_at",
    "users"."avatar",
    "users"."confirmation_token",
    "users"."confirmed_at",
    "users"."confirmation_sent_at",
    "users"."unconfirmed_email",
    "users"."hide_no_ssh_key",
    "users"."admin_email_unsubscribed_at",
    "users"."notification_email",
    "users"."hide_no_password",
    "users"."password_automatically_set",
    "users"."encrypted_otp_secret",
    "users"."encrypted_otp_secret_iv",
    "users"."encrypted_otp_secret_salt",
    "users"."otp_required_for_login",
    "users"."otp_backup_codes",
    "users"."public_email",
    "users"."dashboard",
    "users"."project_view",
    "users"."consumed_timestep",
    "users"."layout",
    "users"."hide_project_limit",
    "users"."note",
    "users"."unlock_token",
    "users"."otp_grace_period_started_at",
    "users"."external",
    "users"."incoming_email_token",
    "users"."auditor",
    "users"."require_two_factor_authentication_from_group",
    "users"."two_factor_grace_period",
    "users"."last_activity_on",
    "users"."notified_of_own_activity",
    "users"."preferred_language",
    "users"."theme_id",
    "users"."accepted_term_id",
    "users"."feed_token",
    "users"."private_profile",
    "users"."roadmap_layout",
    "users"."include_private_contributions",
    "users"."commit_email",
    "users"."group_view",
    "users"."managing_group_id",
    "users"."first_name",
    "users"."last_name",
    "users"."static_object_token",
    "users"."role",
    "users"."user_type",
    "users"."static_object_token_encrypted",
    "users"."otp_secret_expires_at",
    "users"."onboarding_in_progress"
FROM
    "users"
WHERE
    "users"."id" IN (WITH "our_namespace_bans" AS (
            SELECT
                "namespace_bans"."user_id"
            FROM
                "namespace_bans"
            WHERE
                "namespace_bans"."namespace_id" = 9970), "group_ancestors" AS (
                SELECT
                    "namespaces"."id", "namespaces"."name", "namespaces"."path", "namespaces"."owner_id", "namespaces"."created_at", "namespaces"."updated_at", "namespaces"."type", "namespaces"."description", "namespaces"."avatar", "namespaces"."membership_lock", "namespaces"."share_with_group_lock", "namespaces"."visibility_level", "namespaces"."request_access_enabled", "namespaces"."ldap_sync_status", "namespaces"."ldap_sync_error", "namespaces"."ldap_sync_last_update_at", "namespaces"."ldap_sync_last_successful_update_at", "namespaces"."ldap_sync_last_sync_at", "namespaces"."description_html", "namespaces"."lfs_enabled", "namespaces"."parent_id", "namespaces"."shared_runners_minutes_limit", "namespaces"."repository_size_limit", "namespaces"."require_two_factor_authentication", "namespaces"."two_factor_grace_period", "namespaces"."cached_markdown_version", "namespaces"."project_creation_level", "namespaces"."runners_token", "namespaces"."file_template_project_id", "namespaces"."saml_discovery_token", "namespaces"."runners_token_encrypted", "namespaces"."custom_project_templates_group_id", "namespaces"."auto_devops_enabled", "namespaces"."extra_shared_runners_minutes_limit", "namespaces"."last_ci_minutes_notification_at", "namespaces"."last_ci_minutes_usage_notification_level", "namespaces"."subgroup_creation_level", "namespaces"."emails_disabled", "namespaces"."max_pages_size", "namespaces"."max_artifacts_size", "namespaces"."mentions_disabled", "namespaces"."default_branch_protection", "namespaces"."max_personal_access_token_lifetime", "namespaces"."push_rule_id", "namespaces"."shared_runners_enabled", "namespaces"."allow_descendants_override_disabled_shared_runners", "namespaces"."traversal_ids", "namespaces"."organization_id"
                FROM
                    "namespaces"
                WHERE
                    "namespaces"."type" = 'Group'
                    AND "namespaces"."id" IN (9970, 2255492, 2259137, 2259139, 2259140)), "groups_invited_to_group_ancestors_cte" AS (
                    SELECT
                        "namespaces"."traversal_ids"
                    FROM
                        "namespaces"
                    WHERE
                        "namespaces"."type" = 'Group'
                        AND ("namespaces"."id" IN (
                                SELECT
                                    "group_group_links"."shared_with_group_id"
                                FROM
                                    group_group_links
                                WHERE
                                    "group_group_links"."shared_group_id" IN (
                                        SELECT
                                            id
                                        FROM
                                            group_ancestors))))
                        SELECT
                            "user_id"
                        FROM (
                            SELECT
                                "members".*
                            FROM
                                "members"
                            LEFT OUTER JOIN "users" ON "users"."id" = "members"."user_id"
                    WHERE
                        "members"."type" = 'GroupMember'
                        AND "members"."source_type" = 'Namespace'
                        AND ("users"."state" IN ('active'))
                        AND "users"."user_type" IN (0, 4, 5)
                        AND "members"."state" = 0
                        AND "members"."requested_at" IS NULL
                        AND "members"."invite_token" IS NULL
                        AND (members.access_level > 5)
                        AND "members"."user_id" NOT IN (
                            SELECT
                                "user_id"
                            FROM
                                "our_namespace_bans")
                            AND ("members"."source_id" IN (
                                    SELECT
                                        unnest("groups_invited_to_group_ancestors_cte"."traversal_ids")
                                    FROM
                                        groups_invited_to_group_ancestors_cte
                                    UNION
                                    SELECT
                                        id
                                    FROM
                                        group_ancestors))) subquery
)
ORDER BY
    "users"."id" DESC
LIMIT 101;
  1. cold run
  2. hot run

Filter by project query

SQL 
SELECT
    "users"."id",
    "users"."email",
    "users"."encrypted_password",
    "users"."reset_password_token",
    "users"."reset_password_sent_at",
    "users"."remember_created_at",
    "users"."sign_in_count",
    "users"."current_sign_in_at",
    "users"."last_sign_in_at",
    "users"."current_sign_in_ip",
    "users"."last_sign_in_ip",
    "users"."created_at",
    "users"."updated_at",
    "users"."name",
    "users"."admin",
    "users"."projects_limit",
    "users"."failed_attempts",
    "users"."locked_at",
    "users"."username",
    "users"."can_create_group",
    "users"."can_create_team",
    "users"."state",
    "users"."color_scheme_id",
    "users"."password_expires_at",
    "users"."created_by_id",
    "users"."last_credential_check_at",
    "users"."avatar",
    "users"."confirmation_token",
    "users"."confirmed_at",
    "users"."confirmation_sent_at",
    "users"."unconfirmed_email",
    "users"."hide_no_ssh_key",
    "users"."admin_email_unsubscribed_at",
    "users"."notification_email",
    "users"."hide_no_password",
    "users"."password_automatically_set",
    "users"."encrypted_otp_secret",
    "users"."encrypted_otp_secret_iv",
    "users"."encrypted_otp_secret_salt",
    "users"."otp_required_for_login",
    "users"."otp_backup_codes",
    "users"."public_email",
    "users"."dashboard",
    "users"."project_view",
    "users"."consumed_timestep",
    "users"."layout",
    "users"."hide_project_limit",
    "users"."note",
    "users"."unlock_token",
    "users"."otp_grace_period_started_at",
    "users"."external",
    "users"."incoming_email_token",
    "users"."auditor",
    "users"."require_two_factor_authentication_from_group",
    "users"."two_factor_grace_period",
    "users"."last_activity_on",
    "users"."notified_of_own_activity",
    "users"."preferred_language",
    "users"."theme_id",
    "users"."accepted_term_id",
    "users"."feed_token",
    "users"."private_profile",
    "users"."roadmap_layout",
    "users"."include_private_contributions",
    "users"."commit_email",
    "users"."group_view",
    "users"."managing_group_id",
    "users"."first_name",
    "users"."last_name",
    "users"."static_object_token",
    "users"."role",
    "users"."user_type",
    "users"."static_object_token_encrypted",
    "users"."otp_secret_expires_at",
    "users"."onboarding_in_progress"
FROM
    "users"
WHERE
    "users"."id" IN (WITH "our_namespace_bans" AS (
            SELECT
                "namespace_bans"."user_id"
            FROM
                "namespace_bans"
            WHERE
                "namespace_bans"."namespace_id" = 9970), "group_ancestors" AS (
                SELECT
                    "namespaces"."id", "namespaces"."name", "namespaces"."path", "namespaces"."owner_id", "namespaces"."created_at", "namespaces"."updated_at", "namespaces"."type", "namespaces"."description", "namespaces"."avatar", "namespaces"."membership_lock", "namespaces"."share_with_group_lock", "namespaces"."visibility_level", "namespaces"."request_access_enabled", "namespaces"."ldap_sync_status", "namespaces"."ldap_sync_error", "namespaces"."ldap_sync_last_update_at", "namespaces"."ldap_sync_last_successful_update_at", "namespaces"."ldap_sync_last_sync_at", "namespaces"."description_html", "namespaces"."lfs_enabled", "namespaces"."parent_id", "namespaces"."shared_runners_minutes_limit", "namespaces"."repository_size_limit", "namespaces"."require_two_factor_authentication", "namespaces"."two_factor_grace_period", "namespaces"."cached_markdown_version", "namespaces"."project_creation_level", "namespaces"."runners_token", "namespaces"."file_template_project_id", "namespaces"."saml_discovery_token", "namespaces"."runners_token_encrypted", "namespaces"."custom_project_templates_group_id", "namespaces"."auto_devops_enabled", "namespaces"."extra_shared_runners_minutes_limit", "namespaces"."last_ci_minutes_notification_at", "namespaces"."last_ci_minutes_usage_notification_level", "namespaces"."subgroup_creation_level", "namespaces"."emails_disabled", "namespaces"."max_pages_size", "namespaces"."max_artifacts_size", "namespaces"."mentions_disabled", "namespaces"."default_branch_protection", "namespaces"."max_personal_access_token_lifetime", "namespaces"."push_rule_id", "namespaces"."shared_runners_enabled", "namespaces"."allow_descendants_override_disabled_shared_runners", "namespaces"."traversal_ids", "namespaces"."organization_id"
                FROM
                    "namespaces"
                WHERE
                    "namespaces"."type" = 'Group'
                    AND "namespaces"."id" IN (9970)), "groups_invited_to_group_ancestors_cte" AS (
                    SELECT
                        "namespaces"."traversal_ids"
                    FROM
                        "namespaces"
                    WHERE
                        "namespaces"."type" = 'Group'
                        AND ("namespaces"."id" IN (
                                SELECT
                                    "group_group_links"."shared_with_group_id"
                                FROM
                                    group_group_links
                                WHERE
                                    "group_group_links"."shared_group_id" IN (
                                        SELECT
                                            id
                                        FROM
                                            group_ancestors)
                                    UNION
                                    SELECT
                                        "project_group_links"."group_id"
                                    FROM
                                        project_group_links
                                    WHERE
                                        "project_group_links"."project_id" = 278964)))
                        SELECT
                            "members"."user_id"
                        FROM ((
                                SELECT
                                    "members".*
                                FROM
                                    "members"
                                LEFT OUTER JOIN "users" ON "users"."id" = "members"."user_id"
                        WHERE
                            "members"."type" = 'ProjectMember'
                            AND "members"."source_type" = 'Project'
                            AND ("users"."state" IN ('active'))
                            AND "users"."user_type" IN (0, 4, 5)
                            AND "members"."state" = 0
                            AND "members"."requested_at" IS NULL
                            AND "members"."invite_token" IS NULL
                            AND (members.access_level > 5)
                            AND "members"."user_id" NOT IN (
                                SELECT
                                    "user_id"
                                FROM
                                    "our_namespace_bans")
                                AND "members"."source_id" = 278964)
                        UNION (
                            SELECT
                                "members".*
                            FROM
                                "members"
                            LEFT OUTER JOIN "users" ON "users"."id" = "members"."user_id"
                    WHERE
                        "members"."type" = 'GroupMember'
                        AND "members"."source_type" = 'Namespace'
                        AND ("users"."state" IN ('active'))
                        AND "users"."user_type" IN (0, 4, 5)
                        AND "members"."state" = 0
                        AND "members"."requested_at" IS NULL
                        AND "members"."invite_token" IS NULL
                        AND (members.access_level > 5)
                        AND "members"."user_id" NOT IN (
                            SELECT
                                "user_id"
                            FROM
                                "our_namespace_bans")
                            AND ("members"."source_id" IN (
                                    SELECT
                                        unnest("groups_invited_to_group_ancestors_cte"."traversal_ids")
                                    FROM
                                        groups_invited_to_group_ancestors_cte
                                    UNION
                                    SELECT
                                        id
                                    FROM
                                        group_ancestors)))) members
)
ORDER BY
    "users"."id" DESC
LIMIT 101;
  1. cold run
  2. hot run

Implementation

Existing query to fetch all eligible users has been decomposed to smaller methods and reused to generate query fro each case:

  1. full list of eligible users
  2. filtered list by group
  3. filtered list by project

The major difference in the SQL query between group and project filter is the addition of few extra queries for project:

  1. We need to make extra query to also consider ProjectGroupLink for filter_project
  2. We need to make extra query to get direct ProjectMember for filter_project

Rest is same to fetch direct and invited, members of all the ancestor groups.

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 #428880

Edited by Bishwa Hang Rai

Merge request reports