Skip to content

Commit

Permalink
Merge pull request #14023 from Ladas/scanning_for_used_attributes_for…
Browse files Browse the repository at this point in the history 
…_query_optimizations

Scanning for used attributes for query optimizations
  • Loading branch information
@kbrock
kbrock committed Feb 23, 2017
2 parents 196a2e5 + 19e7dcc commit 8773af5
Show file tree
Hide file tree
Showing 6 changed files with 535 additions and 113 deletions.
12 changes: 12 additions & 0 deletions app/models/manager_refresh/application_record_reference.rb
View file
@@ -0,0 +1,12 @@
module ManagerRefresh
class ApplicationRecordReference
attr_reader :base_class_name, :id

# ApplicationRecord is very bloaty in memory, so this class server for storing base_class and primary key
# of the ApplicationRecord, which is just enough for filling up relationships
def initialize(base_class_name, id)
@base_class_name = base_class_name
@id = id
end
end
end
93 changes: 66 additions & 27 deletions app/models/manager_refresh/inventory_collection.rb
View file
@@ -1,6 +1,6 @@
module ManagerRefresh
class InventoryCollection
attr_accessor :saved, :references, :data_collection_finalized
attr_accessor :saved, :references, :attribute_references, :data_collection_finalized

attr_reader :model_class, :strategy, :attributes_blacklist, :attributes_whitelist, :custom_save_block, :parent,
:internal_attributes, :delete_method, :data, :data_index, :dependency_attributes, :manager_ref,
Expand Down Expand Up @@ -37,6 +37,7 @@ def initialize(model_class: nil, manager_ref: nil, association: nil, parent: nil
@attributes_whitelist = Set.new
@transitive_dependency_attributes = Set.new
@references = Set.new
@attribute_references = Set.new
@loaded_references = Set.new
@db_data_index = nil
@data_collection_finalized = false
Expand All @@ -56,13 +57,18 @@ def to_hash
end

def process_strategy(strategy_name)
return unless strategy_name

case strategy_name
when :local_db_cache_all
self.data_collection_finalized = true
self.saved = true
when :local_db_find_references
self.saved = true
when :local_db_find_missing_references
else
raise "Unknown InventoryCollection strategy: :#{strategy_name}, allowed strategies are :local_db_cache_all, "\
":local_db_find_references and :local_db_find_missing_references."
end
strategy_name
end
Expand Down Expand Up @@ -182,6 +188,11 @@ def inventory_object_class
end

def new_inventory_object(hash)
manager_ref.each do |x|
# TODO(lsmola) with some effort, we can do this, but it's complex
raise "A lazy_find with a :key can't be a part of the manager_uuid" if inventory_object_lazy?(hash[x]) && hash[x].key
end

inventory_object_class.new(self, hash)
end

Expand Down Expand Up @@ -269,18 +280,22 @@ def clone
:dependency_attributes => dependency_attributes.clone)
end

def belongs_to_associations
model_class.reflect_on_all_associations.select { |x| x.kind_of? ActiveRecord::Reflection::BelongsToReflection }
end

def association_to_foreign_key_mapping
return {} unless model_class

@association_to_foreign_key_mapping ||= model_class.reflect_on_all_associations.each_with_object({}) do |x, obj|
@association_to_foreign_key_mapping ||= belongs_to_associations.each_with_object({}) do |x, obj|
obj[x.name] = x.foreign_key
end
end

def foreign_key_to_association_mapping
return {} unless model_class

@foreign_key_to_association_mapping ||= model_class.reflect_on_all_associations.each_with_object({}) do |x, obj|
@foreign_key_to_association_mapping ||= belongs_to_associations.each_with_object({}) do |x, obj|
obj[x.foreign_key] = x.name
end
end
Expand All @@ -301,6 +316,14 @@ def foreign_type_to_association_mapping
end
end

def association_to_base_class_mapping
return {} unless model_class

@association_to_base_class_mapping ||= model_class.reflect_on_all_associations.each_with_object({}) do |x, obj|
obj[x.name] = x.klass.base_class.name unless x.polymorphic?
end
end

def base_class_name
return "" unless model_class

Expand All @@ -324,7 +347,7 @@ def inspect

def scan!
data.each do |inventory_object|
scan_inventory_object(inventory_object)
scan_inventory_object!(inventory_object)
end
end

Expand All @@ -335,27 +358,21 @@ def db_collection_for_comparison

private

attr_writer :attributes_blacklist, :attributes_whitelist, :db_data_index, :references
attr_writer :attributes_blacklist, :attributes_whitelist, :db_data_index

# Finds manager_uuid in the DB. Using a configured strategy we cache obtained data in the db_data_index, so the
# same find will not hit database twice. Also if we use lazy_links and this is called when
# data_collection_finalized?, we load all data from the DB, referenced by lazy_links, in one query.
#
# @param manager_uuid [String] a manager_uuid of the InventoryObject we search in the local DB
def find_in_db(manager_uuid)
# TODO(lsmola) selected need to contain also :keys used in other InventoryCollections pointing to this one, once
# we get list of all keys for each InventoryCollection ,we can uncomnent
# selected = [:id] + manager_ref.map { |x| model_class.reflect_on_association(x).try(:foreign_key) || x }
# selected << :type if model_class.new.respond_to? :type
# load_from_db.select(selected).find_each do |record|

# Use the cached db_data_index only data_collection_finalized?, meaning no new reference can occur
if data_collection_finalized? && db_data_index
return db_data_index[manager_uuid]
else
return db_data_index[manager_uuid] if db_data_index && db_data_index[manager_uuid]
# We haven't found the reference, lets add it to the list of references and load it
references << manager_uuid unless references.include?(manager_uuid) # O(C) since references is Set
references << manager_uuid unless references.include?(manager_uuid) # O(1) since references is Set
end

populate_db_data_index!
Expand All @@ -373,6 +390,12 @@ def populate_db_data_index!
# Initialize db_data_index in nil
self.db_data_index ||= {}

# TODO(lsmola) selected need to contain also :keys used in other InventoryCollections pointing to this one, once
# we get list of all keys for each InventoryCollection ,we can uncomnent
# selected = [:id] + manager_ref.map { |x| model_class.reflect_on_association(x).try(:foreign_key) || x }
# selected << :type if model_class.new.respond_to? :type
# load_from_db.select(selected).find_each do |record|

# Return the the correct relation based on strategy and selection&projection
case strategy
when :local_db_cache_all
Expand Down Expand Up @@ -402,6 +425,7 @@ def db_relation(selection = nil, projection = nil)
arel
end
rel = rel.where(selection) if rel && selection
rel = rel.select(projection) if rel && projection
rel
end

Expand Down Expand Up @@ -441,43 +465,58 @@ def process_db_record!(record)
else
stringify_reference(custom_manager_uuid.call(record))
end
db_data_index[index] = new_inventory_object(record.attributes.symbolize_keys)

attributes = record.attributes.symbolize_keys
attribute_references.each do |ref|
# We need to fill all references that are relations, we will use a ManagerRefresh::ApplicationRecordReference which
# can be used for filling a relation and we don't need to do any query here
# TODO(lsmola) maybe loading all, not just referenced here? Otherwise this will have issue for db_cache_all
# and find used in parser
next unless (foreign_key = association_to_foreign_key_mapping[ref])
base_class_name = attributes[association_to_foreign_type_mapping[ref].try(:to_sym)] || association_to_base_class_mapping[ref]
id = attributes[foreign_key.to_sym]
attributes[ref] = ManagerRefresh::ApplicationRecordReference.new(base_class_name, id)
end

db_data_index[index] = new_inventory_object(attributes)
db_data_index[index].id = record.id
end

def scan_inventory_object(inventory_object)
def scan_inventory_object!(inventory_object)
inventory_object.data.each do |key, value|
if value.kind_of?(Array)
value.each { |val| scan_inventory_object_attribute(key, val) }
value.each { |val| scan_inventory_object_attribute!(key, val) }
else
scan_inventory_object_attribute(key, value)
scan_inventory_object_attribute!(key, value)
end
end
end

def scan_inventory_object_attribute(key, value)
return unless inventory_object?(value)
def scan_inventory_object_attribute!(key, value)
return if !inventory_object_lazy?(value) && !inventory_object?(value)

# Storing attributes and their dependencies
(dependency_attributes[key] ||= Set.new) << value.inventory_collection if value.dependency?

# Storing if attribute is a transitive dependency, so a lazy_find :key results in dependency
transitive_dependency_attributes << key if transitive_dependency?(value)

# Storing a reference in the target inventory_collection, then each IC knows about all the references and can
# e.g. load all the referenced uuids from a DB
value.inventory_collection.references << value.to_s

if inventory_object_lazy?(value)
# Storing if attribute is a transitive dependency, so a lazy_find :key results in dependency
transitive_dependency_attributes << key if value.transitive_dependency?

# If we access an attribute of the value, using a :key, we want to keep a track of that
value.inventory_collection.attribute_references << value.key if value.key
end
end

def inventory_object?(value)
value.kind_of?(::ManagerRefresh::InventoryObjectLazy) || value.kind_of?(::ManagerRefresh::InventoryObject)
value.kind_of?(::ManagerRefresh::InventoryObject)
end

def transitive_dependency?(value)
# If the dependency is inventory_collection.lazy_find(:ems_ref, :key => :stack)
# and a :stack is a relation to another object, in the InventoryObject object,
# then this dependency is considered transitive.
(value.kind_of?(::ManagerRefresh::InventoryObjectLazy) && value.transitive_dependency?)
def inventory_object_lazy?(value)
value.kind_of?(::ManagerRefresh::InventoryObjectLazy)
end

def validate_inventory_collection!
Expand Down
8 changes: 2 additions & 6 deletions app/models/manager_refresh/inventory_object.rb
View file
Expand Up @@ -23,10 +23,6 @@ def load
end

def attributes(inventory_collection_scope = nil)
# TODO(lsmola) mark method with !, for performance reasons, this methods can be called only once, the second
# call will not return saveable result. We do not want to cache the result, since we want the lowest memory
# footprint.

# We should explicitly pass a scope, since the inventory_object can be mapped to more InventoryCollections with
# different blacklist and whitelist. The generic code always passes a scope.
inventory_collection_scope ||= inventory_collection
Expand Down Expand Up @@ -138,7 +134,6 @@ def allowed?(inventory_collection_scope, key)
foreign_to_association = inventory_collection_scope.foreign_key_to_association_mapping[key] ||
inventory_collection_scope.foreign_type_to_association_mapping[key]

# TODO(lsmola) can we make this O(1)? This check will be performed for each record in the DB
return false if inventory_collection_scope.attributes_blacklist.present? &&
(inventory_collection_scope.attributes_blacklist.include?(key) ||
(foreign_to_association && inventory_collection_scope.attributes_blacklist.include?(foreign_to_association)))
Expand All @@ -151,7 +146,8 @@ def allowed?(inventory_collection_scope, key)
end

def loadable?(value)
value.kind_of?(::ManagerRefresh::InventoryObjectLazy) || value.kind_of?(::ManagerRefresh::InventoryObject)
value.kind_of?(::ManagerRefresh::InventoryObjectLazy) || value.kind_of?(::ManagerRefresh::InventoryObject) ||
value.kind_of?(::ManagerRefresh::ApplicationRecordReference)
end
end
end
84 changes: 4 additions & 80 deletions ...ls/manager_refresh/save_inventory/graph_of_inventory_collections_targeted_refresh_spec.rb
View file
@@ -1,9 +1,11 @@
require_relative 'spec_helper'
require_relative 'spec_parsed_data'
require_relative 'init_data_helper'

describe ManagerRefresh::SaveInventory do
include SpecHelper
include SpecParsedData
include InitDataHelper

######################################################################################################################
# Spec scenarios showing saving of the inventory with Targeted refresh strategy
Expand Down Expand Up @@ -318,7 +320,8 @@
hardwares_init_data(
:arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}),
:strategy => :local_db_find_missing_references,
:manager_ref => [:vm_or_template])
:manager_ref => [:vm_or_template]
)
)
@data[:disks] = ::ManagerRefresh::InventoryCollection.new(
disks_init_data(
Expand Down Expand Up @@ -694,85 +697,6 @@ def all_collections
%i(orchestration_stacks orchestration_stacks_resources vms miq_templates key_pairs hardwares disks)
end

def initialize_all_inventory_collections
# Initialize the InventoryCollections
@data = {}
all_collections.each do |collection|
@data[collection] = ::ManagerRefresh::InventoryCollection.new(send("#{collection}_init_data"))
end
end

def initialize_inventory_collections(only_collections)
# Initialize the InventoryCollections
@data = {}
only_collections.each do |collection|
@data[collection] = ::ManagerRefresh::InventoryCollection.new(send("#{collection}_init_data",
{
:complete => false
}))
end

(all_collections - only_collections).each do |collection|
@data[collection] = ::ManagerRefresh::InventoryCollection.new(send("#{collection}_init_data",
{
:complete => false,
:strategy => :local_db_cache_all
}))
end
end

def orchestration_stacks_init_data(extra_attributes = {})
# Shadowing the default blacklist so we have an automatically solved graph cycle
init_data(cloud.orchestration_stacks(extra_attributes.merge(:attributes_blacklist => [])))
end

def orchestration_stacks_resources_init_data(extra_attributes = {})
# Shadowing the default blacklist so we have an automatically solved graph cycle
init_data(cloud.orchestration_stacks_resources(extra_attributes))
end

def vms_init_data(extra_attributes = {})
init_data(cloud.vms(extra_attributes.merge(:attributes_blacklist => [])))
end

def miq_templates_init_data(extra_attributes = {})
init_data(cloud.miq_templates(extra_attributes))
end

def key_pairs_init_data(extra_attributes = {})
init_data(cloud.key_pairs(extra_attributes))
end

def hardwares_init_data(extra_attributes = {})
init_data(cloud.hardwares(extra_attributes))
end

def disks_init_data(extra_attributes = {})
init_data(cloud.disks(extra_attributes))
end

def cloud
ManagerRefresh::InventoryCollectionDefault::CloudManager
end

def init_data(extra_attributes)
init_data = {
:parent => @ems,
}

init_data.merge!(extra_attributes)
end

def association_attributes(model_class)
# All association attributes and foreign keys of the model
model_class.reflect_on_all_associations.map { |x| [x.name, x.foreign_key] }.flatten.compact.map(&:to_sym)
end

def custom_association_attributes
# These are associations that are not modeled in a standard rails way, e.g. the ancestry
[:parent, :genealogy_parent, :genealogy_parent_object]
end

def initialize_inventory_collection_data
# Initialize the InventoryCollections data
@orchestration_stack_data_0_1 = orchestration_stack_data("0_1").merge(
Expand Down

0 comments on commit 8773af5

Please sign in to comment.