Commit 9fa56075 authored by drew cimino's avatar drew cimino

Remove backlog work from DestroyAllExpiredService

parent 3dc0c804
......@@ -271,10 +271,6 @@ module Ci
self.where(project: project).sum(:size)
end
def self.distinct_job_ids
distinct.pluck(:job_id)
end
##
# FastDestroyAll concerns
# rubocop: disable CodeReuse/ServiceClass
......
......@@ -42,30 +42,9 @@ module Ci
artifacts = Ci::JobArtifact.expired_before(@start_at).artifact_unlocked.limit(BATCH_SIZE)
service_response = destroy_batch(artifacts)
@removed_artifacts_count += service_response[:destroyed_artifacts_count]
update_locked_status_on_unknown_artifacts if service_response[:destroyed_artifacts_count] == 0
# Return a truthy value here to prevent exiting #loop_until
@removed_artifacts_count
end
end
def update_locked_status_on_unknown_artifacts
build_ids = Ci::JobArtifact.expired_before(@start_at).artifact_unknown.limit(BATCH_SIZE).distinct_job_ids
return unless build_ids.present?
locked_pipeline_build_ids = ::Ci::Build.with_pipeline_locked_artifacts.id_in(build_ids).pluck_primary_key
unlocked_pipeline_build_ids = build_ids - locked_pipeline_build_ids
update_unknown_artifacts(locked_pipeline_build_ids, Ci::JobArtifact.lockeds[:artifacts_locked])
update_unknown_artifacts(unlocked_pipeline_build_ids, Ci::JobArtifact.lockeds[:unlocked])
end
def update_unknown_artifacts(build_ids, locked_value)
Ci::JobArtifact.for_job_ids(build_ids).update_all(locked: locked_value) if build_ids.any?
end
def destroy_job_artifacts_with_slow_iteration
Ci::JobArtifact.expired_before(@start_at).each_batch(of: BATCH_SIZE, column: :expire_at, order: :desc) do |relation, index|
# For performance reasons, join with ci_pipelines after the batch is queried.
......
......@@ -13,8 +13,8 @@ class ExpireBuildArtifactsWorker # rubocop:disable Scalability/IdempotentWorker
feature_category :build_artifacts
def perform
service = Ci::JobArtifacts::DestroyAllExpiredService.new
artifacts_count = service.execute
artifacts_count = Ci::JobArtifacts::DestroyAllExpiredService.new.execute
log_extra_metadata_on_done(:destroyed_job_artifacts_count, artifacts_count)
end
end
......@@ -53,46 +53,6 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s
log = ActiveRecord::QueryRecorder.new { subject }
expect(log.count).to be_within(1).of(8)
end
context 'with several locked-unknown artifact records' do
before do
stub_const("#{described_class}::LOOP_LIMIT", 10)
stub_const("#{described_class}::BATCH_SIZE", 2)
end
let!(:lockable_artifact_records) do
[
create(:ci_job_artifact, :metadata, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: locked_job),
create(:ci_job_artifact, :junit, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: locked_job),
create(:ci_job_artifact, :sast, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: locked_job),
create(:ci_job_artifact, :cobertura, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: locked_job),
create(:ci_job_artifact, :trace, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: locked_job)
]
end
let!(:unlockable_artifact_records) do
[
create(:ci_job_artifact, :metadata, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: job),
create(:ci_job_artifact, :junit, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: job),
create(:ci_job_artifact, :sast, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: job),
create(:ci_job_artifact, :cobertura, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: job),
create(:ci_job_artifact, :trace, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: job),
artifact
]
end
it 'updates the locked status of job artifacts from artifacts-locked pipelines' do
subject
expect(lockable_artifact_records).to be_all(&:persisted?)
expect(lockable_artifact_records).to be_all { |artifact| artifact.reload.artifact_artifacts_locked? }
end
it 'unlocks and then destroys job artifacts from artifacts-unlocked pipelines' do
expect { subject }.to change { Ci::JobArtifact.count }.by(-6)
expect(Ci::JobArtifact.where(id: unlockable_artifact_records.map(&:id))).to be_empty
end
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment