Skip to content
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.

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

Improve service keys quota check performance #2564

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -1,10 +1,10 @@
class MaxServiceKeysPolicy
attr_reader :quota_definition

def initialize(service_key, existing_service_keys_count, quota_definition, error_name)
def initialize(service_key, existing_service_keys_dataset, quota_definition, error_name)
@service_key = service_key
@existing_service_keys_dataset = existing_service_keys_dataset
@quota_definition = quota_definition
@existing_service_keys_count = existing_service_keys_count
@error_name = error_name
@errors = service_key.errors
end
Expand All @@ -19,7 +19,7 @@ def validate

def service_keys_quota_remaining?
@quota_definition.total_service_keys == -1 || # unlimited
@existing_service_keys_count + requested_service_key <= @quota_definition.total_service_keys
@existing_service_keys_dataset.count + requested_service_key <= @quota_definition.total_service_keys
end

def requested_service_key
Expand Down
5 changes: 3 additions & 2 deletions app/models/services/service_key.rb
Expand Up @@ -43,15 +43,16 @@ def validate
validates_unique [:name, :service_instance_id]

if service_instance
space_ids_for_org_dataset = Space.where(organization_id: space.organization.id).select(:id)
MaxServiceKeysPolicy.new(
self,
ServiceKey.filter(service_instance: space.organization.service_instances).count,
ServiceKey.filter(service_instance: ServiceInstance.where(space_id: space_ids_for_org_dataset)),
space.organization.quota_definition,
:service_keys_quota_exceeded
).validate
MaxServiceKeysPolicy.new(
self,
ServiceKey.filter(service_instance: space.service_instances).count,
ServiceKey.filter(service_instance: ServiceInstance.where(space_id: space.id)),
space.space_quota_definition,
:service_keys_space_quota_exceeded
).validate
Expand Down
@@ -0,0 +1,13 @@
Sequel.migration do
up do
alter_table :service_keys do
add_index :service_instance_id, name: :sk_svc_instance_id_index
end
end

down do
alter_table :service_keys do
drop_index :service_instance_id, name: :sk_svc_instance_id_index
end
end
end
Expand Up @@ -11,9 +11,10 @@
let(:total_service_keys) { 2 }
let(:quota) { VCAP::CloudController::QuotaDefinition.make total_service_keys: total_service_keys }
let(:existing_service_key_count) { 0 }
let(:existing_service_key_dataset) { double(Sequel::Dataset, count: existing_service_key_count) }
let(:error_name) { :random_error_name }

let(:policy) { MaxServiceKeysPolicy.new(service_key, existing_service_key_count, quota, error_name) }
let(:policy) { MaxServiceKeysPolicy.new(service_key, existing_service_key_dataset, quota, error_name) }

def make_service_key
VCAP::CloudController::ServiceKey.make service_instance: service_instance
Expand Down
15 changes: 11 additions & 4 deletions spec/unit/models/services/service_key_spec.rb
@@ -1,5 +1,9 @@
require 'spec_helper'

RSpec::Matchers.define :existing_service_key_count do |c|
match { |arg| arg.is_a?(Sequel::Dataset) && arg.count == c }
end

module VCAP::CloudController
RSpec.describe VCAP::CloudController::ServiceKey, type: :model do
let(:client) { double('broker client', unbind: nil, deprovision: nil) }
Expand All @@ -23,15 +27,17 @@ module VCAP::CloudController

context 'MaxServiceKeysPolicy' do
let(:service_key) { ServiceKey.make }
let(:existing_service_key_dataset) { double(Sequel::Dataset, count: 1) }
let(:policy) { double(MaxServiceKeysPolicy) }
let(:space_policy) { double(MaxServiceKeysPolicy) }

it 'validates org quotas using MaxServiceKeysPolicy' do
expect(MaxServiceKeysPolicy).to receive(:new).
with(service_key, 1, service_key.service_instance.organization.quota_definition, :service_keys_quota_exceeded).
with(service_key, existing_service_key_count(1), service_key.service_instance.organization.quota_definition, :service_keys_quota_exceeded).
and_return(policy)
expect(policy).to receive(:validate)
expect(MaxServiceKeysPolicy).to receive(:new).with(service_key, 1, nil, :service_keys_space_quota_exceeded).and_return(space_policy)
expect(MaxServiceKeysPolicy).to receive(:new).with(service_key, existing_service_key_count(1), nil, :service_keys_space_quota_exceeded).
and_return(space_policy)
expect(space_policy).to receive(:validate)

service_key.valid?
Expand All @@ -47,10 +53,11 @@ module VCAP::CloudController

it 'validates space quotas using MaxServiceKeysPolicy' do
expect(MaxServiceKeysPolicy).to receive(:new).
with(service_key, 1, service_key.service_instance.organization.quota_definition, :service_keys_quota_exceeded).
with(service_key, existing_service_key_count(1), service_key.service_instance.organization.quota_definition, :service_keys_quota_exceeded).
and_return(policy)
expect(policy).to receive(:validate)
expect(MaxServiceKeysPolicy).to receive(:new).with(service_key, 1, space_quota, :service_keys_space_quota_exceeded).and_return(space_policy)
expect(MaxServiceKeysPolicy).to receive(:new).with(service_key, existing_service_key_count(1), space_quota, :service_keys_space_quota_exceeded).
and_return(space_policy)
expect(space_policy).to receive(:validate)

service_key.valid?
Expand Down