ManageIQ / manageiq

ManageIQ Open-Source Management Platform
https://manageiq.org
Apache License 2.0
1.35k stars 900 forks source link

Avoid scope query when preloading #23277

Closed kbrock closed 3 days ago

kbrock commented 4 days ago

MiqPreloader.preload(...,...,scope) correctly identifies the scope coming in and passes off to the preloader

This prevents an extra (big) query from being performed


When we upgraded from rails 6.1 to 7.0, the preloader interface changed. We spent most of our efforts on how that affected virtual attributes, but there was one change here.

We only use preload with a scope in one spot - for preloading VimPerformanceState. Joe had documented all the places in the preloader test where the number of records loaded were different. The very name of the variable scope suggests that this code change is correct and should not point to records

This change gets those test values back to where they need to be for all but one (?) of the cases. But of interest, we have a lot of tests talking about arrays instead of scopes. Now in use, we only ever pass scopes. But the tests were (probably) introduced when I was trying to treat that scope as a preloaded records implementation. Never got it working correctly and ended up reverting to preload_with_array instead.

I'm thinking we should remove these preload with an array tests since this method was only ever written for passing a scope that was later applied to the records

EDIT:

Performance Testing

With a small database with 328 Vms and 380,000 vim performance states, passing a vim performance states scope to MiqPreloader.preload:

Before

irb(main):001>  require 'benchmark'; vms = Vm.take(5).to_a; Benchmark.ms { MiqPreloader.preload(vms, :vim_performance_states, VimPerformanceState.all); vms.each {|v| v.vim_performance_states.to_a}; nil }
PostgreSQLAdapter#log_after_checkout, connection_pool: size: 15, connections: 1, in use: 1, waiting_in_queue: 0
  TRANSACTION (0.1ms)  BEGIN
  Vm Load (6.9ms)  SELECT "vms".* FROM "vms" WHERE "vms"."type" IN ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25) AND "vms"."template" = $26 LIMIT $27  [["type", "Vm"], ["type", "VmServer"], ["type", "ManageIQ::Providers::PhysicalInfraManager::Vm"], ["type", "ManageIQ::Providers::InfraManager::Vm"], ["type", "ManageIQ::Providers::CloudManager::Vm"], ["type", "ManageIQ::Providers::CiscoIntersight::PhysicalInfraManager::Vm"], ["type", "ManageIQ::Providers::Vmware::InfraManager::Vm"], ["type", "ManageIQ::Providers::Ovirt::InfraManager::Vm"], ["type", "ManageIQ::Providers::Kubevirt::InfraManager::Vm"], ["type", "ManageIQ::Providers::IbmPowerHmc::InfraManager::Vm"], ["type", "ManageIQ::Providers::Redhat::InfraManager::Vm"], ["type", "ManageIQ::Providers::Openshift::InfraManager::Vm"], ["type", "ManageIQ::Providers::IbmPowerHmc::InfraManager::Vios"], ["type", "ManageIQ::Providers::IbmPowerHmc::InfraManager::Lpar"], ["type", "ManageIQ::Providers::Vmware::CloudManager::Vm"], ["type", "ManageIQ::Providers::OracleCloud::CloudManager::Vm"], ["type", "ManageIQ::Providers::Openstack::CloudManager::Vm"], ["type", "ManageIQ::Providers::IbmCloud::VPC::CloudManager::Vm"], ["type", "ManageIQ::Providers::IbmCloud::PowerVirtualServers::CloudManager::Vm"], ["type", "ManageIQ::Providers::Google::CloudManager::Vm"], ["type", "ManageIQ::Providers::AzureStack::CloudManager::Vm"], ["type", "ManageIQ::Providers::Azure::CloudManager::Vm"], ["type", "ManageIQ::Providers::Amazon::CloudManager::Vm"], ["type", "ManageIQ::Providers::IbmPowerVc::CloudManager::Vm"], ["type", "ManageIQ::Providers::IbmCic::CloudManager::Vm"], ["template", false], ["LIMIT", 5]]
  Vm Inst Including Associations (270.6ms - 5rows)
  VimPerformanceState Load (544.5ms)  SELECT "vim_performance_states".* FROM "vim_performance_states"
  VimPerformanceState Inst Including Associations (1086.7ms - 384307rows)
  VimPerformanceState Load (36.1ms)  SELECT "vim_performance_states".* FROM "vim_performance_states" WHERE "vim_performance_states"."resource_type" = $1 AND "vim_performance_states"."resource_id" IN ($2, $3, $4, $5, $6)  [["resource_type", "VmOrTemplate"], ["resource_id", 91], ["resource_id", 2518], ["resource_id", 56], ["resource_id", 940], ["resource_id", 946]]
  VimPerformanceState Inst Including Associations (49.0ms - 5482rows)
=> 3689.1060000052676

After

irb(main):003> require 'benchmark'; vms = Vm.take(5).to_a; Benchmark.ms { MiqPreloader.preload(vms, :vim_performance_states, VimPerformanceState.all); vms.each {|v| v.vim_performance_states.to_a}; nil }
  Vm Load (4.9ms)  SELECT "vms".* FROM "vms" WHERE "vms"."type" IN ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25) AND "vms"."template" = $26 LIMIT $27  [["type", "Vm"], ["type", "VmServer"], ["type", "ManageIQ::Providers::PhysicalInfraManager::Vm"], ["type", "ManageIQ::Providers::InfraManager::Vm"], ["type", "ManageIQ::Providers::CloudManager::Vm"], ["type", "ManageIQ::Providers::CiscoIntersight::PhysicalInfraManager::Vm"], ["type", "ManageIQ::Providers::Vmware::InfraManager::Vm"], ["type", "ManageIQ::Providers::Ovirt::InfraManager::Vm"], ["type", "ManageIQ::Providers::Kubevirt::InfraManager::Vm"], ["type", "ManageIQ::Providers::IbmPowerHmc::InfraManager::Vm"], ["type", "ManageIQ::Providers::Redhat::InfraManager::Vm"], ["type", "ManageIQ::Providers::Openshift::InfraManager::Vm"], ["type", "ManageIQ::Providers::IbmPowerHmc::InfraManager::Vios"], ["type", "ManageIQ::Providers::IbmPowerHmc::InfraManager::Lpar"], ["type", "ManageIQ::Providers::Vmware::CloudManager::Vm"], ["type", "ManageIQ::Providers::OracleCloud::CloudManager::Vm"], ["type", "ManageIQ::Providers::Openstack::CloudManager::Vm"], ["type", "ManageIQ::Providers::IbmCloud::VPC::CloudManager::Vm"], ["type", "ManageIQ::Providers::IbmCloud::PowerVirtualServers::CloudManager::Vm"], ["type", "ManageIQ::Providers::Google::CloudManager::Vm"], ["type", "ManageIQ::Providers::AzureStack::CloudManager::Vm"], ["type", "ManageIQ::Providers::Azure::CloudManager::Vm"], ["type", "ManageIQ::Providers::Amazon::CloudManager::Vm"], ["type", "ManageIQ::Providers::IbmPowerVc::CloudManager::Vm"], ["type", "ManageIQ::Providers::IbmCic::CloudManager::Vm"], ["template", false], ["LIMIT", 5]]
  Vm Inst Including Associations (0.4ms - 5rows)
  VimPerformanceState Load (26.8ms)  SELECT "vim_performance_states".* FROM "vim_performance_states" WHERE "vim_performance_states"."resource_type" = $1 AND "vim_performance_states"."resource_id" IN ($2, $3, $4, $5, $6)  [["resource_type", "VmOrTemplate"], ["resource_id", 91], ["resource_id", 2518], ["resource_id", 56], ["resource_id", 940], ["resource_id", 946]]
  VimPerformanceState Inst Including Associations (40.3ms - 5482rows)
=> 97.32899998198263
kbrock commented 4 days ago

@miq-bot cross-repo-test /all

Fryguy commented 3 days ago

Backported to radjabov in commit a582ab53ca80ab23cd21b9b049c895ea7173a745.

commit a582ab53ca80ab23cd21b9b049c895ea7173a745
Author: Jason Frey <fryguy9@gmail.com>
Date:   Fri Nov 22 10:40:44 2024 -0500

    Merge pull request #23277 from kbrock/preload_associations

    Avoid scope query when preloading

    (cherry picked from commit 82d5b6230b1f9a59801104ce6d07dbe517ca647d)