Mark or anyone running storeconfigs on 0.25: if you can, it would be great to test this patch to see if there is no regression anymore and it still works as intended.
I only tested it on a small system which didn't exhibit the issue beforehand. I also manually tested the AR generated SQL queries, and they look correct. If the testers say it's ok, I think it could be great to have this fix in 0.25.1 if it's not too late. Please review, test and comment :-) 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 system tags. 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 fix this issue by either including parameters if the collection expression acts on parameters or by including tags if the collection expression deals with tag. Signed-off-by: Brice Figureau <[email protected]> --- lib/puppet/parser/collector.rb | 12 ++++-------- spec/unit/parser/collector.rb | 13 ++++++++++++- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/puppet/parser/collector.rb b/lib/puppet/parser/collector.rb index a7f81b4..a2abb6e 100644 --- a/lib/puppet/parser/collector.rb +++ b/lib/puppet/parser/collector.rb @@ -102,19 +102,15 @@ 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 - end + # filter only by things we currently search on + query = {} + query.merge!( {:include => {:puppet_tags => :resource_tags}} ) if search =~ /puppet_tags/ + query.merge!( {:include => {:param_values => :param_name}} ) if search =~ /param_name/ # We're going to collect objects from rails, but we don't want any # objects from this host. diff --git a/spec/unit/parser/collector.rb b/spec/unit/parser/collector.rb index 926033c..53e46c3 100755 --- a/spec/unit/parser/collector.rb +++ b/spec/unit/parser/collector.rb @@ -506,6 +506,7 @@ describe Puppet::Parser::Collector, "when building its ActiveRecord query for co end it "should return parameter names, parameter values when querying ActiveRecord" do + @collector.equery = "param_names.name = title" Puppet::Rails::Resource.stubs(:find).with { |*arguments| options = arguments[3] options[:include] == {:param_values => :param_name} @@ -518,12 +519,22 @@ describe Puppet::Parser::Collector, "when building its ActiveRecord query for co @collector.equery = "puppet_tags.name = test" Puppet::Rails::Resource.stubs(:find).with { |*arguments| options = arguments[3] - options[:include] == {:param_values => :param_name, :puppet_tags => :resource_tags} + options[:include] == {:puppet_tags => :resource_tags} }.returns([...@resource]) @collector.evaluate.should == [...@resource] end + it "should not query parameters when querying ActiveRecord with a tag exported query" do + @collector.equery = "puppet_tags.name = test" + Puppet::Rails::Resource.stubs(:find).with { |*arguments| + options = arguments[3] + options[:include] == {:param_values => :param_name} + }.returns([...@resource]) + + @collector.evaluate.should be_false + end + it "should only search for exported resources with the matching type" do Puppet::Rails::Resource.stubs(:find).with { |*arguments| options = arguments[3] -- 1.6.4 --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
