Merge branch 'mo-add-destroy-artifact-service' into 'master'

Remove expired pipeline artifacts

See merge request gitlab-org/gitlab!39882

app/models/ci/job_artifact.rb

@@ -161,7 +161,6 @@ module Ci
       where(file_type: types)
     end
 
-    scope :expired, -> (limit) { where('expire_at < ?', Time.current).limit(limit) }
     scope :downloadable, -> { where(file_type: DOWNLOADABLE_TYPES) }
     scope :unlocked, -> { joins(job: :pipeline).merge(::Ci::Pipeline.unlocked).order(expire_at: :desc) }
 

app/models/concerns/ci/artifactable.rb

@@ -17,6 +17,8 @@ module Ci
         zip: 2,
         gzip: 3
       }, _suffix: true
+
+      scope :expired, -> (limit) { where('expire_at < ?', Time.current).limit(limit) }
     end
 
     def each_blob(&blk)

app/services/ci/destroy_expired_job_artifacts_service.rb

@@ -20,18 +20,18 @@ module Ci
     def execute
       in_lock(EXCLUSIVE_LOCK_KEY, ttl: LOCK_TIMEOUT, retries: 1) do
         loop_until(timeout: LOOP_TIMEOUT, limit: LOOP_LIMIT) do
-          destroy_batch
+          destroy_batch(Ci::JobArtifact) || destroy_batch(Ci::PipelineArtifact)
         end
       end
     end
 
     private
 
-    def destroy_batch
-      artifact_batch = if Gitlab::Ci::Features.destroy_only_unlocked_expired_artifacts_enabled?
-                         Ci::JobArtifact.expired(BATCH_SIZE).unlocked
+    def destroy_batch(klass)
+      artifact_batch = if klass == Ci::JobArtifact && Gitlab::Ci::Features.destroy_only_unlocked_expired_artifacts_enabled?
+                         klass.expired(BATCH_SIZE).unlocked
                        else
-                         Ci::JobArtifact.expired(BATCH_SIZE)
+                         klass.expired(BATCH_SIZE)
                        end
 
       artifacts = artifact_batch.to_a

changelogs/unreleased/mo-add-destroy-artifact-service.yml

+---
+title: Add index for expire_at to ci_pipeline_artifacts
+merge_request: 39882
+author:
+type: added

db/migrate/20200827150057_add_index_expire_at_to_pipeline_artifacts.rb

+# frozen_string_literal: true
+
+class AddIndexExpireAtToPipelineArtifacts < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'index_ci_pipeline_artifacts_on_expire_at'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :ci_pipeline_artifacts, :expire_at, name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name(:ci_pipeline_artifacts, INDEX_NAME)
+  end
+end

db/schema_migrations/20200827150057

+85b7ffba53c9cec30e9778dd806277ca8e9877c9a18dc1d6004402c0e66b8ef1
\ No newline at end of file

db/structure.sql

@@ -19329,6 +19329,8 @@ CREATE INDEX index_ci_job_variables_on_job_id ON public.ci_job_variables USING b
 
 CREATE UNIQUE INDEX index_ci_job_variables_on_key_and_job_id ON public.ci_job_variables USING btree (key, job_id);
 
+CREATE INDEX index_ci_pipeline_artifacts_on_expire_at ON public.ci_pipeline_artifacts USING btree (expire_at);
+
 CREATE INDEX index_ci_pipeline_artifacts_on_pipeline_id ON public.ci_pipeline_artifacts USING btree (pipeline_id);
 
 CREATE UNIQUE INDEX index_ci_pipeline_artifacts_on_pipeline_id_and_file_type ON public.ci_pipeline_artifacts USING btree (pipeline_id, file_type);

spec/services/ci/destroy_expired_job_artifacts_service_spec.rb

@@ -120,5 +120,25 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
         expect { subject }.to change { Ci::JobArtifact.count }.by(-2)
       end
     end
+
+    context 'when artifact is a pipeline artifact' do
+      context 'when artifacts are expired' do
+        let!(:pipeline_artifact_1) { create(:ci_pipeline_artifact, expire_at: 1.week.ago) }
+        let!(:pipeline_artifact_2) { create(:ci_pipeline_artifact, expire_at: 1.week.ago) }
+
+        it 'destroys pipeline artifacts' do
+          expect { subject }.to change { Ci::PipelineArtifact.count }.by(-2)
+        end
+      end
+
+      context 'when artifacts are not expired' do
+        let!(:pipeline_artifact_1) { create(:ci_pipeline_artifact, expire_at: 2.days) }
+        let!(:pipeline_artifact_2) { create(:ci_pipeline_artifact, expire_at: 2.days) }
+
+        it 'do not destroy pipeline artifacts' do
+          expect { subject }.not_to change { Ci::PipelineArtifact.count }
+        end
+      end
+    end
   end
 end