Merge pull request #18331 from Fryguy/refactor_vmdb_database_seed

Refactor VmdbDatabase#seed for performance improvements

app/models/vmdb_database.rb

@@ -1,6 +1,8 @@
 class VmdbDatabase < ApplicationRecord
-  has_many :vmdb_tables,           :dependent => :destroy
+  has_many :vmdb_tables            # Destroy will be handled by seeder
   has_many :evm_tables,            :class_name => 'VmdbTableEvm'
+  has_many :text_tables,           :class_name => 'VmdbTableText'
+  has_many :vmdb_indexes,          :through => :vmdb_tables
   has_many :vmdb_database_metrics, :dependent => :destroy
   has_one  :latest_hourly_metric,  -> { where(:capture_interval_name => 'hourly').order("timestamp DESC") }, :class_name => 'VmdbDatabaseMetric'
 

app/models/vmdb_database/seeding.rb

@@ -3,24 +3,34 @@ module VmdbDatabase::Seeding
 
   module ClassMethods
     def seed
-      db = seed_self
-      db.seed
-      db
+      transaction do
+        seed_self.tap do
+          seed_tables
+          seed_indexes
+        end
+      end
     end
 
+    private
+
     def seed_self
-      db = my_database || new
-
-      db.name            = connection.current_database
-      db.vendor          = connection.adapter_name
-      db.version         = connection.database_version
-      db.ipaddress       = db_server_ipaddress
-      db.data_directory  = connection.data_directory            if connection.respond_to?(:data_directory)
-      db.last_start_time = connection.last_start_time           if connection.respond_to?(:last_start_time)
-      db.data_disk       = db_disk_size(db.data_directory) if EvmDatabase.local? && db.data_directory
-
-      db.save!
-      db
+      (my_database || new).tap do |db|
+        data_directory = connection.try(:data_directory)
+        disk_size      = db_disk_size(data_directory) if data_directory && EvmDatabase.local?
+
+        db.name            = connection.current_database
+        db.vendor          = connection.adapter_name
+        db.version         = connection.database_version
+        db.ipaddress       = db_server_ipaddress
+        db.data_directory  = data_directory
+        db.last_start_time = connection.try(:last_start_time)
+        db.data_disk       = disk_size
+
+        if db.changed?
+          _log.info("#{db.new_record? ? "Creating" : "Updating"} VmdbDatabase #{db.name.inspect}")
+          db.save!
+        end
+      end
     end
 
     def db_server_ipaddress
@@ -38,19 +48,63 @@ def db_disk_size(disk)
       return nil if err.message.include?("does not exist")
       raise
     end
-  end
 
-  def seed
-    mine = evm_tables.includes(:text_tables, :vmdb_indexes).index_by(&:name)
+    def seed_tables
+      db = my_database
+      vmdb_tables = db.vmdb_tables.index_by(&:name)
+
+      table_names = connection.tables
+      tables = table_names.zip([]).to_h
+      tables.merge!(connection.text_table_names.slice(*table_names))
+
+      tables.each do |t, tt|
+        table   = vmdb_tables.delete(t)
+        table ||= VmdbTableEvm.create!(:name => t, :vmdb_database => db) do
+          _log.info("Creating VmdbTableEvm #{t.inspect}")
+        end
+
+        next unless tt
+
+        text_table   = vmdb_tables.delete(tt)
+        text_table ||= VmdbTableText.new
 
-    self.class.connection.tables.sort.each do |table_name|
-      table = mine.delete(table_name)
-      VmdbTableEvm.seed_for_database(self, table || table_name)
+        text_table.attributes = {:name => tt, :evm_table => table, :vmdb_database => db}
+        if text_table.new_record? || text_table.changed?
+          _log.info("#{text_table.new_record? ? "Creating" : "Updating"} VmdbTableText #{tt.inspect} for VmdbTableEvm #{t.inspect}")
+          text_table.save!
+        end
+      end
+
+      if vmdb_tables.any?
+        _log.info("Deleting the following VmdbTable(s) as they no longer exist: #{vmdb_tables.keys.sort.collect(&:inspect).join(", ")}")
+        VmdbTable.delete(vmdb_tables.values.map(&:id))
+      end
     end
 
-    mine.each do |name, t|
-      _log.info("Table <#{name}> is no longer in Database <#{self.name}> - deleting")
-      t.destroy
+    def seed_indexes
+      db = my_database
+      vmdb_tables  = db.vmdb_tables.select(:id, :name).index_by(&:name)
+      vmdb_indexes = db.vmdb_indexes.index_by(&:name)
+
+      indexes = connection.index_names.slice(*vmdb_tables.keys)
+
+      indexes.each do |t, is|
+        is.each do |i|
+          index   = vmdb_indexes.delete(i)
+          index ||= VmdbIndex.new
+
+          index.attributes = {:name => i, :vmdb_table => vmdb_tables[t]}
+          if index.new_record? || index.changed?
+            _log.info("#{index.new_record? ? "Creating" : "Updating"} VmdbIndex #{i.inspect} for VmdbTable #{t.inspect}")
+            index.save!
+          end
+        end
+      end
+
+      if vmdb_indexes.any?
+        _log.info("Deleting the following VmdbIndex(es) as they no longer exist: #{vmdb_indexes.keys.sort.collect(&:inspect).join(", ")}")
+        VmdbIndex.delete(vmdb_indexes.values.map(&:id))
+      end
     end
   end
 end

app/models/vmdb_index.rb

@@ -7,7 +7,6 @@ class VmdbIndex < ApplicationRecord
 
   include VmdbDatabaseMetricsMixin
 
-  include_concern 'Seeding'
   include_concern 'VmdbIndex::MetricCapture'
 
   serialize :prior_raw_metrics

app/models/vmdb_table.rb

@@ -1,15 +1,13 @@
 class VmdbTable < ApplicationRecord
   belongs_to :vmdb_database
 
-  has_many :vmdb_indexes,                            :dependent => :destroy
+  has_many :vmdb_indexes                             # Destroy will be handled by seeder
   has_many :vmdb_metrics,          :as => :resource  # Destroy will be handled by purger
 
   has_one  :latest_hourly_metric,  -> { VmdbMetric.where(:capture_interval_name => 'hourly', :resource_type => 'VmdbTable', :timestamp => VmdbMetric.maximum(:timestamp)) }, :as => :resource, :class_name => 'VmdbMetric'
 
   include VmdbDatabaseMetricsMixin
 
-  include_concern 'Seeding'
-
   serialize :prior_raw_metrics
 
   def my_metrics

app/models/vmdb_table_evm.rb

@@ -1,13 +1,5 @@
 class VmdbTableEvm < VmdbTable
-  has_many :text_tables, :class_name => "VmdbTableText", :foreign_key => :parent_id, :dependent => :destroy
+  has_many :text_tables, :class_name => "VmdbTableText", :foreign_key => :parent_id # Destroy will be handled by seeder
 
   include_concern 'VmdbTableEvm::MetricCapture'
-  include_concern 'Seeding'
-
-  def sql_indexes
-    actual  = self.class.connection.indexes(name)
-    pk = self.class.connection.primary_key_index(name)
-    actual << pk if pk
-    actual
-  end
 end

app/models/vmdb_table_text.rb

@@ -1,12 +1,6 @@
 class VmdbTableText < VmdbTable
   belongs_to :evm_table, :class_name => "VmdbTableEvm", :foreign_key => :parent_id
 
-  include_concern 'Seeding'
-
-  def sql_indexes
-    self.class.connection.respond_to?(:text_table_indexes) ? self.class.connection.text_table_indexes(name) : []
-  end
-
   def capture_metrics
     # TODO:
   end

lib/extensions/ar_adapter/ar_dba.rb

@@ -479,71 +479,29 @@ def table_total_size(table)
     select_value("SELECT pg_total_relation_size('#{table}')").to_i
   end
 
-  def text_tables(table)
-    data = select(<<-SQL, "Text Tables")
-      SELECT relname AS table_name
-      FROM pg_class,
-           (SELECT reltoastrelid
-            FROM pg_class
-            WHERE relname = '#{table}') tt
-      WHERE oid = tt.reltoastrelid
-      ORDER BY relname
+  # @return [Hash<String,String>] a hash of {table_name => text_table_name}
+  def text_table_names
+    data = select_rows(<<-SQL, "Text Table Names")
+      SELECT t.relname AS table_name, tt.relname AS text_table_name
+      FROM pg_class t
+        JOIN pg_class tt ON t.reltoastrelid = tt.oid
     SQL
 
-    data.collect { |h| h['table_name'] }
+    data.to_h
   end
 
-  # Returns an array of toast table indexes for the given toast table.
-  def text_table_indexes(table_name, name = "Text Table Indexes")
-    result = query(<<-SQL, name)
-       SELECT distinct i.relname, d.indisunique, d.indkey, i.oid
-       FROM pg_class t
-       INNER JOIN pg_index d ON t.oid = d.indrelid
-       INNER JOIN pg_class i ON d.indexrelid = i.oid
-       WHERE i.relkind = 'i'
-         AND t.relkind = 't'
-         AND i.oid = d.indexrelid
-         AND t.relname = '#{table_name}'
-         AND i.relnamespace IN (SELECT oid FROM pg_namespace WHERE nspname = 'pg_toast' )
-      ORDER BY i.relname
+  # @return [Hash<String,Array<String>>] a hash of {table_name => [index_names]}
+  def index_names
+    data = select_rows(<<-SQL, "Index Names")
+      SELECT DISTINCT t.relname AS table_name, i.relname AS index_name
+      FROM pg_class t
+        JOIN pg_index d ON t.oid = d.indrelid
+        JOIN pg_class i ON d.indexrelid = i.oid
+      WHERE i.relkind = 'i'
     SQL
 
-    result.map do |row|
-      index_name = row[0]
-      unique     = row[1] == 't'
-      indkey     = row[2].split(" ")
-      oid        = row[3]
-
-      columns = Hash[query(<<-SQL, "Columns for index #{index_name} on #{table_name}")]
-      SELECT a.attnum, a.attname
-      FROM pg_attribute a
-      WHERE a.attrelid = #{oid}
-      AND a.attnum IN (#{indkey.join(",")})
-      SQL
-
-      column_names = columns.values_at(*indkey).compact
-      column_names.empty? ? nil : IndexDefinition.new(table_name, index_name, unique, column_names)
-    end.compact
-  end
-
-  # Returns the primary-key index definition for the given table if it exists
-  def primary_key_index(table_name)
-    result = select_all(<<-SQL).cast_values.first
-      SELECT c.relname, array_agg(a.attname)
-      FROM pg_index i
-      JOIN pg_attribute a ON
-        a.attrelid = i.indrelid AND
-        a.attnum = ANY(i.indkey)
-      JOIN pg_class c ON
-        i.indexrelid = c.oid
-      WHERE
-        i.indrelid = '#{table_name}'::regclass AND
-        i.indisprimary group by c.relname;
-    SQL
-
-    return nil unless result
-
-    ActiveRecord::ConnectionAdapters::IndexDefinition.new(table_name, result[0], true, result[1])
+    result = Hash.new { |h, k| h[k] = [] }
+    data.each_with_object(result) { |(k, v), h| h[k] << v }
   end
 
   def primary_key?(table_name)

spec/lib/extensions/ar_dba_spec.rb

@@ -13,32 +13,6 @@
     end
   end
 
-  describe "#primary_key_index" do
-    it "returns nil when there is no primary key" do
-      table_name = "no_pk_test"
-      connection.select_value("CREATE TABLE #{table_name} (id INTEGER)")
-      expect(connection.primary_key_index(table_name)).to be nil
-    end
-
-    it "returns the correct primary key" do
-      index_def = connection.primary_key_index("miq_databases")
-      expect(index_def.table).to eq("miq_databases")
-      expect(index_def.unique).to be true
-      expect(index_def.columns).to eq(["id"])
-    end
-
-    it "works with composite primary keys" do
-      table_name = "comp_pk_test"
-      connection.select_value("CREATE TABLE #{table_name} (id1 INTEGER, id2 INTEGER)")
-      connection.select_value("ALTER TABLE #{table_name} ADD PRIMARY KEY (id1, id2)")
-
-      index_def = connection.primary_key_index(table_name)
-      expect(index_def.table).to eq(table_name)
-      expect(index_def.unique).to be true
-      expect(index_def.columns).to match_array(%w(id1 id2))
-    end
-  end
-
   describe "#primary_key?" do
     it "returns false for a table without a primary key" do
       table_name = "no_pk_test"