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

Skip relationship query when we know there are none #14480

Merged
merged 1 commit into from
Mar 27, 2017

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Mar 23, 2017

# when parent_rel_ids = []

def parent_rels
  Relationships.where(:id => parent_rel_ids)
end

# => SELECT * from "relationships" WHERE 1=0

When parent_rel_ids is an empty array, we perform a no-op query. This PR removes this extra query.

numbers

ms bytes objects queries query (ms) rows comments
3,418.4 77,412,641* 4,659,232 81 363.0 2,941 before
3,305.5 76,674,581* 4,375,785 67 409.2 2,941 after
3% n/a 6% 17% n/a 0 diff

* Memory usage does not reflect 1,550,176 freed objects.
* Memory usage does not reflect 1,274,302 freed objects.

analysis

Typically, the widget report "Host Summary for VMs", shows a number of skipped queries. But results are data dependent, and some data sets show the same query count for the before and after cases.

Since only non-result queries are skipped, row counts will be identical and timings will only show modest savings. (runtime variances actually showed this query time as higher)

Copy link
Member

@isimluk isimluk left a comment

Choose a reason for hiding this comment

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

👍

A majority of the time, parent_rel_ids is an array. So it is easy enough
to get the size.
If parent_rel_ids is an empty array, then essentially we're running
Relationship.where(:id => [])

Which will return no records. Rails agrees as it produces
a query with "WHERE 1=0"

This skips these queries. The savings is variable.
One customer's database returned no savings, the other went from
81 queries to 67 (17%)

Tests were in place for parents and no parents
@kbrock kbrock force-pushed the relationships_none branch from ca9d088 to ed76942 Compare March 24, 2017 12:37
@kbrock
Copy link
Member Author

kbrock commented Mar 24, 2017

fixed cop.

@miq-bot
Copy link
Member

miq-bot commented Mar 24, 2017

Checked commit kbrock@ed76942 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. 🏆

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

👍

@gtanzillo gtanzillo added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 27, 2017
@gtanzillo gtanzillo merged commit 9f3b840 into ManageIQ:master Mar 27, 2017
@kbrock kbrock deleted the relationships_none branch March 27, 2017 19:45
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.

4 participants