Skip to content

Commit 6d1bb45

Browse files
authored
Reduce db calls when presenting processes (#4889)
Eager load apps, buildpack_lifecycle_data, and cnb_lifecycle_data. This data is needed to determine if the process belongs to a docker app to then either get the run_action_user from the execution_metadata or return a default user name. Also remove the check if a droplet belongs to a docker app in 'docker_user'. This issues one or two db requests (check if buildpack_lifecycle_data or cnb_lifecycle_data exists) and it is only called from process_model and task_model (docker_run_action_user) after it has already been verified that it's a docker app.
1 parent 2e472ea commit 6d1bb45

File tree

5 files changed

+26
-10
lines changed

5 files changed

+26
-10
lines changed

app/models/runtime/droplet_model.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,6 @@ def docker_ports
126126
end
127127

128128
def docker_user
129-
return '' unless docker?
130-
131129
container_user = ''
132130
if execution_metadata.present?
133131
begin

app/presenters/v3/process_presenter.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ module V3
88
class ProcessPresenter < BasePresenter
99
include VCAP::CloudController::Presenters::Mixins::MetadataPresentationHelpers
1010

11+
class << self
12+
# :labels and :annotations come from MetadataPresentationHelpers
13+
def associated_resources
14+
super + [{ app: %i[buildpack_lifecycle_data cnb_lifecycle_data] }]
15+
end
16+
end
17+
1118
def to_hash
1219
health_check_data = { timeout: process.health_check_timeout, invocation_timeout: process.health_check_invocation_timeout, interval: process.health_check_interval }
1320
health_check_data[:endpoint] = process.health_check_http_endpoint if process.health_check_type == HealthCheckTypes::HTTP

spec/request/processes_spec.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,23 @@
8383
allow(instances_reporters).to receive(:instances_for_processes).and_return(instances_for_processes)
8484
end
8585

86+
context 'eager loading' do
87+
let(:cnb_app) { VCAP::CloudController::AppModel.make(:cnb, space:) }
88+
let!(:cnb_process_1) { VCAP::CloudController::ProcessModel.make(:cnb, app: cnb_app) }
89+
let!(:cnb_process_2) { VCAP::CloudController::ProcessModel.make(:cnb, app: cnb_app) }
90+
let(:get_processes) { -> { get '/v3/processes', nil, developer_headers } }
91+
92+
it 'eager loads associated data needed to present processes' do
93+
expect { get_processes.call }.to have_queried_db_times(/SELECT .* FROM .processes. /i, 1)
94+
expect(last_response.status).to eq(200)
95+
expect(parsed_response['resources'].count).to eq(4)
96+
97+
expect { get_processes.call }.to have_queried_db_times(/SELECT .* FROM .apps. /i, 1) # instead of 4 w/o eager loading
98+
expect { get_processes.call }.to have_queried_db_times(/SELECT .* FROM .buildpack_lifecycle_data. /i, 1) # instead of 4 w/o eager loading
99+
expect { get_processes.call }.to have_queried_db_times(/SELECT .* FROM .cnb_lifecycle_data. /i, 1) # instead of 2 w/o eager loading
100+
end
101+
end
102+
86103
it_behaves_like 'list query endpoint' do
87104
let(:message) { VCAP::CloudController::ProcessesListMessage }
88105
let(:request) { '/v3/processes' }

spec/unit/controllers/v3/processes_controller_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
it 'eager loads associated resources that the presenter specifies' do
1818
expect(VCAP::CloudController::ProcessListFetcher).to receive(:fetch_for_app).with(
1919
an_instance_of(VCAP::CloudController::ProcessesListMessage),
20-
hash_including(eager_loaded_associations: %i[labels annotations])
20+
hash_including(eager_loaded_associations: [:labels, :annotations, { app: %i[buildpack_lifecycle_data cnb_lifecycle_data] }])
2121
).and_call_original
2222

2323
get :index, params: { app_guid: app.guid }
@@ -103,7 +103,7 @@
103103
it 'eager loads associated resources that the presenter specifies' do
104104
expect(VCAP::CloudController::ProcessListFetcher).to receive(:fetch_all).with(
105105
an_instance_of(VCAP::CloudController::ProcessesListMessage),
106-
hash_including(eager_loaded_associations: %i[labels annotations])
106+
hash_including(eager_loaded_associations: [:labels, :annotations, { app: %i[buildpack_lifecycle_data cnb_lifecycle_data] }])
107107
).and_call_original
108108

109109
get :index

spec/unit/models/runtime/droplet_model_spec.rb

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -349,12 +349,6 @@ module VCAP::CloudController
349349
describe '#docker_user' do
350350
let(:droplet_model) { DropletModel.make }
351351

352-
context 'when the droplet DOES NOT belong to a Docker lifecycle app' do
353-
it 'returns an empty string' do
354-
expect(droplet_model.docker_user).to eq('')
355-
end
356-
end
357-
358352
context 'when the droplet belongs to a Docker lifecycle app' do
359353
let(:droplet_execution_metadata) { '{"entrypoint":["/image-entrypoint.sh"],"user":"cnb"}' }
360354
let(:droplet_model) { DropletModel.make(:docker, execution_metadata: droplet_execution_metadata) }

0 commit comments

Comments
 (0)