Skip to content

Fix cross-join for LDAP sync group query

Thong Kuah requested to merge fix_cross_joins_groups_users into master

What does this MR do and why?

Related issue: #417455 (closed)

Fix cross-join for LDAP sync group query. As part of Cells effort, we can no longer cross-join users to members.

Since we do loop over each item in this query, we can transform into each_batch, then preload related identities for each batch.

This MR should not change any functionality, just alter how SQL queries are executed.

Queries

Single member

Before:

  GroupMember Load (2.4ms)  SELECT "members"."id", identities.extern_uid AS distinguished_name, "members"."access_level", "members"."source_id" FROM "members" INNER JOIN "users" ON "users"."id" = "members"."user_id" INNER JOIN "identities" ON "identities"."user_id" = "users"."id" WHERE "members"."type" = 'GroupMember' AND "members"."source_type" = 'Namespace' AND "members"."source_id" IN (SELECT "namespaces"."id" FROM "namespaces" WHERE "namespaces"."type" = 'Group' AND "namespaces"."id" = 4) AND "members"."requested_at" IS NULL AND "identities"."provider" = 'ldapmain' AND "users"."id" IN (SELECT "identities"."user_id" FROM "identities" WHERE (LOWER("identities"."extern_uid") IN (LOWER('uid=user1,ou=users,dc=example,dc=com')))) /* allow_cross_joins_across_databases */ /*application:test,correlation_id:cdb9b1e1c719057a0ad876d88ad08cca,db_config_name:main,line:/ee/lib/ee/gitlab/auth/ldap/sync/group.rb:168:in `inherit_higher_access_levels'*/
   ee/lib/ee/gitlab/auth/ldap/sync/group.rb:168:in `inherit_higher_access_levels'

After:

  GroupMember Load (1.8ms)  SELECT "members"."id" FROM "members" WHERE "members"."type" = 'GroupMember' AND "members"."source_type" = 'Namespace' AND "members"."source_id" IN (SELECT "namespaces"."id" FROM "namespaces" WHERE "namespaces"."type" = 'Group' AND "namespaces"."id" = 207) AND "members"."requested_at" IS NULL ORDER BY "members"."id" ASC LIMIT 1 /*application:test,correlation_id:612533058c613e0c894c4218348bb916,db_config_name:main,line:/app/models/concerns/each_batch.rb:62:in `each_batch'*/
   app/models/concerns/each_batch.rb:62:in `each_batch'
  GroupMember Load (0.9ms)  SELECT "members"."id" FROM "members" WHERE "members"."type" = 'GroupMember' AND "members"."source_type" = 'Namespace' AND "members"."source_id" IN (SELECT "namespaces"."id" FROM "namespaces" WHERE "namespaces"."type" = 'Group' AND "namespaces"."id" = 207) AND "members"."requested_at" IS NULL AND "members"."id" >= 61 ORDER BY "members"."id" ASC LIMIT 1 OFFSET 1000 /*application:test,correlation_id:612533058c613e0c894c4218348bb916,db_config_name:main,line:/app/models/concerns/each_batch.rb:81:in `block in each_batch'*/
  ↳ app/models/concerns/each_batch.rb:81:in `block in each_batch'
  GroupMember Load (0.5ms)  SELECT "members".* FROM "members" WHERE "members"."type" = 'GroupMember' AND "members"."source_type" = 'Namespace' AND "members"."source_id" IN (SELECT "namespaces"."id" FROM "namespaces" WHERE "namespaces"."type" = 'Group' AND "namespaces"."id" = 207) AND "members"."requested_at" IS NULL AND "members"."id" >= 61 /*application:test,correlation_id:612533058c613e0c894c4218348bb916,db_config_name:main,line:/ee/lib/ee/gitlab/auth/ldap/sync/group.rb:163:in `block in inherit_higher_access_levels'*/
   ee/lib/ee/gitlab/auth/ldap/sync/group.rb:163:in `block in inherit_higher_access_levels'
  Identity Load (0.2ms)  SELECT "identities".* FROM "identities" WHERE "identities"."provider" = 'ldapmain' AND (LOWER("identities"."extern_uid") IN (LOWER('uid=user1,ou=users,dc=example,dc=com'))) AND "identities"."user_id" = 78 /*application:test,correlation_id:612533058c613e0c894c4218348bb916,db_config_name:main,line:/ee/lib/ee/gitlab/auth/ldap/sync/group.rb:169:in `group_by'*/
  ↳ ee/lib/ee/gitlab/auth/ldap/sync/group.rb:169:in `group_by'

Multiple members

Before:

  GroupMember Load (2.2ms)  SELECT "members"."id", identities.extern_uid AS distinguished_name, "members"."access_level", "members"."source_id" FROM "members" INNER JOIN "users" ON "users"."id" = "members"."user_id" INNER JOIN "identities" ON "identities"."user_id" = "users"."id" WHERE "members"."type" = 'GroupMember' AND "members"."source_type" = 'Namespace' AND "members"."source_id" IN (SELECT "namespaces"."id" FROM "namespaces" WHERE "namespaces"."type" = 'Group' AND "namespaces"."id" = 198) AND "members"."requested_at" IS NULL AND "identities"."provider" = 'ldapmain' AND "users"."id" IN (SELECT "identities"."user_id" FROM "identities" WHERE (LOWER("identities"."extern_uid") IN (LOWER('uid=user1,ou=users,dc=example,dc=com'), LOWER('uid=user2,ou=users,dc=example,dc=com')))) /* allow_cross_joins_across_databases */ /*application:test,correlation_id:914f4ccb4ed481c280a3255867017aab,db_config_name:main,line:/ee/lib/ee/gitlab/auth/ldap/sync/group.rb:168:in `inherit_higher_access_levels'*/
   ee/lib/ee/gitlab/auth/ldap/sync/group.rb:168:in `inherit_higher_access_levels'

After:

  GroupMember Load (1.4ms)  SELECT "members"."id" FROM "members" WHERE "members"."type" = 'GroupMember' AND "members"."source_type" = 'Namespace' AND "members"."source_id" IN (SELECT "namespaces"."id" FROM "namespaces" WHERE "namespaces"."type" = 'Group' AND "namespaces"."id" = 203) AND "members"."requested_at" IS NULL ORDER BY "members"."id" ASC LIMIT 1 /*application:test,correlation_id:0fd2a8dbdcacb354e67c9d467915c664,db_config_name:main,line:/app/models/concerns/each_batch.rb:62:in `each_batch'*/
   app/models/concerns/each_batch.rb:62:in `each_batch'
  GroupMember Load (0.5ms)  SELECT "members"."id" FROM "members" WHERE "members"."type" = 'GroupMember' AND "members"."source_type" = 'Namespace' AND "members"."source_id" IN (SELECT "namespaces"."id" FROM "namespaces" WHERE "namespaces"."type" = 'Group' AND "namespaces"."id" = 203) AND "members"."requested_at" IS NULL AND "members"."id" >= 58 ORDER BY "members"."id" ASC LIMIT 1 OFFSET 1000 /*application:test,correlation_id:0fd2a8dbdcacb354e67c9d467915c664,db_config_name:main,line:/app/models/concerns/each_batch.rb:81:in `block in each_batch'*/
  ↳ app/models/concerns/each_batch.rb:81:in `block in each_batch'
  GroupMember Load (0.4ms)  SELECT "members".* FROM "members" WHERE "members"."type" = 'GroupMember' AND "members"."source_type" = 'Namespace' AND "members"."source_id" IN (SELECT "namespaces"."id" FROM "namespaces" WHERE "namespaces"."type" = 'Group' AND "namespaces"."id" = 203) AND "members"."requested_at" IS NULL AND "members"."id" >= 58 /*application:test,correlation_id:0fd2a8dbdcacb354e67c9d467915c664,db_config_name:main,line:/ee/lib/ee/gitlab/auth/ldap/sync/group.rb:163:in `block in inherit_higher_access_levels'*/
   ee/lib/ee/gitlab/auth/ldap/sync/group.rb:163:in `block in inherit_higher_access_levels'
  Identity Load (0.2ms)  SELECT "identities".* FROM "identities" WHERE "identities"."provider" = 'ldapmain' AND (LOWER("identities"."extern_uid") IN (LOWER('uid=user1,ou=users,dc=example,dc=com'), LOWER('uid=user2,ou=users,dc=example,dc=com'))) AND "identities"."user_id" = 76 /*application:test,correlation_id:0fd2a8dbdcacb354e67c9d467915c664,db_config_name:main,line:/ee/lib/ee/gitlab/auth/ldap/sync/group.rb:169:in `group_by'*/
  ↳ ee/lib/ee/gitlab/auth/ldap/sync/group.rb:169:in `group_by'

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

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 Thong Kuah

Merge request reports