Make pipeline schedule worker resilientset-real-next-run-at-for-preventing-duplciate-pipeline-creations

Currently, pipeline schedule worker is unstable because it's
sometimes killed by excessive memory consumption.
In order to improve the performance, we add the following fixes:

1. next_run_at is always real_next_run, which means the value
   always takes into account of worker's cron schedule
1. Remove exlusive lock. This is already covered by real_next_run
   change.
1. Use RunPipelineScheduleWorker for avoiding memory killer.
   Memory consumption is spread to the multiple sidekiq worker.

2 files changed, 33 insertions, 56 deletions

diff --git a/spec/workers/pipeline_schedule_worker_spec.rb b/spec/workers/pipeline_schedule_worker_spec.rb
index 8c604b13297..9326db34209 100644
--- a/spec/workers/pipeline_schedule_worker_spec.rb
+++ b/spec/workers/pipeline_schedule_worker_spec.rb
@@ -41,16 +41,6 @@ describe PipelineScheduleWorker do
 
       it_behaves_like 'successful scheduling'
 
-      context 'when exclusive lease has already been taken by the other instance' do
-        before do
-          stub_exclusive_lease_taken(described_class::EXCLUSIVE_LOCK_KEY, timeout: described_class::LOCK_TIMEOUT)
-        end
-
-        it 'raises an error and does not start creating pipelines' do
-          expect { subject }.to raise_error(Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError)
-        end
-      end
-
       context 'when the latest commit contains [ci skip]' do
         before do
           allow_any_instance_of(Ci::Pipeline)
@@ -77,47 +67,19 @@ describe PipelineScheduleWorker do
         stub_ci_pipeline_yaml_file(YAML.dump(rspec: { variables: 'rspec' } ))
       end
 
-      it 'creates a failed pipeline with the reason' do
-        expect { subject }.to change { project.ci_pipelines.count }.by(1)
-        expect(Ci::Pipeline.last).to be_config_error
-        expect(Ci::Pipeline.last.yaml_errors).not_to be_nil
+      it 'does not creates a new pipeline' do
+        expect { subject }.not_to change { project.ci_pipelines.count }
       end
     end
   end
 
   context 'when the schedule is not runnable by the user' do
-    before do
-      expect(Gitlab::Sentry)
-        .to receive(:track_exception)
-        .with(Ci::CreatePipelineService::CreateError,
-              issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/41231',
-              extra: { schedule_id: pipeline_schedule.id } ).once
-    end
-
     it 'does not deactivate the schedule' do
       subject
 
       expect(pipeline_schedule.reload.active).to be_truthy
     end
 
-    it 'increments Prometheus counter' do
-      expect(Gitlab::Metrics)
-        .to receive(:counter)
-        .with(:pipeline_schedule_creation_failed_total, "Counter of failed attempts of pipeline schedule creation")
-        .and_call_original
-
-      subject
-    end
-
-    it 'logging a pipeline error' do
-      expect(Rails.logger)
-        .to receive(:error)
-        .with(a_string_matching("Insufficient permissions to create a new pipeline"))
-        .and_call_original
-
-      subject
-    end
-
     it 'does not create a pipeline' do
       expect { subject }.not_to change { project.ci_pipelines.count }
     end
@@ -131,21 +93,6 @@ describe PipelineScheduleWorker do
     before do
       stub_ci_pipeline_yaml_file(nil)
       project.add_maintainer(user)
-
-      expect(Gitlab::Sentry)
-        .to receive(:track_exception)
-        .with(Ci::CreatePipelineService::CreateError,
-              issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/41231',
-              extra: { schedule_id: pipeline_schedule.id } ).once
-    end
-
-    it 'logging a pipeline error' do
-      expect(Rails.logger)
-        .to receive(:error)
-        .with(a_string_matching("Missing .gitlab-ci.yml file"))
-        .and_call_original
-
-      subject
     end
 
     it 'does not create a pipeline' do
diff --git a/spec/workers/run_pipeline_schedule_worker_spec.rb b/spec/workers/run_pipeline_schedule_worker_spec.rb
index 690af22f4dc..7414470f8e7 100644
--- a/spec/workers/run_pipeline_schedule_worker_spec.rb
+++ b/spec/workers/run_pipeline_schedule_worker_spec.rb
@@ -32,7 +32,37 @@ describe RunPipelineScheduleWorker do
 
       it 'calls the Service' do
         expect(Ci::CreatePipelineService).to receive(:new).with(project, user, ref: pipeline_schedule.ref).and_return(create_pipeline_service)
-        expect(create_pipeline_service).to receive(:execute).with(:schedule, ignore_skip_ci: true, save_on_errors: false, schedule: pipeline_schedule)
+        expect(create_pipeline_service).to receive(:execute!).with(:schedule, ignore_skip_ci: true, save_on_errors: false, schedule: pipeline_schedule)
+
+        worker.perform(pipeline_schedule.id, user.id)
+      end
+    end
+
+    context 'when database statement timeout happens' do
+      before do
+        allow(Ci::CreatePipelineService).to receive(:new) { raise ActiveRecord::StatementInvalid }
+
+        expect(Gitlab::Sentry)
+          .to receive(:track_exception)
+          .with(ActiveRecord::StatementInvalid,
+                issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/41231',
+                extra: { schedule_id: pipeline_schedule.id } ).once
+      end
+
+      it 'increments Prometheus counter' do
+        expect(Gitlab::Metrics)
+          .to receive(:counter)
+          .with(:pipeline_schedule_creation_failed_total, "Counter of failed attempts of pipeline schedule creation")
+          .and_call_original
+
+        worker.perform(pipeline_schedule.id, user.id)
+      end
+
+      it 'logging a pipeline error' do
+        expect(Rails.logger)
+          .to receive(:error)
+          .with(a_string_matching('ActiveRecord::StatementInvalid'))
+          .and_call_original
 
         worker.perform(pipeline_schedule.id, user.id)
       end