Merge branch 'improve-calendar-json-query' into 'master'

Improve performance of user contribution graph query

See merge request gitlab-org/gitlab!74970

app/models/project.rb

@@ -576,18 +576,12 @@ class Project < ApplicationRecord
       .where('rs.path LIKE ?', "#{sanitize_sql_like(path)}/%")
   end
 
-  # "enabled" here means "not disabled". It includes private features!
   scope :with_feature_enabled, ->(feature) {
-    access_level_attribute = ProjectFeature.arel_table[ProjectFeature.access_level_attribute(feature)]
-    enabled_feature = access_level_attribute.gt(ProjectFeature::DISABLED).or(access_level_attribute.eq(nil))
-
-    with_project_feature.where(enabled_feature)
+    with_project_feature.merge(ProjectFeature.with_feature_enabled(feature))
   }
 
-  # Picks a feature where the level is exactly that given.
   scope :with_feature_access_level, ->(feature, level) {
-    access_level_attribute = ProjectFeature.access_level_attribute(feature)
-    with_project_feature.where(project_features: { access_level_attribute => level })
+    with_project_feature.merge(ProjectFeature.with_feature_access_level(feature, level))
   }
 
   # Picks projects which use the given programming language
@@ -688,37 +682,8 @@ class Project < ApplicationRecord
     end
   end
 
-  # project features may be "disabled", "internal", "enabled" or "public". If "internal",
-  # they are only available to team members. This scope returns projects where
-  # the feature is either public, enabled, or internal with permission for the user.
-  # Note: this scope doesn't enforce that the user has access to the projects, it just checks
-  # that the user has access to the feature. It's important to use this scope with others
-  # that checks project authorizations first (e.g. `filter_by_feature_visibility`).
-  #
-  # This method uses an optimised version of `with_feature_access_level` for
-  # logged in users to more efficiently get private projects with the given
-  # feature.
   def self.with_feature_available_for_user(feature, user)
-    visible = [ProjectFeature::ENABLED, ProjectFeature::PUBLIC]
-
-    if user&.can_read_all_resources?
-      with_feature_enabled(feature)
-    elsif user
-      min_access_level = ProjectFeature.required_minimum_access_level(feature)
-      column = ProjectFeature.quoted_access_level_column(feature)
-
-      with_project_feature
-      .where("#{column} IS NULL OR #{column} IN (:public_visible) OR (#{column} = :private_visible AND EXISTS (:authorizations))",
-            {
-              public_visible: visible,
-              private_visible: ProjectFeature::PRIVATE,
-              authorizations: user.authorizations_for_projects(min_access_level: min_access_level)
-            })
-    else
-      # This has to be added to include features whose value is nil in the db
-      visible << nil
-      with_feature_access_level(feature, visible)
-    end
+    with_project_feature.merge(ProjectFeature.with_feature_available_for_user(feature, user))
   end
 
   def self.projects_user_can(projects, user, action)

app/models/project_feature.rb

@@ -83,6 +83,52 @@ class ProjectFeature < ApplicationRecord
     end
   end
 
+  # "enabled" here means "not disabled". It includes private features!
+  scope :with_feature_enabled, ->(feature) {
+    feature_access_level_attribute = arel_table[access_level_attribute(feature)]
+    enabled_feature = feature_access_level_attribute.gt(DISABLED).or(feature_access_level_attribute.eq(nil))
+
+    where(enabled_feature)
+  }
+
+  # Picks a feature where the level is exactly that given.
+  scope :with_feature_access_level, ->(feature, level) {
+    feature_access_level_attribute = access_level_attribute(feature)
+    where(project_features: { feature_access_level_attribute => level })
+  }
+
+  # project features may be "disabled", "internal", "enabled" or "public". If "internal",
+  # they are only available to team members. This scope returns features where
+  # the feature is either public, enabled, or internal with permission for the user.
+  # Note: this scope doesn't enforce that the user has access to the projects, it just checks
+  # that the user has access to the feature. It's important to use this scope with others
+  # that checks project authorizations first (e.g. `filter_by_feature_visibility`).
+  #
+  # This method uses an optimised version of `with_feature_access_level` for
+  # logged in users to more efficiently get private projects with the given
+  # feature.
+  def self.with_feature_available_for_user(feature, user)
+    visible = [ENABLED, PUBLIC]
+
+    if user&.can_read_all_resources?
+      with_feature_enabled(feature)
+    elsif user
+      min_access_level = required_minimum_access_level(feature)
+      column = quoted_access_level_column(feature)
+
+      where("#{column} IS NULL OR #{column} IN (:public_visible) OR (#{column} = :private_visible AND EXISTS (:authorizations))",
+           {
+             public_visible: visible,
+             private_visible: PRIVATE,
+             authorizations: user.authorizations_for_projects(min_access_level: min_access_level, related_project_column: 'project_features.project_id')
+           })
+    else
+      # This has to be added to include features whose value is nil in the db
+      visible << nil
+      with_feature_access_level(feature, visible)
+    end
+  end
+
   def public_pages?
     return true unless Gitlab.config.pages.access_control
 

db/post_migrate/20211122033501_improve_index_on_events_for_calendar.rb

+# frozen_string_literal: true
+
+class ImproveIndexOnEventsForCalendar < Gitlab::Database::Migration[1.0]
+  INDEX_NAME = 'index_events_author_id_project_id_action_target_type_created_at'
+
+  def up
+    prepare_async_index :events, [:author_id, :project_id, :action, :target_type, :created_at], name: INDEX_NAME
+  end
+
+  def down
+    unprepare_async_index :events, [:author_id, :project_id, :action, :target_type, :created_at], name: INDEX_NAME
+  end
+end

db/schema_migrations/20211122033501

+e010b4c12ae8203d9ea8a4c2035be5e7165aba0030f4d5fd0b0f978f84748707
\ No newline at end of file

lib/gitlab/contributions_calendar.rb

@@ -23,25 +23,28 @@ module Gitlab
     def activity_dates
       return @activity_dates if @activity_dates.present?
 
+      date_interval = "INTERVAL '#{@contributor_time_instance.now.utc_offset} seconds'"
+
       # Can't use Event.contributions here because we need to check 3 different
       # project_features for the (currently) 3 different contribution types
       date_from = @contributor_time_instance.now.years_ago(1)
-      repo_events = event_counts(date_from, :repository)
-        .having(action: :pushed)
-      issue_events = event_counts(date_from, :issues)
-        .having(action: [:created, :closed], target_type: "Issue")
-      mr_events = event_counts(date_from, :merge_requests)
-        .having(action: [:merged, :created, :closed], target_type: "MergeRequest")
-      note_events = event_counts(date_from, :merge_requests)
-        .having(action: :commented)
+      repo_events = event_created_at(date_from, :repository)
+        .where(action: :pushed, target_type: nil)
+      issue_events = event_created_at(date_from, :issues)
+        .where(action: [:created, :closed], target_type: "Issue")
+      mr_events = event_created_at(date_from, :merge_requests)
+        .where(action: [:merged, :created, :closed], target_type: "MergeRequest")
+      note_events = event_created_at(date_from, :merge_requests)
+        .where(action: :commented, target_type: "Note")
 
       events = Event
-        .select(:project_id, :target_type, :action, :date, :total_amount)
-        .from_union([repo_events, issue_events, mr_events, note_events])
+        .select("date(created_at + #{date_interval}) AS date", 'COUNT(*) AS num_events')
+        .from_union([repo_events, issue_events, mr_events, note_events], remove_duplicates: false)
+        .group(:date)
         .map(&:attributes)
 
       @activity_dates = events.each_with_object(Hash.new {|h, k| h[k] = 0 }) do |event, activities|
-        activities[event["date"]] += event["total_amount"]
+        activities[event["date"]] += event["num_events"]
       end
     end
     # rubocop: enable CodeReuse/ActiveRecord
@@ -74,27 +77,25 @@ module Gitlab
     end
 
     # rubocop: disable CodeReuse/ActiveRecord
-    def event_counts(date_from, feature)
+    def event_created_at(date_from, feature)
       t = Event.arel_table
 
       # re-running the contributed projects query in each union is expensive, so
       # use IN(project_ids...) instead. It's the intersection of two users so
       # the list will be (relatively) short
       @contributed_project_ids ||= projects.distinct.pluck(:id)
-      authed_projects = Project.where(id: @contributed_project_ids)
+      authed_projects = ProjectFeature
         .with_feature_available_for_user(feature, current_user)
+        .where(project_id: @contributed_project_ids)
         .reorder(nil)
-        .select(:id)
+        .select(:project_id)
 
       conditions = t[:created_at].gteq(date_from.beginning_of_day)
         .and(t[:created_at].lteq(@contributor_time_instance.today.end_of_day))
         .and(t[:author_id].eq(contributor.id))
 
-      date_interval = "INTERVAL '#{@contributor_time_instance.now.utc_offset} seconds'"
-
       Event.reorder(nil)
-        .select(t[:project_id], t[:target_type], t[:action], "date(created_at + #{date_interval}) AS date", 'count(id) as total_amount')
-        .group(t[:project_id], t[:target_type], t[:action], "date(created_at + #{date_interval})")
+        .select(:created_at)
         .where(conditions)
         .where("events.project_id in (#{authed_projects.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
     end