Commit b9a79988 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Merge branch 'eb-fix-nil-artifact-size-calculation' into 'master'

Count nil artifact size as zero when recalculating

See merge request gitlab-org/gitlab!84509
parents a1f4c5b3 ce604106
...@@ -12,7 +12,7 @@ module Projects ...@@ -12,7 +12,7 @@ module Projects
if batch.any? if batch.any?
# We are doing the sum in ruby because the query takes too long when done in SQL # We are doing the sum in ruby because the query takes too long when done in SQL
total_artifacts_size = batch.sum(&:size) total_artifacts_size = batch.sum { |artifact| artifact.size.to_i }
Projects::BuildArtifactsSizeRefresh.transaction do Projects::BuildArtifactsSizeRefresh.transaction do
# Mark the refresh ready for another worker to pick up and process the next batch # Mark the refresh ready for another worker to pick up and process the next batch
......
...@@ -10,7 +10,8 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl ...@@ -10,7 +10,8 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl
let_it_be(:artifact_1) { create(:ci_job_artifact, project: project, size: 1, created_at: 14.days.ago) } let_it_be(:artifact_1) { create(:ci_job_artifact, project: project, size: 1, created_at: 14.days.ago) }
let_it_be(:artifact_2) { create(:ci_job_artifact, project: project, size: 2, created_at: 13.days.ago) } let_it_be(:artifact_2) { create(:ci_job_artifact, project: project, size: 2, created_at: 13.days.ago) }
let_it_be(:artifact_3) { create(:ci_job_artifact, project: project, size: 5, created_at: 12.days.ago) } let_it_be(:artifact_3) { create(:ci_job_artifact, project: project, size: nil, created_at: 13.days.ago) }
let_it_be(:artifact_4) { create(:ci_job_artifact, project: project, size: 5, created_at: 12.days.ago) }
# This should not be included in the recalculation as it is created later than the refresh start time # This should not be included in the recalculation as it is created later than the refresh start time
let_it_be(:future_artifact) { create(:ci_job_artifact, project: project, size: 8, created_at: 2.days.from_now) } let_it_be(:future_artifact) { create(:ci_job_artifact, project: project, size: 8, created_at: 2.days.from_now) }
...@@ -33,7 +34,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl ...@@ -33,7 +34,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl
end end
before do before do
stub_const("#{described_class}::BATCH_SIZE", 2) stub_const("#{described_class}::BATCH_SIZE", 3)
stats = create(:project_statistics, project: project, build_artifacts_size: 120) stats = create(:project_statistics, project: project, build_artifacts_size: 120)
stats.increment_counter(:build_artifacts_size, 30) stats.increment_counter(:build_artifacts_size, 30)
...@@ -48,7 +49,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl ...@@ -48,7 +49,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl
end end
it 'updates the last_job_artifact_id to the ID of the last artifact from the batch' do it 'updates the last_job_artifact_id to the ID of the last artifact from the batch' do
expect { service.execute }.to change { refresh.reload.last_job_artifact_id.to_i }.to(artifact_2.id) expect { service.execute }.to change { refresh.reload.last_job_artifact_id.to_i }.to(artifact_3.id)
end end
it 'requeues the refresh job' do it 'requeues the refresh job' do
...@@ -62,7 +63,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl ...@@ -62,7 +63,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl
:project_build_artifacts_size_refresh, :project_build_artifacts_size_refresh,
:pending, :pending,
project: project, project: project,
last_job_artifact_id: artifact_2.id last_job_artifact_id: artifact_3.id
) )
end end
...@@ -73,7 +74,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl ...@@ -73,7 +74,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl
end end
it 'keeps the last_job_artifact_id unchanged' do it 'keeps the last_job_artifact_id unchanged' do
expect(refresh.reload.last_job_artifact_id).to eq(artifact_2.id) expect(refresh.reload.last_job_artifact_id).to eq(artifact_3.id)
end end
it 'keeps the state of the refresh record at running' do it 'keeps the state of the refresh record at running' do
...@@ -89,7 +90,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl ...@@ -89,7 +90,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl
project: project, project: project,
updated_at: 2.days.ago, updated_at: 2.days.ago,
refresh_started_at: now, refresh_started_at: now,
last_job_artifact_id: artifact_3.id last_job_artifact_id: artifact_4.id
) )
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