More N+1s in calculating notification recipients
@stanhu found these in https://gitlab.com/gitlab-org/gitlab-ce/issues/45534#note_73318286
Subscribable#subscribers
SQL:
SELECT "subscriptions".* FROM "subscriptions" WHERE "subscriptions"."subscribable_id" = $1 AND "subscriptions"."subscribable_type" = $2 AND ("subscriptions"."project_id" IS NULL OR "subscriptions"."project_id" = 1) AND "subscriptions"."subscribed" = $3
--> /Users/stanhu/gdk/gitlab/app/models/concerns/subscribable.rb:34:in `subscribers' --> /Users/stanhu/gdk/gitlab/app/services/notification_recipient_service.rb:161:in `add_subscribed_users' --> /Users/stanhu/gdk/gitlab/app/services/notification_recipient_service.rb:322:in `build!' --> /Users/stanhu/gdk/gitlab/app/services/notification_recipient_service.rb:84:in `notification_recipients' --> /Users/stanhu/gdk/gitlab/app/services/notification_recipient_service.rb:18:in `build_new_note_recipients'
NotificationRecipient#has_access? (team_member?)
SQL:
SELECT 1 AS one FROM "users" INNER JOIN "project_authorizations" ON "users"."id" = "project_authorizations"."user_id" WHERE "project_authorizations"."project_id" = $1 AND "users"."id" = $2 LIMIT 1
--> /Users/stanhu/gdk/gitlab/app/policies/project_policy.rb:386:in `team_member?' --> /Users/stanhu/gdk/gitlab/app/policies/project_policy.rb:40:in `block in <class:ProjectPolicy>' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/condition.rb:21:in `instance_eval' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/condition.rb:21:in `compute' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/condition.rb:42:in `block in pass?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/base.rb:280:in `cache' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/condition.rb:42:in `pass?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/rule.rb:79:in `pass?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/step.rb:79:in `pass?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/runner.rb:89:in `block in run' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/runner.rb:177:in `block in steps_by_score' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/runner.rb:146:in `loop' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/runner.rb:146:in `steps_by_score' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/runner.rb:79:in `run' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/runner.rb:57:in `pass?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/base.rb:215:in `block in allowed?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/base.rb:215:in `each' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/base.rb:215:in `all?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/base.rb:215:in `allowed?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/base.rb:207:in `can?' --> /Users/stanhu/gdk/gitlab/app/models/ability.rb:70:in `allowed?' --> /Users/stanhu/gdk/gitlab/app/models/user.rb:694:in `can?' --> /Users/stanhu/gdk/gitlab/app/models/notification_recipient.rb:95:in `block in has_access?'
NotificationRecipient#has_access? (Issue#assignee_or_author?)
SQL:
SELECT 1 AS one FROM "users" INNER JOIN "issue_assignees" ON "users"."id" = "issue_assignees"."user_id" WHERE "issue_assignees"."issue_id" = $1 AND "users"."id" = $2 LIMIT 1
--> /Users/stanhu/gdk/gitlab/app/models/issue.rb:150:in `assignee_or_author?' --> /Users/stanhu/gdk/gitlab/app/policies/issuable_policy.rb:9:in `block in <class:IssuablePolicy>' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/condition.rb:21:in `instance_eval' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/condition.rb:21:in `compute' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/condition.rb:42:in `block in pass?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/base.rb:280:in `cache' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/condition.rb:42:in `pass?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/rule.rb:79:in `pass?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/step.rb:79:in `pass?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/runner.rb:89:in `block in run' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/runner.rb:177:in `block in steps_by_score' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/runner.rb:146:in `loop' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/runner.rb:146:in `steps_by_score' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/runner.rb:79:in `run' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/runner.rb:57:in `pass?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/base.rb:215:in `block in allowed?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/base.rb:215:in `each' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/base.rb:215:in `all?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/base.rb:215:in `allowed?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/base.rb:207:in `can?' --> /Users/stanhu/gdk/gitlab/app/models/ability.rb:70:in `allowed?' --> /Users/stanhu/gdk/gitlab/app/models/user.rb:694:in `can?' --> /Users/stanhu/gdk/gitlab/app/models/notification_recipient.rb:95:in `block in has_access?' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/preferred_scope.rb:7:in `with_preferred_scope' --> /Users/stanhu/gdk/gitlab/lib/declarative_policy/preferred_scope.rb:21:in `subject_scope' --> /Users/stanhu/gdk/gitlab/app/models/notification_recipient.rb:85:in `has_access?' --> /Users/stanhu/gdk/gitlab/app/models/notification_recipient.rb:28:in `notifiable?'
We can extend spec/services/notification_recipient_service_spec.rb
for these cases and squash them too. A simple test case is this on a production console:
# paste the contents of https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/support/helpers/query_recorder.rb
merge_request = MergeRequest.find(8845120)
user = User.find_by_username('smcgivern')
record = ActiveRecord::QueryRecorder.new { NotificationRecipientService.build_recipients(merge_request, user, action: "resolve_all_discussions") }
record.count
record.log
Edited by Sean McGivern