diff options
author | Shinya Maeda <shinya@gitlab.com> | 2018-09-27 12:17:43 +0300 |
---|---|---|
committer | Alessio Caiazza <acaiazza@gitlab.com> | 2018-10-02 18:08:12 +0300 |
commit | 587560757faaedb6c61ef7aa77f11934cb76084b (patch) | |
tree | 4504e3cd7d5135ba2b65301268cdc6d8dff8134e | |
parent | 71fc37c9cf8aee5de3d4d43ec6ea97a56e34537d (diff) |
Fix StuckCiJobsWorker and added tests
-rw-r--r-- | app/models/commit_status.rb | 2 | ||||
-rw-r--r-- | app/workers/stuck_ci_jobs_worker.rb | 2 | ||||
-rw-r--r-- | spec/workers/stuck_ci_jobs_worker_spec.rb | 41 |
3 files changed, 43 insertions, 2 deletions
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 2fd3365098a..2020dd637a0 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -84,7 +84,7 @@ class CommitStatus < ActiveRecord::Base end event :drop do - transition [:created, :pending, :running] => :failed + transition [:created, :pending, :running, :scheduled] => :failed end event :success do diff --git a/app/workers/stuck_ci_jobs_worker.rb b/app/workers/stuck_ci_jobs_worker.rb index ab090b96c52..821ea75703f 100644 --- a/app/workers/stuck_ci_jobs_worker.rb +++ b/app/workers/stuck_ci_jobs_worker.rb @@ -68,7 +68,7 @@ class StuckCiJobsWorker # `ci_builds` table has a partial index on `id` with `scheduled_at <> NULL` condition. # Therefore this query's first step uses Index Search, and the following expensive # filter `scheduled_at < ?` will only perform on a small subset (max: 100 rows) - Ci::Build.include(EachBatch).where('scheduled_at <> NULL').each_batch(of: 100) do |relation| + Ci::Build.include(EachBatch).where('scheduled_at IS NOT NULL').each_batch(of: 100) do |relation| relation.where('scheduled_at < ?', BUILD_SCHEDULED_OUTDATED_TIMEOUT.ago).find_each do |build| drop_build(:outdated, build, :scheduled, BUILD_SCHEDULED_OUTDATED_TIMEOUT, :schedule_expired) end diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb index 856886e3df5..2f8ba4859d7 100644 --- a/spec/workers/stuck_ci_jobs_worker_spec.rb +++ b/spec/workers/stuck_ci_jobs_worker_spec.rb @@ -127,6 +127,47 @@ describe StuckCiJobsWorker do end end + describe 'drop_stale_scheduled_builds' do + let(:status) { 'scheduled' } + let(:updated_at) { } + + context 'when scheduled at 2 hours ago but it is not executed yet' do + let!(:job) { create(:ci_build, :scheduled, scheduled_at: 2.hours.ago) } + + it 'drops the stale scheduled build' do + expect(Ci::Build.scheduled.count).to eq(1) + expect(job).to be_scheduled + + worker.perform + job.reload + + expect(Ci::Build.scheduled.count).to eq(0) + expect(job).to be_failed + expect(job).to be_schedule_expired + end + end + + context 'when scheduled at 30 minutes ago but it is not executed yet' do + let!(:job) { create(:ci_build, :scheduled, scheduled_at: 30.minutes.ago) } + + it 'does not drop the stale scheduled build yet' do + expect(Ci::Build.scheduled.count).to eq(1) + expect(job).to be_scheduled + + worker.perform + + expect(Ci::Build.scheduled.count).to eq(1) + expect(job).to be_scheduled + end + end + + context 'when there are no stale scheduled builds' do + it 'does not drop the stale scheduled build yet' do + expect { worker.perform }.not_to raise_error + end + end + end + describe 'exclusive lease' do let(:status) { 'running' } let(:updated_at) { 2.days.ago } |