From 03b020f2e4a4e48dc4ddceb1656b84aaa7938149 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Wed, 21 Mar 2018 10:03:50 +1100 Subject: 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. --- app/models/ci/job_artifact.rb | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) (limited to 'app/models/ci/job_artifact.rb') 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 -- cgit v1.2.1 From a28f25b565c58088d80545ebe483f8cd25c22c10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Fri, 13 Apr 2018 10:20:07 +0200 Subject: Fix direct_upload when records with null file_store are used Old records have a null value of file_store column. This causes the problems with current direct_upload implementation, as this makes it to choose Store::REMOTE instead of Store::LOCAL. This change moves the store save when change saving the object. --- app/models/ci/job_artifact.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'app/models/ci/job_artifact.rb') diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index f846482cdeb..39676efa08c 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -7,14 +7,15 @@ module Ci belongs_to :project belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id - before_save :update_file_store + mount_uploader :file, JobArtifactUploader + 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]) } + after_save :update_file_store - mount_uploader :file, JobArtifactUploader + scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) } delegate :exists?, :open, to: :file @@ -25,7 +26,9 @@ module Ci } def update_file_store - self.file_store = file.object_store + # The file.object_store is set during `uploader.store!` + # which happens after object is inserted/updated + self.update_column(:file_store, file.object_store) end def self.artifacts_size_for(project) -- cgit v1.2.1