Revert fair scheduling for all builds
Per discussion in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9646#note_65730532 this logic is being optimized elsewhere and it will simplify things if we make less changes to this code right now.
This commit is contained in:
parent
0077679ae8
commit
87740df2ba
|
@ -99,24 +99,6 @@ module Ci
|
|||
self.token = SecureRandom.hex(15) if self.token.blank?
|
||||
end
|
||||
|
||||
def accessible_projects
|
||||
accessible_projects =
|
||||
if shared?
|
||||
Project.with_shared_runners
|
||||
elsif project?
|
||||
projects
|
||||
elsif group?
|
||||
hierarchy_groups = Gitlab::GroupHierarchy.new(groups).base_and_descendants
|
||||
Project.where(namespace_id: hierarchy_groups)
|
||||
else
|
||||
Project.none
|
||||
end
|
||||
|
||||
accessible_projects
|
||||
.with_builds_enabled
|
||||
.without_deleted
|
||||
end
|
||||
|
||||
def assign_to(project, current_user = nil)
|
||||
self.is_shared = false if shared?
|
||||
self.save
|
||||
|
|
|
@ -14,7 +14,14 @@ module Ci
|
|||
end
|
||||
|
||||
def execute
|
||||
builds = builds_for_runner
|
||||
builds =
|
||||
if runner.shared?
|
||||
builds_for_shared_runner
|
||||
elsif runner.group?
|
||||
builds_for_group_runner
|
||||
else
|
||||
builds_for_project_runner
|
||||
end
|
||||
|
||||
valid = true
|
||||
|
||||
|
@ -63,27 +70,39 @@ module Ci
|
|||
|
||||
private
|
||||
|
||||
def builds_for_runner
|
||||
new_builds
|
||||
.joins("LEFT JOIN (#{running_projects.to_sql}) AS running_projects ON ci_builds.project_id=running_projects.project_id")
|
||||
.order('COALESCE(running_projects.running_builds, 0) ASC', 'ci_builds.id ASC')
|
||||
def builds_for_shared_runner
|
||||
new_builds.
|
||||
# don't run projects which have not enabled shared runners and builds
|
||||
joins(:project).where(projects: { shared_runners_enabled: true, pending_delete: false })
|
||||
.joins('LEFT JOIN project_features ON ci_builds.project_id = project_features.project_id')
|
||||
.where('project_features.builds_access_level IS NULL or project_features.builds_access_level > 0').
|
||||
|
||||
# Implement fair scheduling
|
||||
# this returns builds that are ordered by number of running builds
|
||||
# we prefer projects that don't use shared runners at all
|
||||
joins("LEFT JOIN (#{running_builds_for_shared_runners.to_sql}) AS project_builds ON ci_builds.project_id=project_builds.project_id")
|
||||
.order('COALESCE(project_builds.running_builds, 0) ASC', 'ci_builds.id ASC')
|
||||
end
|
||||
|
||||
# New builds from the accessible projects
|
||||
def new_builds
|
||||
filter_builds(Ci::Build.pending.unstarted)
|
||||
def builds_for_project_runner
|
||||
new_builds.where(project: runner.projects.without_deleted.with_builds_enabled).order('created_at ASC')
|
||||
end
|
||||
|
||||
# Count running builds from the accessible projects
|
||||
def running_projects
|
||||
filter_builds(Ci::Build.running)
|
||||
def builds_for_group_runner
|
||||
hierarchy_groups = Gitlab::GroupHierarchy.new(runner.groups).base_and_descendants
|
||||
projects = Project.where(namespace_id: hierarchy_groups).without_deleted.with_builds_enabled
|
||||
new_builds.where(project: projects.without_deleted.with_builds_enabled).order('created_at ASC')
|
||||
end
|
||||
|
||||
def running_builds_for_shared_runners
|
||||
Ci::Build.running.where(runner: Ci::Runner.shared)
|
||||
.group(:project_id).select(:project_id, 'count(*) AS running_builds')
|
||||
end
|
||||
|
||||
# Filter the builds from the accessible projects
|
||||
def filter_builds(builds)
|
||||
def new_builds
|
||||
builds = Ci::Build.pending.unstarted
|
||||
builds = builds.ref_protected if runner.ref_protected?
|
||||
builds.where(project: runner.accessible_projects)
|
||||
builds
|
||||
end
|
||||
|
||||
def register_failure
|
||||
|
|
|
@ -792,73 +792,6 @@ describe Ci::Runner do
|
|||
end
|
||||
end
|
||||
|
||||
describe '#accessible_projects' do
|
||||
let!(:shared_runner) { create(:ci_runner, :shared) }
|
||||
let!(:shared_project) { create(:project, shared_runners_enabled: true) }
|
||||
|
||||
let!(:project_runner) { create(:ci_runner) }
|
||||
let!(:project_project) { create(:project, runners: [project_runner], shared_runners_enabled: false) }
|
||||
|
||||
let!(:group_runner) { create(:ci_runner) }
|
||||
|
||||
let!(:parent_group) { create(:group) }
|
||||
let!(:parent_group_project) do
|
||||
create(:project, group: parent_group, shared_runners_enabled: false)
|
||||
end
|
||||
|
||||
let!(:group) { create :group, runners: [group_runner], parent: parent_group }
|
||||
let!(:group_project) do
|
||||
create(:project, group: group, shared_runners_enabled: false)
|
||||
end
|
||||
|
||||
let!(:nested_group_project) do
|
||||
nested_group = create :group, parent: group
|
||||
create(:project, group: nested_group, shared_runners_enabled: false)
|
||||
end
|
||||
|
||||
it 'returns the project with a shared runner' do
|
||||
expect(shared_runner.reload.accessible_projects).to eq [shared_project]
|
||||
end
|
||||
|
||||
it 'returns the project with a project runner' do
|
||||
expect(project_runner.reload.accessible_projects).to eq [project_project]
|
||||
end
|
||||
|
||||
it 'returns the projects with a group and nested group runner', :nested_groups do
|
||||
expect(group_runner.reload.accessible_projects).to eq [group_project, nested_group_project]
|
||||
end
|
||||
|
||||
context 'deleted' do
|
||||
before do
|
||||
shared_project.update_attributes!(pending_delete: true)
|
||||
project_project.update_attributes!(pending_delete: true)
|
||||
group_project.update_attributes!(pending_delete: true)
|
||||
nested_group_project.update_attributes!(pending_delete: true)
|
||||
end
|
||||
|
||||
it 'returns no projects' do
|
||||
expect(shared_runner.reload.accessible_projects).to be_empty
|
||||
expect(project_runner.reload.accessible_projects).to be_empty
|
||||
expect(group_runner.reload.accessible_projects).to be_empty
|
||||
end
|
||||
end
|
||||
|
||||
context 'builds disabled' do
|
||||
before do
|
||||
shared_project.update_attributes!(builds_enabled: false)
|
||||
project_project.update_attributes!(builds_enabled: false)
|
||||
group_project.update_attributes!(builds_enabled: false)
|
||||
nested_group_project.update_attributes!(builds_enabled: false)
|
||||
end
|
||||
|
||||
it 'returns no projects' do
|
||||
expect(shared_runner.reload.accessible_projects).to be_empty
|
||||
expect(project_runner.reload.accessible_projects).to be_empty
|
||||
expect(group_runner.reload.accessible_projects).to be_empty
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#invalidate_build_cache!' do
|
||||
context 'runner can pick the build' do
|
||||
it 'calls #tick_runner_queue' do
|
||||
|
|
|
@ -195,56 +195,25 @@ module Ci
|
|||
let!(:unrelated_group_runner) { create :ci_runner, groups: [unrelated_group] }
|
||||
|
||||
it 'does not consider builds from other group runners' do
|
||||
expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 6
|
||||
expect(described_class.new(group_runner).send(:builds_for_group_runner).count).to eq 6
|
||||
execute(group_runner)
|
||||
|
||||
expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 5
|
||||
expect(described_class.new(group_runner).send(:builds_for_group_runner).count).to eq 5
|
||||
execute(group_runner)
|
||||
|
||||
expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 4
|
||||
expect(described_class.new(group_runner).send(:builds_for_group_runner).count).to eq 4
|
||||
execute(group_runner)
|
||||
|
||||
expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 3
|
||||
expect(described_class.new(group_runner).send(:builds_for_group_runner).count).to eq 3
|
||||
execute(group_runner)
|
||||
|
||||
expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 2
|
||||
expect(described_class.new(group_runner).send(:builds_for_group_runner).count).to eq 2
|
||||
execute(group_runner)
|
||||
|
||||
expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 1
|
||||
expect(described_class.new(group_runner).send(:builds_for_group_runner).count).to eq 1
|
||||
execute(group_runner)
|
||||
|
||||
expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 0
|
||||
expect(execute(group_runner)).to be_nil
|
||||
end
|
||||
|
||||
it 'prefers projects without builds first' do
|
||||
# it gets for one build from each of the projects
|
||||
expect(execute(group_runner)).to eq(build1_project1)
|
||||
expect(execute(group_runner)).to eq(build1_project2)
|
||||
expect(execute(group_runner)).to eq(build1_project3)
|
||||
|
||||
# then it gets a second build from each of the projects
|
||||
expect(execute(group_runner)).to eq(build2_project1)
|
||||
expect(execute(group_runner)).to eq(build2_project2)
|
||||
|
||||
# in the end the third build
|
||||
expect(execute(group_runner)).to eq(build3_project1)
|
||||
|
||||
expect(execute(group_runner)).to be_nil
|
||||
end
|
||||
|
||||
it 'equalises number of running builds' do
|
||||
# after finishing the first build for project 1, get a second build from the same project
|
||||
expect(execute(group_runner)).to eq(build1_project1)
|
||||
build1_project1.reload.success
|
||||
expect(execute(group_runner)).to eq(build2_project1)
|
||||
|
||||
expect(execute(group_runner)).to eq(build1_project2)
|
||||
build1_project2.reload.success
|
||||
expect(execute(group_runner)).to eq(build2_project2)
|
||||
expect(execute(group_runner)).to eq(build1_project3)
|
||||
expect(execute(group_runner)).to eq(build3_project1)
|
||||
|
||||
expect(described_class.new(group_runner).send(:builds_for_group_runner).count).to eq 0
|
||||
expect(execute(group_runner)).to be_nil
|
||||
end
|
||||
end
|
||||
|
@ -282,7 +251,7 @@ module Ci
|
|||
let!(:other_build) { create :ci_build, pipeline: pipeline }
|
||||
|
||||
before do
|
||||
allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_runner)
|
||||
allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_project_runner)
|
||||
.and_return(Ci::Build.where(id: [pending_job, other_build]))
|
||||
end
|
||||
|
||||
|
@ -294,7 +263,7 @@ module Ci
|
|||
|
||||
context 'when single build is in queue' do
|
||||
before do
|
||||
allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_runner)
|
||||
allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_project_runner)
|
||||
.and_return(Ci::Build.where(id: pending_job))
|
||||
end
|
||||
|
||||
|
@ -305,7 +274,7 @@ module Ci
|
|||
|
||||
context 'when there is no build in queue' do
|
||||
before do
|
||||
allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_runner)
|
||||
allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_project_runner)
|
||||
.and_return(Ci::Build.none)
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in New Issue