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

Avoid scope query when preloading #23277

Merged
merged 1 commit into from
Nov 22, 2024
Merged

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Nov 21, 2024

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 kbrock requested a review from Fryguy as a code owner November 21, 2024 22:48
@kbrock
Copy link
Member Author

kbrock commented Nov 21, 2024

@miq-bot cross-repo-test /all

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 21, 2024
MiqPreloader.preload(...,...,scope) correctly identifies the scope coming in
and passes off to the preloader

This prevents an extra (big) query from being performed
@kbrock kbrock force-pushed the preload_associations branch from 5cf8c1c to 0da1cae Compare November 22, 2024 14:52
@Fryguy Fryguy self-assigned this Nov 22, 2024
@kbrock kbrock changed the title Preload Scopes Don't run scope query for preloader Nov 22, 2024
@kbrock kbrock changed the title Don't run scope query for preloader Avoid scope query when preloading Nov 22, 2024
@Fryguy Fryguy merged commit 82d5b62 into ManageIQ:master Nov 22, 2024
8 checks passed
@Fryguy
Copy link
Member

Fryguy commented Nov 22, 2024

Backported to radjabov in commit a582ab5.

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)

Fryguy added a commit that referenced this pull request Nov 22, 2024
Avoid scope query when preloading

(cherry picked from commit 82d5b62)
@kbrock kbrock deleted the preload_associations branch November 22, 2024 21:41
jrafanie added a commit to jrafanie/manageiq-rpm_build that referenced this pull request Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants