Skip to content
This repository was archived by the owner on Jun 2, 2021. It is now read-only.

Commit 036ec09

Browse files
blgmFelisiaM
andauthored
v3(services): eager load for instances endpoint (cloudfoundry#1955)
* v3(services): eager load for instances endpoint Increases performance by about 20% for the following test: ```ruby context 'given many instances' do let!(:instances) { Array.new(1000) { VCAP::CloudController::ManagedServiceInstance.make } } it 'does it fast' do start = Time.now get '/v3/service_instances?per_page=5000', nil, admin_headers expect(last_response).to have_status_code(200) p "duration: #{Time.now - start}" end end ``` [#175715881](https://www.pivotaltracker.com/story/show/175715881) * Fixed formatting Co-authored-by: Felisia Martini <fmartini@pivotal.io>
1 parent 89eb8e3 commit 036ec09

5 files changed

Lines changed: 42 additions & 4 deletions

File tree

app/controllers/v3/service_instances_controller.rb

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,17 @@ def index
3636
invalid_param!(message.errors.full_messages) unless message.valid?
3737

3838
dataset = if permission_queryer.can_read_globally?
39-
ServiceInstanceListFetcher.fetch(message, omniscient: true)
39+
ServiceInstanceListFetcher.fetch(
40+
message,
41+
eager_loaded_associations: Presenters::V3::ServiceInstancePresenter.associated_resources,
42+
omniscient: true,
43+
)
4044
else
41-
ServiceInstanceListFetcher.fetch(message, readable_space_guids: permission_queryer.readable_space_guids)
45+
ServiceInstanceListFetcher.fetch(
46+
message,
47+
eager_loaded_associations: Presenters::V3::ServiceInstancePresenter.associated_resources,
48+
readable_space_guids: permission_queryer.readable_space_guids,
49+
)
4250
end
4351

4452
render status: :ok, json: Presenters::V3::PaginatedListPresenter.new(

app/fetchers/service_instance_list_fetcher.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
module VCAP::CloudController
55
class ServiceInstanceListFetcher < BaseListFetcher
66
class << self
7-
def fetch(message, omniscient: false, readable_space_guids: [])
8-
dataset = ServiceInstance.dataset.
7+
def fetch(message, omniscient: false, readable_space_guids: [], eager_loaded_associations: [])
8+
dataset = ServiceInstance.dataset.eager(eager_loaded_associations).
99
join(:spaces, id: Sequel[:service_instances][:space_id]).
1010
left_join(:service_instance_shares, service_instance_guid: Sequel[:service_instances][:guid])
1111

app/presenters/v3/service_instance_presenter.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,17 @@ module V3
77
class ServiceInstancePresenter < BasePresenter
88
include VCAP::CloudController::Presenters::Mixins::MetadataPresentationHelpers
99

10+
class << self
11+
# :labels and :annotations come from MetadataPresentationHelpers
12+
def associated_resources
13+
super + [
14+
:space,
15+
:service_instance_operation,
16+
:service_plan_sti_eager_load,
17+
]
18+
end
19+
end
20+
1021
def to_hash
1122
hash = correct_order(
1223
hash_common.deep_merge(

spec/request/service_instances_spec.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,18 @@ def check_filtered_instances(*instances)
472472
end
473473
end
474474

475+
describe 'eager loading' do
476+
it 'eager loads associated resources that the presenter specifies' do
477+
expect(VCAP::CloudController::ServiceInstanceListFetcher).to receive(:fetch).with(
478+
an_instance_of(VCAP::CloudController::ServiceInstancesListMessage),
479+
hash_including(eager_loaded_associations: [:labels, :annotations, :space, :service_instance_operation, :service_plan_sti_eager_load])
480+
).and_call_original
481+
482+
get '/v3/service_instances', nil, admin_headers
483+
expect(last_response).to have_status_code(200)
484+
end
485+
end
486+
475487
it_behaves_like 'list_endpoint_with_common_filters' do
476488
let(:resource_klass) { VCAP::CloudController::ServiceInstance }
477489
let(:api_call) do

spec/unit/fetchers/service_instance_list_fetcher_spec.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ module VCAP::CloudController
4040
expect(dataset.all).to contain_exactly(msi_2, ssi)
4141
end
4242

43+
it 'eager loads the specified resources for the processes' do
44+
dataset = fetcher.fetch(message, omniscient: true, eager_loaded_associations: [:labels])
45+
46+
expect(dataset.all.first.associations.key?(:labels)).to be true
47+
expect(dataset.all.first.associations.key?(:annotations)).to be false
48+
end
49+
4350
context 'filtering' do
4451
context 'by names' do
4552
let(:filters) { { names: [msi_1.name, ssi.name, 'no-such-name'] } }

0 commit comments

Comments
 (0)