Update ProjectStatistics#build_artifacts_size synchronously without summing (#41059)

Previously we scheduled a worker to just some this but we were running
into performance issues when the build table was getting too large. So
now we've updated the code such that this column is updated immediately
and incremented/decremented by the correct amount whenever artifacts are
created or deleted.

We've also added the performance optimization that we do not update this
statistic if a project is deleted because it could result in many
updates for a project with many builds.

2 files changed, 39 insertions, 14 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 4aa65bf4273..79408f69083 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -20,7 +20,7 @@ module Ci
     has_one :last_deployment, -> { order('deployments.id DESC') }, as: :deployable, class_name: 'Deployment'
     has_many :trace_sections, class_name: 'Ci::BuildTraceSection'
 
-    has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+    has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, dependent: :destroy, inverse_of: :job # rubocop:disable Cop/ActiveRecordDependent
     has_one :job_artifacts_archive, -> { where(file_type: Ci::JobArtifact.file_types[:archive]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
     has_one :job_artifacts_metadata, -> { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
     has_one :job_artifacts_trace, -> { where(file_type: Ci::JobArtifact.file_types[:trace]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
@@ -95,8 +95,8 @@ module Ci
       run_after_commit { BuildHooksWorker.perform_async(build.id) }
     end
 
-    after_commit :update_project_statistics_after_save, on: [:create, :update]
-    after_commit :update_project_statistics, on: :destroy
+    after_save :update_project_statistics_after_save, if: :artifacts_size_changed?
+    after_destroy :update_project_statistics_after_destroy, unless: :project_destroyed?
 
     class << self
       # This is needed for url_for to work,
@@ -664,16 +664,20 @@ module Ci
       pipeline.config_processor.build_attributes(name)
     end
 
-    def update_project_statistics
-      return unless project
+    def update_project_statistics_after_save
+      update_project_statistics(read_attribute(:artifacts_size).to_i - artifacts_size_was.to_i)
+    end
 
-      ProjectCacheWorker.perform_async(project_id, [], [:build_artifacts_size])
+    def update_project_statistics_after_destroy
+      update_project_statistics(-artifacts_size)
     end
 
-    def update_project_statistics_after_save
-      if previous_changes.include?('artifacts_size')
-        update_project_statistics
-      end
+    def update_project_statistics(difference)
+      ProjectStatistics.increment_statistic(project_id, :build_artifacts_size, difference)
+    end
+
+    def project_destroyed?
+      project.pending_delete?
     end
   end
 end
diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb
index fbb95fe16df..f846482cdeb 100644
--- a/app/models/ci/job_artifact.rb
+++ b/app/models/ci/job_artifact.rb
@@ -9,6 +9,8 @@ module Ci
 
     before_save :update_file_store
     before_save :set_size, if: :file_changed?
+    after_save :update_project_statistics_after_save, if: :size_changed?
+    after_destroy :update_project_statistics_after_destroy, unless: :project_destroyed?
 
     scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) }
 
@@ -34,10 +36,6 @@ module Ci
       [nil, ::JobArtifactUploader::Store::LOCAL].include?(self.file_store)
     end
 
-    def set_size
-      self.size = file.size
-    end
-
     def expire_in
       expire_at - Time.now if expire_at
     end
@@ -48,5 +46,28 @@ module Ci
           ChronicDuration.parse(value)&.seconds&.from_now
         end
     end
+
+    private
+
+    def set_size
+      self.size = file.size
+    end
+
+    def update_project_statistics_after_save
+      update_project_statistics(size.to_i - size_was.to_i)
+    end
+
+    def update_project_statistics_after_destroy
+      update_project_statistics(-self.size)
+    end
+
+    def update_project_statistics(difference)
+      ProjectStatistics.increment_statistic(project_id, :build_artifacts_size, difference)
+    end
+
+    def project_destroyed?
+      # Use job.project to avoid extra DB query for project
+      job.project.pending_delete?
+    end
   end
 end