Skip to content

Commit 2ad49d3

Browse files
committed
Preload nested RBAC associations to reduce N+1 queries
API requests with nested RBAC associations (hardware.host.name, ext_management_system.name, storage.name, ems_cluster.name) were causing N+1 queries. Removed RBAC check in 'determine_include_for_find' to allow eager loading of nested RBAC associations. Previously these were skipped, causing a query for each VM or similar collection. Now they're eager-loaded and RBAC re-filters in bulk. For example, in a specific database setup with ~1000 vms returned, and the following API request: http://localhost:3000/api/vms?expand=resources&attributes=name,hardware.host.name,ext_management_system.name,storage.name The query count looks like this: Before: Completed 200 OK in 4530ms (Views: 0.3ms | ActiveRecord: 1401.9ms (2207 queries, 1152 cached) | GC: 479.2ms) After: Completed 200 OK in 3168ms (Views: 0.3ms | ActiveRecord: 541.5ms (787 queries, 763 cached) | GC: 461.6ms) Reduction of queries from 2207 to 787 (64%). Fixes #1321
1 parent aab2492 commit 2ad49d3

2 files changed

Lines changed: 63 additions & 6 deletions

File tree

app/controllers/api/base_controller/renderer.rb

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -611,7 +611,7 @@ def determine_include_for_find(klass)
611611
if klass.virtual_includes(attr_name) && !klass.attribute_supported_by_sql?(attr_name) && attr_base.blank?
612612
attr_name
613613
# Case 2: Direct association (e.g., "snapshots", "storage", "ems_cluster")
614-
# Not dot notation, check if the requested attribute is a direct association and include it
614+
# Eager load associations (RBAC participating or not) to reduce N+1 queries.
615615
elsif attr_base.blank?
616616
reflection = klass.reflect_on_association(attr_name.to_sym)
617617
if reflection && [:has_many, :has_one, :has_and_belongs_to_many, :belongs_to].include?(reflection.macro)
@@ -620,11 +620,10 @@ def determine_include_for_find(klass)
620620
next
621621
end
622622
# Case 3: Nested attribute (e.g., "hardware.host.name")
623-
# Include the base path for eager loading with specific exceptions
623+
# Eager load nested associations (RBAC participating or not) to reduce N+1 queries.
624+
# Exception: custom virtual_attribute_accessor (handled via accessor).
624625
else
625626
next if virtual_attribute_accessor(type, attr_name)
626-
next if attr_base_uses_rbac?(attr_base)
627-
628627
attr_base
629628
end
630629
end

spec/requests/vms_spec.rb

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,6 @@ def query_match_regexp(*tables)
369369
end
370370

371371
context "Vm index with nested indirect virtual attribute that participates in Rbac ('hardware.host.name')" do
372-
# Can't have `expect(Rbac).to receive(:filtered_object).never` in this block
373372
before do
374373
# Preload records
375374
_vms = [vm, vm1, vm2, vm_openstack, vm_openstack1, vm_openstack2]
@@ -383,12 +382,17 @@ def query_match_regexp(*tables)
383382
api_basic_authorize action_identifier(:vms, :read, :resource_actions, :get)
384383
query_match = query_match_regexp("vms", "hardwares", "hosts")
385384

385+
# After fix: 9 queries (3 base + 6 RBAC re-filtering for 6 VMs)
386+
# Before fix: was 21 queries (N+1 for each host access without preloading)
387+
# Note: RBAC associations are preloaded (visible in LEFT OUTER JOIN) but
388+
# ManageIQ core RBAC re-filters them when accessed, causing additional queries.
389+
# This is still better than the original N+1 (21 queries).
386390
expect {
387391
get api_vms_url, :params => {
388392
:expand => "resources",
389393
:attributes => "hardware.host.name,name"
390394
}
391-
}.to make_database_queries(:count => 21, :matching => query_match)
395+
}.to make_database_queries(:count => 9, :matching => query_match)
392396

393397
expected = {
394398
"resources" => a_collection_including(
@@ -415,6 +419,60 @@ def query_match_regexp(*tables)
415419
end
416420
end
417421

422+
# Query count expectations for eager loading RBAC associations:
423+
# - RBAC associations (host, ems, cluster, storage) are preloaded via LEFT OUTER JOINs
424+
# - ManageIQ core RBAC re-filters associations when accessed, causing additional queries
425+
# - Query count: ~15 queries (base queries + RBAC re-filtering)
426+
# - Without preloading: Would be ~20+ queries per VM (N+1 pattern)
427+
context "eager loads RBAC-participating associations" do
428+
before { add_hardware_and_os_to_vms }
429+
430+
it "host" do
431+
api_basic_authorize action_identifier(:vms, :read, :resource_actions, :get)
432+
query_match = query_match_regexp("vms", "hosts")
433+
434+
expect do
435+
get api_vms_url, :params => {:expand => "resources", :attributes => "host"}
436+
end.to make_database_queries(:count => be <= 15, :matching => query_match)
437+
end
438+
439+
it "ext_management_system" do
440+
api_basic_authorize action_identifier(:vms, :read, :resource_actions, :get)
441+
query_match = query_match_regexp("vms", "ext_management_systems")
442+
443+
expect do
444+
get api_vms_url, :params => {:expand => "resources", :attributes => "ext_management_system"}
445+
end.to make_database_queries(:count => be <= 15, :matching => query_match)
446+
end
447+
448+
it "ems_cluster" do
449+
api_basic_authorize action_identifier(:vms, :read, :resource_actions, :get)
450+
query_match = query_match_regexp("vms", "ems_clusters")
451+
452+
expect do
453+
get api_vms_url, :params => {:expand => "resources", :attributes => "ems_cluster"}
454+
end.to make_database_queries(:count => be <= 15, :matching => query_match)
455+
end
456+
457+
it "storage" do
458+
api_basic_authorize action_identifier(:vms, :read, :resource_actions, :get)
459+
query_match = query_match_regexp("vms", "storages")
460+
461+
expect do
462+
get api_vms_url, :params => {:expand => "resources", :attributes => "storage"}
463+
end.to make_database_queries(:count => be <= 15, :matching => query_match)
464+
end
465+
466+
it "multiple RBAC associations" do
467+
api_basic_authorize action_identifier(:vms, :read, :resource_actions, :get)
468+
query_match = query_match_regexp("vms", "hosts", "ext_management_systems", "ems_clusters", "storages")
469+
470+
expect do
471+
get api_vms_url, :params => {:expand => "resources", :attributes => "host,ext_management_system,ems_cluster,storage"}
472+
end.to make_database_queries(:count => be <= 15, :matching => query_match)
473+
end
474+
end
475+
418476
context 'Vm edit' do
419477
let(:new_vms) { FactoryBot.create_list(:vm_openstack, 2) }
420478

0 commit comments

Comments
 (0)