Merge branch 'avoid-race-condition-of-archive-trace-cron-worker' into 'master'
Avoid conflicts between ArchiveTracesCronWorker and ArchiveTraceWorker See merge request gitlab-org/gitlab-ce!31376
This commit is contained in:
commit
4e4f8534fd
5 changed files with 70 additions and 5 deletions
|
@ -121,6 +121,8 @@ module Ci
|
||||||
scope :scheduled_actions, ->() { where(when: :delayed, status: COMPLETED_STATUSES + %i[scheduled]) }
|
scope :scheduled_actions, ->() { where(when: :delayed, status: COMPLETED_STATUSES + %i[scheduled]) }
|
||||||
scope :ref_protected, -> { where(protected: true) }
|
scope :ref_protected, -> { where(protected: true) }
|
||||||
scope :with_live_trace, -> { where('EXISTS (?)', Ci::BuildTraceChunk.where('ci_builds.id = ci_build_trace_chunks.build_id').select(1)) }
|
scope :with_live_trace, -> { where('EXISTS (?)', Ci::BuildTraceChunk.where('ci_builds.id = ci_build_trace_chunks.build_id').select(1)) }
|
||||||
|
scope :with_stale_live_trace, -> { with_live_trace.finished_before(12.hours.ago) }
|
||||||
|
scope :finished_before, -> (date) { finished.where('finished_at < ?', date) }
|
||||||
|
|
||||||
scope :matches_tag_ids, -> (tag_ids) do
|
scope :matches_tag_ids, -> (tag_ids) do
|
||||||
matcher = ::ActsAsTaggableOn::Tagging
|
matcher = ::ActsAsTaggableOn::Tagging
|
||||||
|
|
|
@ -10,7 +10,7 @@ module Ci
|
||||||
# Archive stale live traces which still resides in redis or database
|
# Archive stale live traces which still resides in redis or database
|
||||||
# This could happen when ArchiveTraceWorker sidekiq jobs were lost by receiving SIGKILL
|
# This could happen when ArchiveTraceWorker sidekiq jobs were lost by receiving SIGKILL
|
||||||
# More details in https://gitlab.com/gitlab-org/gitlab-ce/issues/36791
|
# More details in https://gitlab.com/gitlab-org/gitlab-ce/issues/36791
|
||||||
Ci::Build.finished.with_live_trace.find_each(batch_size: 100) do |build|
|
Ci::Build.with_stale_live_trace.find_each(batch_size: 100) do |build|
|
||||||
Ci::ArchiveTraceService.new.execute(build, worker_name: self.class.name)
|
Ci::ArchiveTraceService.new.execute(build, worker_name: self.class.name)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Avoid conflicts between ArchiveTracesCronWorker and ArchiveTraceWorker
|
||||||
|
merge_request: 31376
|
||||||
|
author:
|
||||||
|
type: fixed
|
|
@ -149,6 +149,56 @@ describe Ci::Build do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '.with_stale_live_trace' do
|
||||||
|
subject { described_class.with_stale_live_trace }
|
||||||
|
|
||||||
|
context 'when build has a stale live trace' do
|
||||||
|
let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 1.day.ago) }
|
||||||
|
|
||||||
|
it 'selects the build' do
|
||||||
|
is_expected.to eq([build])
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when build does not have a stale live trace' do
|
||||||
|
let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 1.hour.ago) }
|
||||||
|
|
||||||
|
it 'does not select the build' do
|
||||||
|
is_expected.to be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '.finished_before' do
|
||||||
|
subject { described_class.finished_before(date) }
|
||||||
|
|
||||||
|
let(:date) { 1.hour.ago }
|
||||||
|
|
||||||
|
context 'when build has finished one day ago' do
|
||||||
|
let!(:build) { create(:ci_build, :success, finished_at: 1.day.ago) }
|
||||||
|
|
||||||
|
it 'selects the build' do
|
||||||
|
is_expected.to eq([build])
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when build has finished 30 minutes ago' do
|
||||||
|
let!(:build) { create(:ci_build, :success, finished_at: 30.minutes.ago) }
|
||||||
|
|
||||||
|
it 'returns an empty array' do
|
||||||
|
is_expected.to be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when build is still running' do
|
||||||
|
let!(:build) { create(:ci_build, :running) }
|
||||||
|
|
||||||
|
it 'returns an empty array' do
|
||||||
|
is_expected.to be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe '.with_reports' do
|
describe '.with_reports' do
|
||||||
subject { described_class.with_reports(Ci::JobArtifact.test_reports) }
|
subject { described_class.with_reports(Ci::JobArtifact.test_reports) }
|
||||||
|
|
||||||
|
|
|
@ -5,6 +5,8 @@ require 'spec_helper'
|
||||||
describe Ci::ArchiveTracesCronWorker do
|
describe Ci::ArchiveTracesCronWorker do
|
||||||
subject { described_class.new.perform }
|
subject { described_class.new.perform }
|
||||||
|
|
||||||
|
let(:finished_at) { 1.day.ago }
|
||||||
|
|
||||||
before do
|
before do
|
||||||
stub_feature_flags(ci_enable_live_trace: true)
|
stub_feature_flags(ci_enable_live_trace: true)
|
||||||
end
|
end
|
||||||
|
@ -28,7 +30,7 @@ describe Ci::ArchiveTracesCronWorker do
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when a job succeeded' do
|
context 'when a job succeeded' do
|
||||||
let!(:build) { create(:ci_build, :success, :trace_live) }
|
let!(:build) { create(:ci_build, :success, :trace_live, finished_at: finished_at) }
|
||||||
|
|
||||||
it_behaves_like 'archives trace'
|
it_behaves_like 'archives trace'
|
||||||
|
|
||||||
|
@ -39,9 +41,15 @@ describe Ci::ArchiveTracesCronWorker do
|
||||||
subject
|
subject
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'when the job finished recently' do
|
||||||
|
let(:finished_at) { 1.hour.ago }
|
||||||
|
|
||||||
|
it_behaves_like 'does not archive trace'
|
||||||
|
end
|
||||||
|
|
||||||
context 'when a trace had already been archived' do
|
context 'when a trace had already been archived' do
|
||||||
let!(:build) { create(:ci_build, :success, :trace_live, :trace_artifact) }
|
let!(:build) { create(:ci_build, :success, :trace_live, :trace_artifact) }
|
||||||
let!(:build2) { create(:ci_build, :success, :trace_live) }
|
let!(:build2) { create(:ci_build, :success, :trace_live, finished_at: finished_at) }
|
||||||
|
|
||||||
it 'continues to archive live traces' do
|
it 'continues to archive live traces' do
|
||||||
subject
|
subject
|
||||||
|
@ -52,7 +60,7 @@ describe Ci::ArchiveTracesCronWorker do
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when an unexpected exception happened during archiving' do
|
context 'when an unexpected exception happened during archiving' do
|
||||||
let!(:build) { create(:ci_build, :success, :trace_live) }
|
let!(:build) { create(:ci_build, :success, :trace_live, finished_at: finished_at) }
|
||||||
|
|
||||||
before do
|
before do
|
||||||
allow(Gitlab::Sentry).to receive(:track_exception)
|
allow(Gitlab::Sentry).to receive(:track_exception)
|
||||||
|
@ -71,7 +79,7 @@ describe Ci::ArchiveTracesCronWorker do
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when a job was cancelled' do
|
context 'when a job was cancelled' do
|
||||||
let!(:build) { create(:ci_build, :canceled, :trace_live) }
|
let!(:build) { create(:ci_build, :canceled, :trace_live, finished_at: finished_at) }
|
||||||
|
|
||||||
it_behaves_like 'archives trace'
|
it_behaves_like 'archives trace'
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue