Skip to content
This repository has been archived by the owner on Jul 25, 2023. It is now read-only.

Load Lans efficiently in ProvisionWorkflow #73

Merged

Conversation

NickLaMuro
Copy link
Member

@NickLaMuro NickLaMuro commented May 3, 2018

Instead of preloading Lan records, splitting them up based on if they are shareable and instantiating entire ActiveRecord instances to only use the name column in a hash, use a specific query to gather and filter those values and generate a hash from that.

IMPORTANT NOTE: There will be a PR on ManageIQ/manageiq that will remove the preloading in app/models/miq_provision_virt_workflow.rb that this relied on to avoid an N+1. The idea is that we would merge any provider PRs first, and then the fix in the main repo so the performance impact during this is minimal.

Benchmarks

Since this is part of a multi-PR fix, benchmarks will be displayed in the main PR in the ManageIQ/manageiq repo. Links to the related PRs will be posted down below when available.

Links

Instead of preloading Lan records, splitting them up based on if they
are shareable and instantiating entire ActiveRecord instances to only
use the name column in a hash, use a specific query to gather and filter
those values and generate a hash from that.
@miq-bot
Copy link
Member

miq-bot commented May 3, 2018

Checked commit NickLaMuro@3dc4b58 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. ⭐

@agrare agrare self-assigned this May 3, 2018
Copy link
Contributor

@syncrou syncrou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NickLaMuro - LGTM

@agrare agrare merged commit 3dc4b58 into ManageIQ:master May 18, 2018
agrare added a commit that referenced this pull request May 18, 2018
…_workflow

Load Lans efficiently in ProvisionWorkflow
@agrare agrare added this to the Sprint 86 Ending May 21, 2018 milestone May 18, 2018
@NickLaMuro
Copy link
Member Author

@miq-bot add_label performance, fine/yes, gaprindashvili/yes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants