Merge branch 'upsert_mr_metrics' into 'master'

Use upsert for MR metrics

See merge request gitlab-org/gitlab!69240

app/models/merge_request.rb

@@ -1853,25 +1853,29 @@ class MergeRequest < ApplicationRecord
 
   override :ensure_metrics
   def ensure_metrics
-    # Backward compatibility: some merge request metrics records will not have target_project_id filled in.
-    # In that case the first `safe_find_or_create_by` will return false.
-    # The second finder call will be eliminated in https://gitlab.com/gitlab-org/gitlab/-/issues/233507
-    metrics_record = MergeRequest::Metrics.safe_find_or_create_by(merge_request_id: id, target_project_id: target_project_id) || MergeRequest::Metrics.safe_find_or_create_by(merge_request_id: id)
-
-    metrics_record.tap do |metrics_record|
-      # Make sure we refresh the loaded association object with the newly created/loaded item.
-      # This is needed in order to have the exact functionality than before.
-      #
-      # Example:
-      #
-      # merge_request.metrics.destroy
-      # merge_request.ensure_metrics
-      # merge_request.metrics # should return the metrics record and not nil
-      # merge_request.metrics.merge_request # should return the same MR record
-
-      metrics_record.target_project_id = target_project_id
-      metrics_record.association(:merge_request).target = self
-      association(:metrics).target = metrics_record
+    if Feature.enabled?(:use_upsert_query_for_mr_metrics)
+      MergeRequest::Metrics.record!(self)
+    else
+      # Backward compatibility: some merge request metrics records will not have target_project_id filled in.
+      # In that case the first `safe_find_or_create_by` will return false.
+      # The second finder call will be eliminated in https://gitlab.com/gitlab-org/gitlab/-/issues/233507
+      metrics_record = MergeRequest::Metrics.safe_find_or_create_by(merge_request_id: id, target_project_id: target_project_id) || MergeRequest::Metrics.safe_find_or_create_by(merge_request_id: id)
+
+      metrics_record.tap do |metrics_record|
+        # Make sure we refresh the loaded association object with the newly created/loaded item.
+        # This is needed in order to have the exact functionality than before.
+        #
+        # Example:
+        #
+        # merge_request.metrics.destroy
+        # merge_request.ensure_metrics
+        # merge_request.metrics # should return the metrics record and not nil
+        # merge_request.metrics.merge_request # should return the same MR record
+
+        metrics_record.target_project_id = target_project_id
+        metrics_record.association(:merge_request).target = self
+        association(:metrics).target = metrics_record
+      end
     end
   end
 

app/models/merge_request/metrics.rb

@@ -14,8 +14,23 @@ class MergeRequest::Metrics < ApplicationRecord
   scope :with_valid_time_to_merge, -> { where(arel_table[:merged_at].gt(arel_table[:created_at])) }
   scope :by_target_project, ->(project) { where(target_project_id: project) }
 
-  def self.time_to_merge_expression
-    Arel.sql('EXTRACT(epoch FROM SUM(AGE(merge_request_metrics.merged_at, merge_request_metrics.created_at)))')
+  class << self
+    def time_to_merge_expression
+      Arel.sql('EXTRACT(epoch FROM SUM(AGE(merge_request_metrics.merged_at, merge_request_metrics.created_at)))')
+    end
+
+    def record!(mr)
+      sql = <<~SQL
+        INSERT INTO #{self.table_name} (merge_request_id, target_project_id, updated_at, created_at)
+        VALUES (#{mr.id}, #{mr.target_project_id}, NOW(), NOW())
+        ON CONFLICT (merge_request_id)
+        DO UPDATE SET
+        target_project_id = EXCLUDED.target_project_id,
+        updated_at = NOW()
+      SQL
+
+      connection.execute(sql)
+    end
   end
 
   private

config/feature_flags/development/use_upsert_query_for_mr_metrics.yml

+---
+name: use_upsert_query_for_mr_metrics
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69240
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/339677
+milestone: '14.3'
+type: development
+group: group::optimize
+default_enabled: false

ee/lib/analytics/merge_request_metrics_refresh.rb

@@ -8,7 +8,9 @@ module Analytics
 
     def execute(force: false)
       merge_requests.each do |mr|
-        metrics = mr.ensure_metrics
+        mr.ensure_metrics
+        mr.reset # clear already loaded (nil) metrics association
+        metrics = mr.metrics
 
         next if !force && metric_already_present?(metrics)