[POC][WIP] Make use of platform column to further increase the speed of #allowed_templates #18354

NickLaMuro · 2019-01-11T19:29:31Z

REQUIRES ManageIQ/manageiq-schema#322 TO FUNCTION PROPERLY

(Will also require a pre-requisite PR to properly add before save hooks to models)

By adding a few key virtual_attributes, we are able to process over 100k records for MiqProvisionVirtWorkflow#allowed_templates in under 5 seconds, and use very limited memory. There are still a few things required to make this work in all cases, but the main changes required after the update include:

Make use of virtual_delegate :platform to avoid preloading of OperatingSystem records for VmOrTemplate records.
Since models are not used, switch to pulling back records as hashes, and make use of those results for creating MiqHashStruct results

Benchmarks

Note, metrics are taken only around the #allowed_templates method, so extra memory and time to process the rest of the request is not currently being shown.

PR #18353 Improvements

Memory: ~1.2Gigs

ms	queries	query (ms)	rows
27107	33	1958.5	243133
26803	33	1944.2	243133
27642	33	1965.5	243133

After switching to virtual_delegate :platform in SELECT

Memory: ~900MB

ms	queries	query (ms)	rows
14344	33	1759.6	243133
14631	33	1729.0	243133
13405	33	1752.3	243133

After skipping ActiveRecord objects

Memory: ~400MB

ms	queries	query (ms)	rows
4102	33	1751.3	243133
4124	33	1787.1	243133
4133	33	1746.9	243133

TODO

There are still a few spots that don't fully support pure hashes that need to be addressed

Links

Additional Fixes for: https://bugzilla.redhat.com/show_bug.cgi?id=1662972
Requires Schema migration: [WIP] Add cache columns to operating_systems table manageiq-schema#322
Built on top of: Performance improvements to MiqProvisionVirtWorkflow#allowed_templates #18353

Steps for Testing/QA

Just like #18353 I was using a combination of this script for testing the full routes:

$ cat script_1
ActiveRecord::Base.logger = Logger.new(STDOUT).tap {|l| l.level = Logger::INFO }

Rails.application.load_console
Rails.env = ENV["RAILS_ENV"]
include Rails::ConsoleMethods
MiqUiWorker.preload_for_console # required


app.post "/dashboard/authenticate", :params => { :user_name => "admin", :user_password => "smartvm" }
app.get  "/vm_cloud/explorer"
csrf_token = app.response.body.match(/.*csrf-token.*content="(?<CSRF_TOKEN>[^"]*)"/)[:CSRF_TOKEN]

headers = { :"X-CSRF-Token" => csrf_token }
app.post "/vm_cloud/x_button?pressed=instance_miq_request_new", :headers => headers
app.post "/miq_request/pre_prov/?sel_id=3192",                  :headers => headers
app.post "/vm_cloud/pre_prov?button=continue",                  :headers => headers
$ bin/rails r script_1

And this one, which tested MiqProvisionVirtWorkflow#allowed_templates on it's own:

$ cat script_2
user     = User.find(1)
wf_klass = ManageIQ::Providers::Amazon::CloudManager::ProvisionWorkflow
workflow = wf_klass.new({}, user, { :initial_pass => true, :src_vm_id => [1] })

dialog_options =  {
  :initial_pass            => true,
  :src_vm_id               => [ 1, "[SOME AWS PUBLIC IMAGE NAME HERE]" ],
  :miq_request_dialog_name => "miq_provision_amazon_dialogs_template",
  :customize_enabled       => [ "enabled" ],
  :current_tab_key         => :requester
}

profile do # use whatever profiler you want here
  workflow.init_from_dialog dialog_options
end
$ bin/rails r script_2

NickLaMuro · 2019-01-11T19:33:51Z

@miq-bot add_label performance

@kbrock (and maybe @Fryguy): More 🍅 🍅 🍅 for this one (and related ManageIQ/manageiq-schema#322 )

miq-bot · 2019-05-24T19:50:54Z

This pull request is not mergeable. Please rebase and repush.

kbrock

This is looking good

kbrock · 2019-05-28T18:31:05Z

app/models/miq_provision_virt_workflow.rb

+
+        # only fetch the ems if not fetched previously
+        unless @_ems_allowed_templates_cache.key?(vm_or_template[:ems_id])
+          @_ems_allowed_templates_cache[ems_id] = ExtManagementSystem.find(ems_id).try(:name)


Will we have a bunch of EMSes?

If so, would it make sense to collecting the :ems_id values first and make a single query?

I'm guessing you already looked at this and deemed it unnecessary

ugh - ignore this - we already discussed

kbrock · 2019-05-28T19:22:51Z

app/models/miq_provision_virt_workflow.rb

      end
    end

-    MiqPreloader.preload(allowed_templates_list, [:snapshots, :operating_system, :ext_management_system, {:hardware => :disks}])


yay getting rid of this

app/models/miq_provision_virt_workflow.rb

kbrock · 2019-05-28T19:28:21Z

app/models/miq_provision_virt_workflow.rb

+      if vm_or_template[:operating_system_product_name]
+        data_hash[:operating_system] = MiqHashStruct.new(:product_name => vm_or_template[:operating_system_product_name])
+      end
+      # TODO: solve owning_blue_folder for the "Hash method" (if needed...)


owning_blue_folder tends to be a performance issue in lots of places

I agree that we should not address here

lib/rbac/filterer.rb

miq-bot · 2019-12-02T15:03:35Z

This pull request has been automatically closed because it has not been updated for at least 6 months.

Feel free to reopen this pull request if these changes are still valid.

Thank you for all your contributions!

NickLaMuro · 2019-12-04T18:17:58Z

Opening this one up for now, but really not going to start much with it for now. Discussion is still going on in ManageIQ/manageiq-schema#322 for the time being.

Might rebase out the stuff that was merged with #18353 however...

Very large performance improvement from this patch on large collections of templates since it avoids preloading all together, reducing the number of iterations over the VM collection, and the number of support objects needing to be introduced (that would later need to be garbage collected. Benchmark --------- **Before** | ms | queries | query (ms) | rows | | ---: | ---: | ---: | ---: | | 27107 | 33 | 1958.5 | 243133 | | 26803 | 33 | 1944.2 | 243133 | | 27642 | 33 | 1965.5 | 243133 | **After** | ms | queries | query (ms) | rows | | ---: | ---: | ---: | ---: | | 14344 | 33 | 1759.6 | 243133 | | 14631 | 33 | 1729.0 | 243133 | | 13405 | 33 | 1752.3 | 243133 |

This significantly improves the speed of the method by: - Avoiding instantiating intermediate `ActiveRecord` objects that are just garbage collected - Reusing hash results to create the `MiqHashStruct` instantiate Benchmarks ---------- This tests against a AWS EMS with 100k+ templates (public images): **Before** | ms | queries | query (ms) | rows | | ---: | ---: | ---: | ---: | | 14344 | 33 | 1759.6 | 243133 | | 14631 | 33 | 1729.0 | 243133 | | 13405 | 33 | 1752.3 | 243133 | **After** | ms | queries | query (ms) | rows | | ---: | ---: | ---: | ---: | | 4102 | 33 | 1751.3 | 243133 | | 4124 | 33 | 1787.1 | 243133 | | 4133 | 33 | 1746.9 | 243133 |

miq-bot · 2019-12-04T18:46:11Z

Checked commits NickLaMuro/manageiq@d78171a~...fe57a37 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

miq-bot · 2020-06-11T22:35:12Z

This pull request has been automatically closed because it has not been updated for at least 3 months.

Feel free to reopen this pull request if these changes are still valid.

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the traige process documentation.

miq-bot added the wip label Jan 11, 2019

NickLaMuro mentioned this pull request Jan 11, 2019

Performance improvements to MiqProvisionVirtWorkflow#allowed_templates #18353

Merged

miq-bot added the performance label Jan 11, 2019

NickLaMuro force-pushed the miq_provision_virt_workflow_allowed_templates_as_hashes branch 2 times, most recently from a123f90 to a4ea2b0 Compare January 15, 2019 01:15

NickLaMuro changed the title ~~[WIP] Make use of platform column to further increase the speed of #allowed_templates~~ [POC][WIP] Make use of platform column to further increase the speed of #allowed_templates Jan 15, 2019

NickLaMuro mentioned this pull request Jan 16, 2019

Use OpenStruct over MiqHashStruct #17359

Closed

miq-bot added the unmergeable label May 24, 2019

kbrock reviewed May 28, 2019

View reviewed changes

miq-bot closed this Dec 2, 2019

NickLaMuro reopened this Dec 4, 2019

NickLaMuro mentioned this pull request Dec 4, 2019

[WIP] Add cache columns to operating_systems table ManageIQ/manageiq-schema#322

Open

NickLaMuro added 2 commits December 4, 2019 12:30

Add virtual_attributes for VmOrTemplate#platform

d78171a

NickLaMuro force-pushed the miq_provision_virt_workflow_allowed_templates_as_hashes branch from a4ea2b0 to fe57a37 Compare December 4, 2019 18:38

miq-bot added the stale label Jun 11, 2020

miq-bot closed this Jun 11, 2020

NickLaMuro mentioned this pull request Jun 25, 2020

Provision dialog for ec2 with public images fails - high worker memory utilization ManageIQ/manageiq-automation_engine#446

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[POC][WIP] Make use of platform column to further increase the speed of #allowed_templates #18354

[POC][WIP] Make use of platform column to further increase the speed of #allowed_templates #18354

NickLaMuro commented Jan 11, 2019 •

edited

NickLaMuro commented Jan 11, 2019

miq-bot commented May 24, 2019

kbrock left a comment

kbrock May 28, 2019

kbrock May 29, 2019

kbrock May 28, 2019

kbrock May 28, 2019 •

edited

miq-bot commented Dec 2, 2019

NickLaMuro commented Dec 4, 2019

miq-bot commented Dec 4, 2019

miq-bot commented Jun 11, 2020

[POC][WIP] Make use of platform column to further increase the speed of #allowed_templates #18354

[POC][WIP] Make use of platform column to further increase the speed of #allowed_templates #18354

Conversation

NickLaMuro commented Jan 11, 2019 • edited

Benchmarks

TODO

Links

Steps for Testing/QA

NickLaMuro commented Jan 11, 2019

miq-bot commented May 24, 2019

kbrock left a comment

Choose a reason for hiding this comment

kbrock May 28, 2019

Choose a reason for hiding this comment

kbrock May 29, 2019

Choose a reason for hiding this comment

kbrock May 28, 2019

Choose a reason for hiding this comment

kbrock May 28, 2019 • edited

Choose a reason for hiding this comment

miq-bot commented Dec 2, 2019

NickLaMuro commented Dec 4, 2019

miq-bot commented Dec 4, 2019

miq-bot commented Jun 11, 2020

NickLaMuro commented Jan 11, 2019 •

edited

kbrock May 28, 2019 •

edited