Skip to content

Resolve "Limit ProjectAuthorizations refresh jobs to "distinct" users." [RUN ALL RSPEC] [RUN AS-IF-FOSS]

What does this MR do?

Related to #322098 (closed)

Premise

Imagine your user_id is 1 in the users table, and there is a group named GitLabGroup and imagine that you are explicitly added as a member of each of the following groups

  • the ancestor of the group GitLabGroup,
  • the GitLabGroup group itself
  • and of a group that is shared with GitLabGroup.

If we execute, GitLabGroup.user_ids_for_project_authorizations , it would return an array of user ids, but with duplicate items included like [1,1,1...] (one entry for each of the groups that you are explicitly a member of)

and when it comes to project authroizations worker that uses these array of ids, UserProjectAccessChangedService enqueues multiple jobs but for these same ids.

Using distinct would make sure that we only enqueue one job per user_id, which would of course helps us with enqueuing even fewer jobs than what we enqueue today when a project authorizations refresh is triggered for a specific group, thus

  • reducing number of jobs in the queue, leading to better performance
  • fewer db queries fired
  • lesser time taken for authorization refreshes to complete for a specific group.

PS: Even though our sidekiq de-duplication strategy would have avoided these jobs from being worked on more than once for the same user_id, if we can avoid duplicate jobs from being enqueued in the first place, that would be best 🙂

Link to Kibana dasboard showing deduplicated jobs from AuthorizedProjectsWorker

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Manoj M J

Merge request reports