On 23/10/09 20:32, Luke Kanies wrote: > On Oct 22, 2009, at 11:13 AM, Brice Figureau wrote: > >> Hi, >> >> Please disregard the previous version of this patch which was plain >> wrong. >> Mainly due to the fact that I thought ":include" was a synonim of >> ":joins" >> in Active Record parlance. >> This one is now correct and should generate the correct query. >> >> Marc: can you try this one and report success/failure? >> >> Please review and comment, >> >> Thanks, >> Brice >> >> Original Commit Msg: >> >> f9516d introduced a change in the way the user tags are persisted >> to the database: user tags are now treated as regular tags (they are >> stored to the tags table). >> Thus this commit changed the AR collector request to also look at the >> tags tables when collecting. >> >> Unfortunately this added a performance regression since tag request >> were still importing the resources parameters tables and AR was >> issuing a large request which was returning all the resource >> parameters >> joined with the tags. >> >> This commit fixes the AR request to join to the needed table, instead >> of doing an include. Including (ie eager loading) parameter values was >> not working for resource parameters anyway since at least 0.24 because >> searching by parameter add a constraint to the joins and only the >> searched parameter was returned instead of all parameter for a given >> exported resource. So on a performance standpoint this new code should >> be as fast 0.24 was. >> >> Signed-off-by: Brice Figureau <[email protected]> >> --- >> lib/puppet/parser/collector.rb | 22 ++++++++++++++-------- >> spec/unit/parser/collector.rb | 29 ++++++++++++++++++++--------- >> 2 files changed, 34 insertions(+), 17 deletions(-) >> >> diff --git a/lib/puppet/parser/collector.rb b/lib/puppet/parser/ >> collector.rb >> index a7f81b4..a6763c4 100644 >> --- a/lib/puppet/parser/collector.rb >> +++ b/lib/puppet/parser/collector.rb >> @@ -102,18 +102,24 @@ class Puppet::Parser::Collector >> raise Puppet::DevError, "Cannot collect resources for a nil >> host" unless @scope.host >> host = Puppet::Rails::Host.find_by_name(@scope.host) >> >> - query = {:include => {:param_values => :param_name}} >> - >> search = "(exported=? AND restype=?)" >> values = [true, @type] >> >> search += " AND (%s)" % @equery if @equery >> >> - # this is a small performance optimisation >> - # the tag mechanism involves 3 more joins, which are >> - # needed only if we query on tags. >> - if search =~ /puppet_tags/ >> - query[:include][:puppet_tags] = :resource_tags >> + # note: >> + # we're not eagerly including any relations here because >> + # it can creates so much objects we'll throw out later. >> + # We used to eagerly include param_names/values but the way >> + # the search filter is built ruined those efforts and we >> + # were eagerly loading only the searched parameter and not >> + # the other ones. >> + query = {} >> + case search >> + when /puppet_tags/ >> + query = {:joins => {:resource_tags => :puppet_tag}} >> + when /param_name/ >> + query = {:joins => {:param_values => :param_name}} >> end >> >> # We're going to collect objects from rails, but we don't >> want any >> @@ -139,7 +145,7 @@ class Puppet::Parser::Collector >> # and such, we'll need to vary the conditions, but this >> works with no >> # attributes, anyway. >> time = Puppet::Util.thinmark do >> - Puppet::Rails::Resource.find(:all, @type, true, >> query).each do |obj| >> + Puppet::Rails::Resource.find(:all, query).each do |obj| > > What happened to the use of @type and 'exported' to reduce the number > of resources returned? Won't this return all resources, regardless of > whether they're exported or the right type?
I didn't get why they were there since they were already in the :conditions block of 'query' (see the hunk above this one or the current code), so I removed them. AFAIK the queries still look OK and still return only exported resources. -- Brice Figureau My Blog: http://www.masterzen.fr/ --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
