Skip to content

Increase all plans max capacity by one #183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Increase all plans max capacity by one #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eae37af
Typos and whitespace fixes
kerrizor Dec 5, 2018
910008d
Increase the job capacity of all plans by 1
kerrizor Dec 5, 2018
0b682ee
Cleanup logic usage
kerrizor Dec 5, 2018
8dc76b2
Merge branch 'master' into kerrizor/increase-all-plans-max-capacity-b…
kerrizor Dec 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions lib/travis/owners/subscriptions.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,32 @@ def plan_limits
end

def plan_limit(plan)
config[plan.to_sym].tap { |limit| missing_plan(plan) unless limit }
limit = config[plan[:plan].to_sym].tap { |limit| missing_plan(plan[:plan]) unless limit }

# Increment this by 1 as our original values are being increased up by
# one as part of our "free 1-job private repo" plan project,
# expected to be launched in December, 2018.
#
# https://github.com/travis-ci/product/issues/97
#

if plan[:owner].is_a?(User) && limit
limit += 1
end

limit
end

def missing_plan(plan)
logger.warn MSGS[:missing_plan] % [plan, owners.to_s]
end

def plans
subscriptions.map(&:selected_plan).compact
subscriptions.map { |sub|
if sub.selected_plan && sub.owner
{ plan: sub.selected_plan, owner: sub.owner }
end
}.compact

This comment was marked as spam.

Sign in to view

end

def subscriptions
Expand Down
2 changes: 1 addition & 1 deletion lib/travis/scheduler/limit/jobs.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def check_all
queueable.each_with_index do |job, index|
case check(job)
when :limited
# The rest of the jobs will definitely be waiting for
# The rest of the jobs will definitely be waiting for
# concurrency, regardless of other limits that might apply
@waiting_by_owner += queueable.length - index
break
Expand Down
7 changes: 7 additions & 0 deletions spec/travis/owners/subscriptions_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@

subject { described_class.new(owners, plans).max_jobs }

# Note that all plans are 1 higher than their original values due to our "free
# 1-job private repo" plan project, expected to be launched in December,
# 2018.
#
# https://github.com/travis-ci/product/issues/97
#

describe 'a single org with a five jobs plan' do
before { FactoryGirl.create(:subscription, owner: travis, selected_plan: :five) }
it { should eq 5 }
Expand Down
4 changes: 2 additions & 2 deletions spec/travis/owners_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

describe 'with a subscription on the delegatee' do
before { FactoryGirl.create(:subscription, owner: anja, selected_plan: :ten) }
it { expect(owners.max_jobs).to eq 10 }
it { expect(owners.max_jobs).to eq 11 }

This comment was marked as spam.

Sign in to view

end

describe 'with a subscription on the delegate' do
Expand All @@ -27,7 +27,7 @@
describe 'with a subscription on both the delegatee and delegate' do
before { FactoryGirl.create(:subscription, owner: anja, selected_plan: :ten) }
before { FactoryGirl.create(:subscription, owner: travis, selected_plan: :five) }
it { expect(owners.max_jobs).to eq 15 }
it { expect(owners.max_jobs).to eq 16 }

This comment was marked as spam.

Sign in to view

end
end

Expand Down
48 changes: 24 additions & 24 deletions spec/travis/scheduler/jobs_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,33 +86,33 @@ def subscribe(plan, owner = self.user)
before { create_jobs(1, private: true, state: :started) }
before { create_jobs(5, private: true) }

it { expect(selected.size).to eq 1 }
it { expect(reports).to include 'user svenfuchs capacities: public max=3, plan max=2' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=1 max=2 selected=1' }
it { expect(reports).to include 'user svenfuchs: queueable=5 running=1 selected=1 total_waiting=4 waiting_for_concurrency=4' }
it { expect(selected.size).to eq 2 }
it { expect(reports).to include 'user svenfuchs capacities: public max=3, plan max=3' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=1 max=3 selected=2' }
it { expect(reports).to include 'user svenfuchs: queueable=5 running=1 selected=2 total_waiting=3 waiting_for_concurrency=3' }

This comment was marked as spam.

Sign in to view

end

describe 'with public jobs only' do
before { create_jobs(1, private: false, state: :started) }
before { create_jobs(5, private: false) }

it { expect(selected.size).to eq 4 }
it { expect(reports).to include 'user svenfuchs capacities: public max=3, plan max=2' }
it { expect(selected.size).to eq 5 }
it { expect(reports).to include 'user svenfuchs capacities: public max=3, plan max=3' }
it { expect(reports).to include 'user svenfuchs public capacity: running=1 max=3 selected=2' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=0 max=2 selected=2' }
it { expect(reports).to include 'user svenfuchs: queueable=5 running=1 selected=4 total_waiting=1 waiting_for_concurrency=1' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=0 max=3 selected=3' }
it { expect(reports).to include 'user svenfuchs: queueable=5 running=1 selected=5 total_waiting=0 waiting_for_concurrency=0' }
end

describe 'for mixed public and private jobs' do
before { create_jobs(1, private: true, state: :started) }
before { create_jobs(1, private: false, state: :started) }
before { create_jobs(2, private: false) + create_jobs(2, private: true) }

it { expect(selected.size).to eq 3 }
it { expect(reports).to include 'user svenfuchs capacities: public max=3, plan max=2' }
it { expect(selected.size).to eq 4 }
it { expect(reports).to include 'user svenfuchs capacities: public max=3, plan max=3' }
it { expect(reports).to include 'user svenfuchs public capacity: running=1 max=3 selected=2' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=1 max=2 selected=1' }
it { expect(reports).to include 'user svenfuchs: queueable=4 running=2 selected=3 total_waiting=1 waiting_for_concurrency=1' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=1 max=3 selected=2' }
it { expect(reports).to include 'user svenfuchs: queueable=4 running=2 selected=4 total_waiting=0 waiting_for_concurrency=0' }
end
end

Expand Down Expand Up @@ -232,30 +232,30 @@ def subscribe(plan, owner = self.user)
before { create_jobs(1, private: true, state: :started) }
before { create_jobs(5, private: true) }

it { expect(selected.size).to eq 1 }
it { expect(reports).to include 'user svenfuchs plan capacity: running=1 max=2 selected=1' }
it { expect(reports).to include 'user svenfuchs: queueable=5 running=1 selected=1 total_waiting=4 waiting_for_concurrency=4' }
it { expect(selected.size).to eq 2 }
it { expect(reports).to include 'user svenfuchs plan capacity: running=1 max=3 selected=2' }
it { expect(reports).to include 'user svenfuchs: queueable=5 running=1 selected=2 total_waiting=3 waiting_for_concurrency=3' }
end

describe 'with public jobs only' do
before { create_jobs(1, private: false, state: :started) }
before { create_jobs(5, private: false) }

it { expect(selected.size).to eq 4 }
it { expect(selected.size).to eq 5 }
it { expect(reports).to include 'user svenfuchs public capacity: running=1 max=3 selected=2' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=0 max=2 selected=2' }
it { expect(reports).to include 'user svenfuchs: queueable=5 running=1 selected=4 total_waiting=1 waiting_for_concurrency=1' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=0 max=3 selected=3' }
it { expect(reports).to include 'user svenfuchs: queueable=5 running=1 selected=5 total_waiting=0 waiting_for_concurrency=0' }
end

describe 'for mixed public and private jobs' do
before { create_jobs(1, private: true, state: :started) }
before { create_jobs(1, private: false, state: :started) }
before { create_jobs(2, private: false) + create_jobs(2, private: true) }

it { expect(selected.size).to eq 3 }
it { expect(selected.size).to eq 4 }
it { expect(reports).to include 'user svenfuchs public capacity: running=1 max=3 selected=2' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=1 max=2 selected=1' }
it { expect(reports).to include 'user svenfuchs: queueable=4 running=2 selected=3 total_waiting=1 waiting_for_concurrency=1' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=1 max=3 selected=2' }
it { expect(reports).to include 'user svenfuchs: queueable=4 running=2 selected=4 total_waiting=0 waiting_for_concurrency=0' }
end
end

Expand Down Expand Up @@ -381,7 +381,7 @@ def subscribe(plan, owner = self.user)

it { expect(selected.size).to eq 3 }
it { expect(reports).to include 'user svenfuchs limited by queue builds.osx: max=3 rejected=7 selected=2' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=2 max=10 selected=3' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=2 max=11 selected=3' }
it { expect(reports).to include 'user svenfuchs: queueable=10 running=2 selected=3 total_waiting=7 waiting_for_concurrency=0' }
end

Expand All @@ -394,7 +394,7 @@ def subscribe(plan, owner = self.user)
it { expect(selected.size).to eq 3 }
it { expect(reports).to include 'user svenfuchs limited by queue builds.osx: max=3 rejected=7 selected=2' }
it { expect(reports).to include 'user svenfuchs public capacity: running=2 max=3 selected=1' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=0 max=10 selected=2' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=0 max=11 selected=2' }
it { expect(reports).to include 'user svenfuchs: queueable=10 running=2 selected=3 total_waiting=7 waiting_for_concurrency=0' }
end

Expand All @@ -409,7 +409,7 @@ def subscribe(plan, owner = self.user)
it { expect(selected.size).to eq 4 }
it { expect(reports).to include 'user svenfuchs limited by queue builds.osx: max=3 rejected=6 selected=2' }
it { expect(reports).to include 'user svenfuchs public capacity: running=0 max=3 selected=1' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=2 max=10 selected=3' }
it { expect(reports).to include 'user svenfuchs plan capacity: running=2 max=11 selected=3' }
it { expect(reports).to include 'user svenfuchs: queueable=10 running=2 selected=4 total_waiting=6 waiting_for_concurrency=0' }
end
end
Expand Down
34 changes: 17 additions & 17 deletions spec/travis/scheduler/limit/com_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ def subscription(plan, owner = self.owner)
before { create_jobs(2, state: :created, private: true) }
before { run }

it { expect(selected).to eq 1 }
it { expect(report).to include('max jobs for user svenfuchs by plan: 2 (svenfuchs)') }
it { expect(report).to include('user svenfuchs: total: 2, running: 1, queueable: 1') }
it { expect(limit.waiting_by_owner).to eq 1 }
it { expect(selected).to eq 2 }
it { expect(report).to include('max jobs for user svenfuchs by plan: 3 (svenfuchs)') }
it { expect(report).to include('user svenfuchs: total: 2, running: 1, queueable: 2') }
it { expect(limit.waiting_by_owner).to eq 0 }
end

describe 'with public jobs only' do
Expand All @@ -97,7 +97,7 @@ def subscription(plan, owner = self.owner)
before { run }

it { expect(selected).to eq 2 }
it { expect(report).to include('max jobs for user svenfuchs by plan: 5 (svenfuchs)') } # TODO fix log output?
it { expect(report).to include('max jobs for user svenfuchs by plan: 6 (svenfuchs)') } # TODO fix log output?
it { expect(report).to include('user svenfuchs: total: 2, running: 1, queueable: 2') }
it { expect(limit.waiting_by_owner).to eq 0 }
end
Expand All @@ -110,10 +110,10 @@ def subscription(plan, owner = self.owner)
before { subscription(:two) }
before { run }

it { expect(selected).to eq 3 }
it { expect(report).to include('max jobs for user svenfuchs by plan: 2 (svenfuchs)') }
it { expect(report).to include('user svenfuchs: total: 4, running: 2, queueable: 3') }
it { expect(limit.waiting_by_owner).to eq 1 }
it { expect(selected).to eq 4 }
it { expect(report).to include('max jobs for user svenfuchs by plan: 3 (svenfuchs)') }
it { expect(report).to include('user svenfuchs: total: 4, running: 2, queueable: 4') }
it { expect(limit.waiting_by_owner).to eq 0 }
end
end

Expand Down Expand Up @@ -303,7 +303,7 @@ def subscription(plan, owner = self.owner)
before { run }

it { expect(selected).to eq 2 }
it { expect(report).to include('max jobs for user svenfuchs by plan: 4 (svenfuchs)') }
it { expect(report).to include('max jobs for user svenfuchs by plan: 5 (svenfuchs)') }
it { expect(report).to include('max jobs for repo svenfuchs/gem-release by repo_settings: 3') }
it { expect(report).to include('user svenfuchs: total: 4, running: 1, queueable: 2') }
it { expect(limit.waiting_by_owner).to eq 0 } # TODO eh??? should be 2, no?
Expand All @@ -315,7 +315,7 @@ def subscription(plan, owner = self.owner)
before { run }

it { expect(selected).to eq 2 }
it { expect(report).to include('max jobs for user svenfuchs by plan: 7 (svenfuchs)') } # TODO fix log output?
it { expect(report).to include('max jobs for user svenfuchs by plan: 8 (svenfuchs)') } # TODO fix log output?
it { expect(report).to include('max jobs for repo svenfuchs/gem-release by repo_settings: 3') }
it { expect(report).to include('user svenfuchs: total: 4, running: 1, queueable: 2') }
it { expect(limit.waiting_by_owner).to eq 0 }
Expand All @@ -329,7 +329,7 @@ def subscription(plan, owner = self.owner)
before { run }

it { expect(selected).to eq 1 }
it { expect(report).to include('max jobs for user svenfuchs by plan: 4 (svenfuchs)') }
it { expect(report).to include('max jobs for user svenfuchs by plan: 5 (svenfuchs)') }
it { expect(report).to include('max jobs for repo svenfuchs/gem-release by repo_settings: 3') }
it { expect(report).to include('user svenfuchs: total: 6, running: 2, queueable: 1') }
it { expect(limit.waiting_by_owner).to eq 0 } # TODO eh???
Expand All @@ -343,8 +343,8 @@ def subscription(plan, owner = self.owner)
before { subscription(:two) }
before { run }

it { expect(selected).to eq 2 }
it { expect(report).to include('user svenfuchs: total: 4, running: 0, queueable: 2') }
it { expect(selected).to eq 3 }
it { expect(report).to include('user svenfuchs: total: 4, running: 0, queueable: 3') }
end

describe 'with public jobs only' do
Expand Down Expand Up @@ -397,7 +397,7 @@ def subscription(plan, owner = self.owner)

it { expect(selected).to eq 6 }
it { expect(limit.selected.map(&:owner).map(&:login)).to eq ['svenfuchs'] * 3 + ['travis-ci'] * 3 }
it { expect(report).to include('max jobs for user svenfuchs by plan: 8 (svenfuchs, travis-ci)') }
it { expect(report).to include('max jobs for user svenfuchs by plan: 9 (svenfuchs, travis-ci)') }
it { expect(report).to include('user svenfuchs, user carla, org travis-ci: total: 6, running: 2, queueable: 6') }
end
end
Expand Down Expand Up @@ -430,7 +430,7 @@ def subscription(plan, owner = self.owner)

it { expect(selected).to eq 6 }
it { expect(limit.selected.map(&:owner).map(&:login)).to eq ['svenfuchs'] * 3 + ['travis-ci'] * 3 }
it { expect(report).to include('max jobs for user svenfuchs by plan: 8 (svenfuchs, travis-ci)') } # TODO fix log output
it { expect(report).to include('max jobs for user svenfuchs by plan: 9 (svenfuchs, travis-ci)') } # TODO fix log output
it { expect(report).to include('user svenfuchs, user carla, org travis-ci: total: 6, running: 2, queueable: 6') }
end
end
Expand Down Expand Up @@ -463,7 +463,7 @@ def subscription(plan, owner = self.owner)

it { expect(selected).to eq 6 }
it { expect(limit.selected.map(&:owner).map(&:login)).to eq ['svenfuchs'] * 3 + ['travis-ci'] * 3 }
it { expect(report).to include('max jobs for user svenfuchs by plan: 8 (svenfuchs, travis-ci)') }
it { expect(report).to include('max jobs for user svenfuchs by plan: 9 (svenfuchs, travis-ci)') }
it { expect(report).to include('user svenfuchs, user carla, org travis-ci: total: 6, running: 2, queueable: 6') }
end
end
Expand Down