From 8a1c12aa6f9f0f4d7815b5c7c945caadcfc5e10d Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 6 Apr 2017 14:31:38 +0200 Subject: [PATCH] Introduce endpoint optimisations --- app/models/ci/build.rb | 10 ++------ app/models/ci/pipeline.rb | 27 +++++++++----------- app/models/project.rb | 14 +++++++--- app/serializers/pipeline_entity.rb | 8 +++--- app/services/ci/retry_pipeline_service.rb | 4 +-- spec/serializers/pipeline_serializer_spec.rb | 2 +- 6 files changed, 31 insertions(+), 34 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index ad0be70c32a..9ef55fa6e18 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -103,18 +103,13 @@ module Ci end def playable? - project.builds_enabled? && has_commands? && - action? && manual? + action? && manual? end def action? self.when == 'manual' end - def has_commands? - commands.present? - end - def play(current_user) # Try to queue a current build if self.enqueue @@ -131,8 +126,7 @@ module Ci end def retryable? - project.builds_enabled? && has_commands? && - (success? || failed? || canceled?) + success? || failed? || canceled? end def retried? diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 49dec770096..7f9b0aaa0f1 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -12,6 +12,12 @@ module Ci has_many :builds, 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 validates :sha, presence: { unless: :importing? } @@ -160,10 +166,6 @@ module Ci end end - def artifacts - builds.latest.with_artifacts_not_expired.includes(project: [:namespace]) - end - def valid_commit_sha if self.sha == Gitlab::Git::BLANK_SHA self.errors.add(:sha, " cant be 00000000 (branch removal)") @@ -200,27 +202,22 @@ module Ci !tag? end - def manual_actions - builds.latest.manual_actions.includes(project: [:namespace]) - end - def stuck? - builds.pending.includes(:project).any?(&:stuck?) + pending_builds.any?(&:stuck?) end def retryable? - builds.latest.failed_or_canceled.any?(&:retryable?) + retryable_builds.any? end def cancelable? - statuses.cancelable.any? + cancelable_statuses.any? end def cancel_running - Gitlab::OptimisticLocking.retry_lock( - statuses.cancelable) do |cancelable| - cancelable.find_each(&:cancel) - end + Gitlab::OptimisticLocking.retry_lock(cancelable_statuses) do |cancelable| + cancelable.find_each(&:cancel) + end end def retry_failed(current_user) diff --git a/app/models/project.rb b/app/models/project.rb index 83660d8c431..4da0e016756 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1093,15 +1093,23 @@ class Project < ActiveRecord::Base end def shared_runners - shared_runners_available? ? Ci::Runner.shared : Ci::Runner.none + @shared_runners ||= shared_runners_available? ? Ci::Runner.shared : Ci::Runner.none + end + + def active_runners + @active_runners ||= runners.active + end + + def active_shared_runners + @active_shared_runners ||= shared_runners.active end def any_runners?(&block) - if runners.active.any?(&block) + if active_runners.any?(&block) return true end - shared_runners.active.any?(&block) + active_shared_runners.any?(&block) end def valid_runners_token?(token) diff --git a/app/serializers/pipeline_entity.rb b/app/serializers/pipeline_entity.rb index 3f16dd66d54..ad8b4d43e8f 100644 --- a/app/serializers/pipeline_entity.rb +++ b/app/serializers/pipeline_entity.rb @@ -69,13 +69,13 @@ class PipelineEntity < Grape::Entity alias_method :pipeline, :object def can_retry? - pipeline.retryable? && - can?(request.user, :update_pipeline, pipeline) + can?(request.user, :update_pipeline, pipeline) && + pipeline.retryable? end def can_cancel? - pipeline.cancelable? && - can?(request.user, :update_pipeline, pipeline) + can?(request.user, :update_pipeline, pipeline) && + pipeline.cancelable? end def detailed_status diff --git a/app/services/ci/retry_pipeline_service.rb b/app/services/ci/retry_pipeline_service.rb index f72ddbf690c..ecc6173a96a 100644 --- a/app/services/ci/retry_pipeline_service.rb +++ b/app/services/ci/retry_pipeline_service.rb @@ -7,9 +7,7 @@ module Ci raise Gitlab::Access::AccessDeniedError end - pipeline.builds.latest.failed_or_canceled.find_each do |build| - next unless build.retryable? - + pipeline.retryable_builds.find_each do |build| Ci::RetryBuildService.new(project, current_user) .reprocess(build) end diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index 657d10aae99..add43182cc2 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -105,7 +105,7 @@ describe PipelineSerializer do it "verifies number of queries" do recorded = ActiveRecord::QueryRecorder.new { subject } - expect(recorded.count).to be_within(320).of(10) + expect(recorded.count).to be_within(200).of(10) end def create_pipeline(status)