Commit a050fc8e authored by Lin Jen-Shin's avatar Lin Jen-Shin

Revert "Merge branch 'optimise-pipelines' into 'master'

This reverts commit 5fbb9e95, reversing
changes made to 197dad2e.
parent f139d129
...@@ -103,6 +103,7 @@ module Ci ...@@ -103,6 +103,7 @@ module Ci
end end
def playable? def playable?
project.builds_enabled? && has_commands? &&
action? && manual? action? && manual?
end end
...@@ -110,6 +111,10 @@ module Ci ...@@ -110,6 +111,10 @@ module Ci
self.when == 'manual' self.when == 'manual'
end end
def has_commands?
commands.present?
end
def play(current_user) def play(current_user)
# Try to queue a current build # Try to queue a current build
if self.enqueue if self.enqueue
...@@ -126,7 +131,8 @@ module Ci ...@@ -126,7 +131,8 @@ module Ci
end end
def retryable? def retryable?
success? || failed? || canceled? project.builds_enabled? && has_commands? &&
(success? || failed? || canceled?)
end end
def retried? def retried?
......
...@@ -17,12 +17,6 @@ module Ci ...@@ -17,12 +17,6 @@ module Ci
has_many :builds, foreign_key: :commit_id has_many :builds, foreign_key: :commit_id
has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id
has_many :pending_builds, -> { pending }, foreign_key: :commit_id, class_name: 'Ci::Build'
has_many :retryable_builds, -> { latest.failed_or_canceled }, foreign_key: :commit_id, class_name: 'Ci::Build'
has_many :cancelable_statuses, -> { cancelable }, foreign_key: :commit_id, class_name: 'CommitStatus'
has_many :manual_actions, -> { latest.manual_actions }, foreign_key: :commit_id, class_name: 'Ci::Build'
has_many :artifacts, -> { latest.with_artifacts_not_expired }, foreign_key: :commit_id, class_name: 'Ci::Build'
delegate :id, to: :project, prefix: true delegate :id, to: :project, prefix: true
validates :sha, presence: { unless: :importing? } validates :sha, presence: { unless: :importing? }
...@@ -176,6 +170,10 @@ module Ci ...@@ -176,6 +170,10 @@ module Ci
end end
end end
def artifacts
builds.latest.with_artifacts_not_expired.includes(project: [:namespace])
end
def valid_commit_sha def valid_commit_sha
if self.sha == Gitlab::Git::BLANK_SHA if self.sha == Gitlab::Git::BLANK_SHA
self.errors.add(:sha, " cant be 00000000 (branch removal)") self.errors.add(:sha, " cant be 00000000 (branch removal)")
...@@ -212,16 +210,20 @@ module Ci ...@@ -212,16 +210,20 @@ module Ci
!tag? !tag?
end end
def manual_actions
builds.latest.manual_actions.includes(project: [:namespace])
end
def stuck? def stuck?
pending_builds.any?(&:stuck?) builds.pending.includes(:project).any?(&:stuck?)
end end
def retryable? def retryable?
retryable_builds.any? builds.latest.failed_or_canceled.any?(&:retryable?)
end end
def cancelable? def cancelable?
cancelable_statuses.any? statuses.cancelable.any?
end end
def auto_canceled? def auto_canceled?
...@@ -229,8 +231,8 @@ module Ci ...@@ -229,8 +231,8 @@ module Ci
end end
def cancel_running def cancel_running
Gitlab::OptimisticLocking.retry_lock(cancelable_statuses) do |cancelable| Gitlab::OptimisticLocking.retry_lock(
cancelable.find_each do |job| statuses.cancelable) do |job|
yield(job) if block_given? yield(job) if block_given?
job.cancel job.cancel
end end
......
...@@ -173,8 +173,6 @@ class Project < ActiveRecord::Base ...@@ -173,8 +173,6 @@ class Project < ActiveRecord::Base
has_many :environments, dependent: :destroy has_many :environments, dependent: :destroy
has_many :deployments, dependent: :destroy has_many :deployments, dependent: :destroy
has_many :active_runners, -> { active }, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
accepts_nested_attributes_for :variables, allow_destroy: true accepts_nested_attributes_for :variables, allow_destroy: true
accepts_nested_attributes_for :project_feature accepts_nested_attributes_for :project_feature
...@@ -1074,15 +1072,15 @@ class Project < ActiveRecord::Base ...@@ -1074,15 +1072,15 @@ class Project < ActiveRecord::Base
end end
def shared_runners def shared_runners
@shared_runners ||= shared_runners_available? ? Ci::Runner.shared : Ci::Runner.none shared_runners_available? ? Ci::Runner.shared : Ci::Runner.none
end end
def active_shared_runners def any_runners?(&block)
@active_shared_runners ||= shared_runners.active if runners.active.any?(&block)
return true
end end
def any_runners?(&block) shared_runners.active.any?(&block)
active_runners.any?(&block) || active_shared_runners.any?(&block)
end end
def valid_runners_token?(token) def valid_runners_token?(token)
......
...@@ -69,13 +69,13 @@ class PipelineEntity < Grape::Entity ...@@ -69,13 +69,13 @@ class PipelineEntity < Grape::Entity
alias_method :pipeline, :object alias_method :pipeline, :object
def can_retry? def can_retry?
can?(request.user, :update_pipeline, pipeline) && pipeline.retryable? &&
pipeline.retryable? can?(request.user, :update_pipeline, pipeline)
end end
def can_cancel? def can_cancel?
can?(request.user, :update_pipeline, pipeline) && pipeline.cancelable? &&
pipeline.cancelable? can?(request.user, :update_pipeline, pipeline)
end end
def detailed_status def detailed_status
......
...@@ -13,15 +13,7 @@ class PipelineSerializer < BaseSerializer ...@@ -13,15 +13,7 @@ class PipelineSerializer < BaseSerializer
def represent(resource, opts = {}) def represent(resource, opts = {})
if resource.is_a?(ActiveRecord::Relation) if resource.is_a?(ActiveRecord::Relation)
resource = resource.preload([ resource = resource.includes(project: :namespace)
:retryable_builds,
:cancelable_statuses,
:trigger_requests,
:project,
{ pending_builds: :project },
{ manual_actions: :project },
{ artifacts: :project }
])
end end
if paginated? if paginated?
......
...@@ -7,7 +7,9 @@ module Ci ...@@ -7,7 +7,9 @@ module Ci
raise Gitlab::Access::AccessDeniedError raise Gitlab::Access::AccessDeniedError
end end
pipeline.retryable_builds.find_each do |build| pipeline.builds.latest.failed_or_canceled.find_each do |build|
next unless build.retryable?
Ci::RetryBuildService.new(project, current_user) Ci::RetryBuildService.new(project, current_user)
.reprocess(build) .reprocess(build)
end end
......
---
title: Optimise pipelines.json endpoint
merge_request:
author:
FactoryGirl.define do FactoryGirl.define do
factory :ci_trigger_without_token, class: Ci::Trigger do factory :ci_trigger_without_token, class: Ci::Trigger do
factory :ci_trigger do factory :ci_trigger do
sequence(:token) { |n| "token#{n}" } token 'token'
factory :ci_trigger_for_trigger_schedule do factory :ci_trigger_for_trigger_schedule do
token { SecureRandom.hex(15) } token { SecureRandom.hex(15) }
......
...@@ -92,11 +92,6 @@ pipelines: ...@@ -92,11 +92,6 @@ pipelines:
- auto_canceled_by - auto_canceled_by
- auto_canceled_pipelines - auto_canceled_pipelines
- auto_canceled_jobs - auto_canceled_jobs
- pending_builds
- retryable_builds
- cancelable_statuses
- manual_actions
- artifacts
statuses: statuses:
- project - project
- pipeline - pipeline
...@@ -214,7 +209,6 @@ project: ...@@ -214,7 +209,6 @@ project:
- builds - builds
- runner_projects - runner_projects
- runners - runners
- active_runners
- variables - variables
- triggers - triggers
- trigger_schedules - trigger_schedules
......
...@@ -764,6 +764,40 @@ describe Ci::Build, :models do ...@@ -764,6 +764,40 @@ describe Ci::Build, :models do
end end
end end
describe '#has_commands?' do
context 'when build has commands' do
let(:build) do
create(:ci_build, commands: 'rspec')
end
it 'has commands' do
expect(build).to have_commands
end
end
context 'when does not have commands' do
context 'when commands are an empty string' do
let(:build) do
create(:ci_build, commands: '')
end
it 'has no commands' do
expect(build).not_to have_commands
end
end
context 'when commands are not set at all' do
let(:build) do
create(:ci_build, commands: nil)
end
it 'has no commands' do
expect(build).not_to have_commands
end
end
end
end
describe '#has_tags?' do describe '#has_tags?' do
context 'when build has tags' do context 'when build has tags' do
subject { create(:ci_build, tag_list: ['tag']) } subject { create(:ci_build, tag_list: ['tag']) }
......
...@@ -17,8 +17,8 @@ describe Ci::Trigger, models: true do ...@@ -17,8 +17,8 @@ describe Ci::Trigger, models: true do
expect(trigger.token).not_to be_nil expect(trigger.token).not_to be_nil
end end
it 'does not set a random token if one provided' do it 'does not set an random token if one provided' do
trigger = create(:ci_trigger, project: project, token: 'token') trigger = create(:ci_trigger, project: project)
expect(trigger.token).to eq('token') expect(trigger.token).to eq('token')
end end
......
...@@ -58,7 +58,6 @@ describe Project, models: true do ...@@ -58,7 +58,6 @@ describe Project, models: true do
it { is_expected.to have_many(:builds) } it { is_expected.to have_many(:builds) }
it { is_expected.to have_many(:runner_projects) } it { is_expected.to have_many(:runner_projects) }
it { is_expected.to have_many(:runners) } it { is_expected.to have_many(:runners) }
it { is_expected.to have_many(:active_runners) }
it { is_expected.to have_many(:variables) } it { is_expected.to have_many(:variables) }
it { is_expected.to have_many(:triggers) } it { is_expected.to have_many(:triggers) }
it { is_expected.to have_many(:pages_domains) } it { is_expected.to have_many(:pages_domains) }
......
...@@ -93,44 +93,6 @@ describe PipelineSerializer do ...@@ -93,44 +93,6 @@ describe PipelineSerializer do
end end
end end
end end
context 'number of queries' do
let(:resource) { Ci::Pipeline.all }
let(:project) { create(:empty_project) }
before do
Ci::Pipeline::AVAILABLE_STATUSES.each do |status|
create_pipeline(status)
end
RequestStore.begin!
end
after do
RequestStore.end!
RequestStore.clear!
end
it "verifies number of queries" do
recorded = ActiveRecord::QueryRecorder.new { subject }
expect(recorded.count).to be_within(1).of(50)
expect(recorded.cached_count).to eq(0)
end
def create_pipeline(status)
create(:ci_empty_pipeline, project: project, status: status).tap do |pipeline|
Ci::Build::AVAILABLE_STATUSES.each do |status|
create_build(pipeline, status, status)
end
end
end
def create_build(pipeline, stage, status)
create(:ci_build, :tags, :triggered, :artifacts,
pipeline: pipeline, stage: stage,
name: stage, status: status)
end
end
end end
describe '#represent_status' do describe '#represent_status' do
......
...@@ -462,9 +462,7 @@ describe Ci::ProcessPipelineService, '#execute', :services do ...@@ -462,9 +462,7 @@ describe Ci::ProcessPipelineService, '#execute', :services do
builds.find_by(name: name).play(user) builds.find_by(name: name).play(user)
end end
def manual_actions delegate :manual_actions, to: :pipeline
pipeline.manual_actions(true)
end
def create_build(name, **opts) def create_build(name, **opts)
create(:ci_build, :created, pipeline: pipeline, name: name, **opts) create(:ci_build, :created, pipeline: pipeline, name: name, **opts)
......
module ActiveRecord module ActiveRecord
class QueryRecorder class QueryRecorder
attr_reader :log, :cached attr_reader :log
def initialize(&block) def initialize(&block)
@log = [] @log = []
@cached = []
ActiveSupport::Notifications.subscribed(method(:callback), 'sql.active_record', &block) ActiveSupport::Notifications.subscribed(method(:callback), 'sql.active_record', &block)
end end
def callback(name, start, finish, message_id, values) def callback(name, start, finish, message_id, values)
if values[:name]&.include?("CACHE") return if %w(CACHE SCHEMA).include?(values[:name])
@cached << values[:sql]
elsif !values[:name]&.include?("SCHEMA")
@log << values[:sql] @log << values[:sql]
end end
end
def count def count
@log.count @log.count
end end
def cached_count
@cached.count
end
def log_message def log_message
@log.join("\n\n") @log.join("\n\n")
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