Rails is now significantly faster. I refactored all of the queries; they are mostly reduced to three queries, each of which is relatively fast, although there are still a ton of file- and tag-related queries that I cannot find the source of. Note that this speedup requires indexes, which will only get added if you start puppetmasterd with --dbmigrate (although you cannot always start with that, as there is an error in the init code). I expect that the indexes will not help unless you forcibly reindex your database, but after that you should see significant speed improvements.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2344 980ebf18-57e1-0310-9a29-db15c13687c0

5 files changed, 141 insertions, 49 deletions

diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index bdace28cd..8ef382204 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -256,40 +256,59 @@ class Puppet::Parser::Resource
     end
 
     # Turn our parser resource into a Rails resource.
-    def to_rails(host)
+    def to_rails(host, resource = nil)
         args = {}
-        %w{type title tags file line exported}.each do |param|
+        [:type, :title, :tags, :file, :line, :exported].each do |param|
+            # 'type' isn't a valid column name, so we have to use something else.
+            if param == :type
+                to = :restype
+            else
+                to = param
+            end
             if value = self.send(param)
-                args[param] = value
+                args[to] = value
             end
         end
 
-        # 'type' isn't a valid column name, so we have to use something else.
-        args = symbolize_options(args)
-        args[:restype] = args[:type]
-        args.delete(:type)
-
-        # Let's see if the object exists
-        if obj = host.resources.find_by_restype_and_title(self.type, self.title)
+        # If we were passed an object, just make sure all of the attributes are correct.
+        if resource
             # We exist
             args.each do |param, value|
-                obj[param] = value
+                unless resource[param] == value
+                    resource[param] = value
+                end
             end
         else
             # Else create it anew
-            obj = host.resources.build(args)
+            resource = host.resources.build(args)
         end
 
-        if l = self.line
-            obj.line = l
+        # Either way, now add our parameters
+        newparams = @params.dup
+        remove = []
+        resource.param_names.each do |pn|
+            name = pn.name.intern
+            if param = newparams[name]
+                # Mark that we found this in the db
+                newparams.delete(name)
+                param.to_rails(resource, pn)
+            else
+                remove << pn
+            end
         end
 
-        # Either way, now add our parameters
-        obj.collection_merge(:param_names, @params) do |name, param|
-            param.to_rails(obj)
+        newparams.each do |name, param|
+            param.to_rails(resource)
         end
 
-        return obj
+        remove.each do |param|
+            resource.param_names.delete(param)
+        end
+        #obj.collection_merge(:param_names, @params) do |name, param|
+        #    param.to_rails(obj)
+        #end
+
+        return resource
     end
 
     def to_s
diff --git a/lib/puppet/parser/resource/param.rb b/lib/puppet/parser/resource/param.rb
index 1f7a66aae..56a50de1c 100644
--- a/lib/puppet/parser/resource/param.rb
+++ b/lib/puppet/parser/resource/param.rb
@@ -16,10 +16,12 @@ class Puppet::Parser::Resource::Param
     end
 
     # Store this parameter in a Rails db.
-    def to_rails(res)
+    def to_rails(res, pn = nil)
         values = value.is_a?(Array) ? value : [value]
 
-        unless pn = res.param_names.find_by_name(self.name.to_s)
+        values = values.collect { |v| v.to_s }
+
+        unless pn
             # We're creating it anew.
             pn = res.param_names.build(:name => self.name.to_s)
         end
@@ -30,13 +32,30 @@ class Puppet::Parser::Resource::Param
             pn.line = Integer(l)
         end
 
-        pn.collection_merge(:param_values, values) do |value|
-            unless pv = pn.param_values.find_by_value(value)
-                pv = pn.param_values.build(:value => value)
+        oldvals = []
+
+        if pv = pn.param_values
+            newvals = pv.each do |val|
+                oldvals << val.value
+            end
+        end
+
+        if oldvals != values
+            #pn.param_values = values.collect { |v| pn.param_values.build(:value => v.to_s) }
+            objects = values.collect do |v|
+                pn.param_values.build(:value => v.to_s)
             end
-            pv
+            pn.param_values = objects
+            #pn.save
         end
 
+#        pn.collection_merge(:param_values, values) do |value|
+#            unless pv = pn.param_values.find_by_value(value)
+#                pv = pn.param_values.build(:value => value)
+#            end
+#            pv
+#        end
+
         return pn
     end
 
diff --git a/lib/puppet/rails/database/001_add_indexes.rb b/lib/puppet/rails/database/001_add_indexes.rb
index 456501e52..becae5ca0 100644
--- a/lib/puppet/rails/database/001_add_indexes.rb
+++ b/lib/puppet/rails/database/001_add_indexes.rb
@@ -5,8 +5,8 @@ class AddIndexes < ActiveRecord::Migration
         :puppet_classes => [:name, :host_id],
         :hosts => [:name, :ip, :updated_at],
         :fact_names => [:name, :host_id],
-        #:fact_values => [:value, :fact_name_id],
-        #:param_values => [:value, :param_name_id],
+        :fact_values => [:fact_name_id],
+        :param_values => [:param_name_id],
         :param_names => [:name, :resource_id],
         :tags => [:name, :updated_at],
         :taggings => [:tag_id, :taggable_id, :taggable_type]
diff --git a/lib/puppet/rails/host.rb b/lib/puppet/rails/host.rb
index c8e46b1a5..ac547e013 100644
--- a/lib/puppet/rails/host.rb
+++ b/lib/puppet/rails/host.rb
@@ -4,6 +4,7 @@ require 'puppet/rails/source_file'
 require 'puppet/util/rails/collection_merger'
 
 class Puppet::Rails::Host < ActiveRecord::Base
+    include Puppet::Util
     include Puppet::Util::CollectionMerger
 
     has_many :fact_values, :through => :fact_names 
@@ -34,25 +35,33 @@ class Puppet::Rails::Host < ActiveRecord::Base
 
         args = {}
 
-        unless host = find_by_name(name)
-            host = new(:name => name)
-        end
-        if ip = hash[:facts]["ipaddress"]
-            host.ip = ip
-        end
+        host = nil
+        transaction do
+            #unless host = find_by_name(name)
+            seconds = Benchmark.realtime {
+                #unless host = find_by_name(name, :include => {:resources => {:param_names => :param_values}, :fact_names => :fact_values})
+                unless host = find_by_name(name)
+                    host = new(:name => name)
+                end
+            }
+            Puppet.notice("Searched for host in %0.2f seconds" % seconds) if defined?(Puppet::TIME_DEBUG)
+            if ip = hash[:facts]["ipaddress"]
+                host.ip = ip
+            end
 
-        # Store the facts into the database.
-        host.setfacts(hash[:facts])
+            # Store the facts into the database.
+            host.setfacts(hash[:facts])
 
-        unless hash[:resources]
-            raise ArgumentError, "You must pass resources"
-        end
+            unless hash[:resources]
+                raise ArgumentError, "You must pass resources"
+            end
 
-        host.setresources(hash[:resources])
+            host.setresources(hash[:resources])
 
-        host.last_compile = Time.now
+            host.last_compile = Time.now
 
-        host.save
+            host.save
+        end
 
         return host
     end
@@ -73,23 +82,68 @@ class Puppet::Rails::Host < ActiveRecord::Base
     end
 
     def setfacts(facts)
-        collection_merge(:fact_names, facts) do |name, value|
-            fn = fact_names.find_by_name(name) || fact_names.build(:name => name)
-            # We're only ever going to have one fact value, at this point.
-            unless fv = fn.fact_values.find_by_value(value)
-                fv = fn.fact_values.build(:value => value)
+        facts = facts.dup
+        remove = []
+
+        existing = nil
+        seconds = Benchmark.realtime {
+            existing = fact_names.find(:all, :include => :fact_values)
+        }
+        Puppet.debug("Searched for facts in %0.2f seconds" % seconds) if defined?(Puppet::TIME_DEBUG)
+
+        existing.each do |fn|
+            if value = facts[fn.name]
+                facts.delete(fn.name)
+                fn.fact_values.each do |fv|
+                    unless value == fv.value
+                        fv.value = value
+                    end
+                end
+            else
+                remove << fn
             end
-            fn.fact_values = [fv]
+        end
 
-            fn
+        # Make a new fact for the rest of them
+        facts.each do |fact, value|
+            fn = fact_names.build(:name => fact)
+            fn.fact_values = [fn.fact_values.build(:value => value)]
+        end
+
+        # Now remove anything necessary.
+        remove.each do |fn|
+            fact_names.delete(fn)
         end
     end
 
     # Set our resources.
     def setresources(list)
-        collection_merge(:resources, list) do |resource|
+        compiled = {}
+        remove = []
+        existing = nil
+        seconds = Benchmark.realtime {
+            existing = resources.find(:all, :include => {:param_names => :param_values})
+        }
+        Puppet.notice("Searched for resources in %0.2f seconds" % seconds) if defined?(Puppet::TIME_DEBUG)
+        list.each do |resource|
+            compiled[resource.ref] = resource
+        end
+        existing.each do |resource|
+            if comp = compiled[resource.ref]
+                compiled.delete(comp.ref)
+                comp.to_rails(self, resource)
+            else
+                remove << resource
+            end
+        end
+
+        compiled.each do |name, resource|
             resource.to_rails(self)
         end
+
+        remove.each do |resource|
+            resources.delete(resource)
+        end
     end
 
     def update_connect_time
diff --git a/lib/puppet/rails/resource.rb b/lib/puppet/rails/resource.rb
index 6656c2450..c3bf8c38a 100644
--- a/lib/puppet/rails/resource.rb
+++ b/lib/puppet/rails/resource.rb
@@ -47,7 +47,7 @@ class Puppet::Rails::Resource < ActiveRecord::Base
     end
 
     def ref
-        "%s[%s]" % [self[:restype], self[:title]]
+        "%s[%s]" % [self[:restype].capitalize, self[:title]]
     end
 
     # Convert our object to a resource.  Do not retain whether the object