From 3c17a0bb437d20e4dc8c1157e346df4291e8b44d Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Tue, 21 Feb 2017 11:09:03 +0100 Subject: [PATCH 01/11] Add scanner for scannign dependencies and references Add scanner for scannign dependencies and references of the inventory collections. --- .../inventory_collection/scanner.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 app/models/manager_refresh/inventory_collection/scanner.rb diff --git a/app/models/manager_refresh/inventory_collection/scanner.rb b/app/models/manager_refresh/inventory_collection/scanner.rb new file mode 100644 index 00000000000..9142f3a3dda --- /dev/null +++ b/app/models/manager_refresh/inventory_collection/scanner.rb @@ -0,0 +1,19 @@ +module ManagerRefresh + class InventoryCollection + class Scanner + class << self + # Scanning inventory_collections for dependencies and references, storing the results in the inventory_collections + # themselves. Dependencies are needed for building a graph, references are needed for effective DB querying, where + # we can load all referenced objects of some InventoryCollection by one DB query. + # + # @param inventory_collections [Array] Array fo + def scan!(inventory_collections) + inventory_collections.each do |inventory_collection| + inventory_collection.data_collection_finalized = true + inventory_collection.scan! + end + end + end + end + end +end From 8d970be11948bf371ede3569222070b88895e439 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Tue, 21 Feb 2017 11:10:23 +0100 Subject: [PATCH 02/11] Before the saving, we need to scan the inventory collections Before the saving, we need to scan the inventory collections --- app/models/manager_refresh/save_inventory.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models/manager_refresh/save_inventory.rb b/app/models/manager_refresh/save_inventory.rb index 48444f50274..3680aa2c9a1 100644 --- a/app/models/manager_refresh/save_inventory.rb +++ b/app/models/manager_refresh/save_inventory.rb @@ -2,6 +2,10 @@ module ManagerRefresh class SaveInventory class << self def save_inventory(ems, inventory_collections) + _log.info("#{log_header(ems)} Scanning Inventory Collections...Start") + ManagerRefresh::InventoryCollection::Scanner.scan!(inventory_collections) + _log.info("#{log_header(ems)} Scanning Inventory Collections...Complete") + _log.info("#{log_header(ems)} Saving EMS Inventory...Start") inventory_object_saving_strategy = Settings.ems_refresh[ems.class.ems_type].try(:[], :inventory_object_saving_strategy) From a9055c0212ad32bb0d3c2e73872a7aafaf006b55 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Tue, 21 Feb 2017 11:12:40 +0100 Subject: [PATCH 03/11] Scanning of IventoryCollections including changed strategies Scanning of IventoryCollections that scans for dependendies and refrerences. Based on those, we can change the strategies for finding in the DB to find only refrerenced by doing 1 DB query. --- .../manager_refresh/inventory_collection.rb | 262 ++++++++++++------ 1 file changed, 178 insertions(+), 84 deletions(-) diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb index be374a1efc1..644dc25cf49 100644 --- a/app/models/manager_refresh/inventory_collection.rb +++ b/app/models/manager_refresh/inventory_collection.rb @@ -1,11 +1,11 @@ module ManagerRefresh class InventoryCollection - attr_accessor :saved + attr_accessor :saved, :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, :association, :complete, :update_only, :transitive_dependency_attributes, :custom_manager_uuid, - :custom_db_finder, :check_changed, :arel, :builder_params + :custom_db_finder, :check_changed, :arel, :builder_params, :loaded_references, :db_data_index delegate :each, :size, :to => :to_a @@ -13,28 +13,33 @@ def initialize(model_class: nil, manager_ref: nil, association: nil, parent: nil custom_save_block: nil, delete_method: nil, data_index: nil, data: nil, dependency_attributes: nil, attributes_blacklist: nil, attributes_whitelist: nil, complete: nil, update_only: nil, check_changed: nil, custom_manager_uuid: nil, custom_db_finder: nil, arel: nil, builder_params: {}) - @model_class = model_class - @manager_ref = manager_ref || [:ems_ref] - @custom_manager_uuid = custom_manager_uuid - @custom_db_finder = custom_db_finder - @association = association || [] - @parent = parent || nil - @arel = arel - @dependency_attributes = dependency_attributes || {} - @transitive_dependency_attributes = Set.new - @data = data || [] - @data_index = data_index || {} - @saved = saved || false - @strategy = process_strategy(strategy) - @delete_method = delete_method || :destroy + @model_class = model_class + @manager_ref = manager_ref || [:ems_ref] + @custom_manager_uuid = custom_manager_uuid + @custom_db_finder = custom_db_finder + @association = association || [] + @parent = parent || nil + @arel = arel + @dependency_attributes = dependency_attributes || {} + @data = data || [] + @data_index = data_index || {} + @saved = saved || false + @strategy = process_strategy(strategy) + @delete_method = delete_method || :destroy + @custom_save_block = custom_save_block + @check_changed = check_changed.nil? ? true : check_changed + @internal_attributes = [:__feedback_edge_set_parent] + @complete = complete.nil? ? true : complete + @update_only = update_only.nil? ? false : update_only + @builder_params = builder_params + @attributes_blacklist = Set.new @attributes_whitelist = Set.new - @custom_save_block = custom_save_block - @check_changed = check_changed.nil? ? true : check_changed - @internal_attributes = [:__feedback_edge_set_parent] - @complete = complete.nil? ? true : complete - @update_only = update_only.nil? ? false : update_only - @builder_params = builder_params + @transitive_dependency_attributes = Set.new + @references = Set.new + @loaded_references = Set.new + @db_data_index = nil + @data_collection_finalized = false blacklist_attributes!(attributes_blacklist) if attributes_blacklist.present? whitelist_attributes!(attributes_whitelist) if attributes_whitelist.present? @@ -52,40 +57,16 @@ def to_hash def process_strategy(strategy_name) case strategy_name - when :local_db_cache_all, :local_db_find_one - send("process_strategy_#{strategy_name}") - when :find_missing_in_local_db + 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 end strategy_name end - def load_from_db - return arel unless arel.nil? - parent.send(association) - end - - def process_strategy_local_db_cache_all - self.saved = true - # 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| - load_from_db.find_each do |record| - index = if custom_manager_uuid.nil? - object_index(record) - else - stringify_reference(custom_manager_uuid.call(record)) - end - data_index[index] = new_inventory_object(record.attributes.symbolize_keys) - data_index[index].id = record.id - end - end - - def process_strategy_local_db_find_one - self.saved = true - end - def check_changed? check_changed end @@ -114,12 +95,14 @@ def saveable? dependencies.all?(&:saved?) end + def data_collection_finalized? + data_collection_finalized + end + def <<(inventory_object) unless data_index[inventory_object.manager_uuid] data_index[inventory_object.manager_uuid] = inventory_object data << inventory_object - - actualize_dependencies(inventory_object) end self end @@ -174,9 +157,9 @@ def find_or_build_by(manager_uuid_hash) def find(manager_uuid) return if manager_uuid.nil? case strategy - when :local_db_find_one + when :local_db_find_references, :local_db_cache_all find_in_db(manager_uuid) - when :find_missing_in_local_db + when :local_db_find_missing_references data_index[manager_uuid] || find_in_db(manager_uuid) else data_index[manager_uuid] @@ -190,21 +173,6 @@ def find_by(manager_uuid_hash) find(object_index(manager_uuid_hash)) end - def find_in_db(manager_uuid) - manager_uuids = manager_uuid.split(stringify_joiner) - hash_uuid_by_ref = manager_ref.zip(manager_uuids).to_h - record = if custom_db_finder.nil? - parent.send(association).where(hash_uuid_by_ref).first - else - custom_db_finder.call(self, hash_uuid_by_ref) - end - return unless record - - inventory_object = new_inventory_object(record.attributes.symbolize_keys) - inventory_object.id = record.id - inventory_object - end - def lazy_find(manager_uuid, key: nil, default: nil) ::ManagerRefresh::InventoryObjectLazy.new(self, manager_uuid, :key => key, :default => default) end @@ -354,29 +322,155 @@ def inspect to_s end - def actualize_dependency(key, value) - if dependency?(value) - (dependency_attributes[key] ||= Set.new) << value.inventory_collection - transitive_dependency_attributes << key if transitive_dependency?(value) - elsif value.kind_of?(Array) && value.any? { |x| dependency?(x) } - (dependency_attributes[key] ||= Set.new) << value.detect { |x| dependency?(x) }.inventory_collection - transitive_dependency_attributes << key if value.any? { |x| transitive_dependency?(x) } + def scan! + data.each do |inventory_object| + scan_inventory_object(inventory_object) end end + def db_collection_for_comparison + return arel unless arel.nil? + parent.send(association) + end + private - attr_writer :attributes_blacklist, :attributes_whitelist + attr_writer :attributes_blacklist, :attributes_whitelist, :db_data_index, :references + + # 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 + end - def actualize_dependencies(inventory_object) + populate_db_data_index! + + db_data_index[manager_uuid] + end + + # Fills db_data_index with InventoryObjects obtained from the DB + def populate_db_data_index! + # Load only new references from the DB + new_references = references - loaded_references + # And store which references we've already loaded + loaded_references.merge(new_references) + + # Initialize db_data_index in nil + self.db_data_index ||= {} + + # Return the the correct relation based on strategy and selection&projection + case strategy + when :local_db_cache_all + selection = nil + projection = nil + else + selection = extract_references(new_references) + projection = nil + end + + db_relation(selection, projection).find_each do |record| + process_db_record!(record) + end + end + + # Return a Rails relation or array that will be used to obtain the records we need to load from the DB + # + # @param selection [Hash] A selection hash resulting in Select operation (in Relation algebra terms) + # @param projection [Array] A projection array resulting in Project operation (in Relation algebra terms) + def db_relation(selection = nil, projection = nil) + relation = if !custom_db_finder.blank? + custom_db_finder.call(self, selection, projection) + else + rel = if !parent.nil? && !association.nil? + parent.send(association) + elsif !arel.nil? + arel + end + rel = rel.where(selection) if rel && selection + rel + end + + relation || [] + end + + # Extracting references to a relation friendly format, or a format processable by a custom_db_finder + # + # @param new_references [Array] array of manager_uuids of the InventoryObjects + def extract_references(new_references = []) + # We collect what manager_uuids of this IC were referenced and we load only those + # TODO(lsmola) maybe in can be obj[x] = Set.new, since rails will do a query "col1 IN [a,b,b] AND col2 IN [e,f,e]" + # which is equivalent to "col1 IN [a,b] AND col2 IN [e,f]". The best would be to forcing rails to query + # "(col1, col2) IN [(a,e), (b,f), (b,e)]" which would load exactly what we need. Postgree supports this, but rails + # doesn't seem to. So for now, we can load a bit more from the DB than we need, in case of manager_ref.count > 1 + hash_uuids_by_ref = manager_ref.each_with_object({}) {|x, obj| obj[x] = [] } + + # TODO(lsmola) hm, if we call find in the parser code, not all references will be here, so this will really work + # only for lazy_find. So if we want to call find, I suppose we can cache all, possibly we could optimize this to + # set references upfront? + new_references.each do |reference| + refs = reference.split(stringify_joiner) + + refs.each_with_index do |ref, index| + hash_uuids_by_ref[manager_ref[index]] << ref + end + end + hash_uuids_by_ref + end + + # Takes ApplicationRecord record, converts it to the InventoryObject and places it to db_data_index + # + # @param record [ApplicationRecord] ApplicationRecord record we want to place to the db_data_index + def process_db_record!(record) + index = if custom_manager_uuid.nil? + object_index(record) + else + stringify_reference(custom_manager_uuid.call(record)) + end + db_data_index[index] = new_inventory_object(record.attributes.symbolize_keys) + db_data_index[index].id = record.id + end + + def scan_inventory_object(inventory_object) inventory_object.data.each do |key, value| - actualize_dependency(key, value) + if value.kind_of?(Array) + value.each { |val| scan_inventory_object_attribute(key, val) } + else + scan_inventory_object_attribute(key, value) + end end end - def dependency?(value) - (value.kind_of?(::ManagerRefresh::InventoryObjectLazy) || value.kind_of?(::ManagerRefresh::InventoryObject)) && - value.dependency? + def scan_inventory_object_attribute(key, value) + return unless 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 + end + + def inventory_object?(value) + value.kind_of?(::ManagerRefresh::InventoryObjectLazy) || value.kind_of?(::ManagerRefresh::InventoryObject) end def transitive_dependency?(value) From 3f7aee28ddf988e76118b14299b427ebd6beff0e Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Tue, 21 Feb 2017 11:15:56 +0100 Subject: [PATCH 04/11] load_from_db method name changed load_from_db method name changed --- app/models/manager_refresh/save_collection/helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/manager_refresh/save_collection/helper.rb b/app/models/manager_refresh/save_collection/helper.rb index 92b7b467714..3bff0a10692 100644 --- a/app/models/manager_refresh/save_collection/helper.rb +++ b/app/models/manager_refresh/save_collection/helper.rb @@ -36,7 +36,7 @@ def log_format_deletes(deletes) def save_inventory(inventory_collection) inventory_collection.parent.reload if inventory_collection.parent - association = inventory_collection.load_from_db + association = inventory_collection.db_collection_for_comparison record_index = {} create_or_update_inventory!(inventory_collection, record_index, association) From 5be35f7a0d597272a17d9c1a33e3d828a4fb31b1 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Tue, 21 Feb 2017 11:20:54 +0100 Subject: [PATCH 05/11] We don't need to actualize dependendencies per assignement We don't need to actualize dependendencies per assignement, it's being solved in the separate scanning step. --- app/models/manager_refresh/inventory_object.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/app/models/manager_refresh/inventory_object.rb b/app/models/manager_refresh/inventory_object.rb index ef5ddfc1f8a..f6bf56b0084 100644 --- a/app/models/manager_refresh/inventory_object.rb +++ b/app/models/manager_refresh/inventory_object.rb @@ -4,7 +4,7 @@ class InventoryObject attr_reader :inventory_collection, :data delegate :manager_ref, :base_class_name, :model_class, :to => :inventory_collection - delegate :[], :to => :data + delegate :[], :[]=, :to => :data def initialize(inventory_collection, data) @inventory_collection = inventory_collection @@ -85,12 +85,6 @@ def dependency? !inventory_collection.saved? end - def []=(key, value) - data[key] = value - inventory_collection.actualize_dependency(key, value) - value - end - def allowed_writers return [] unless model_class From 525d6114e94dee8a28cf05a2908f281ac40b9839 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Tue, 21 Feb 2017 11:22:22 +0100 Subject: [PATCH 06/11] Custom db finder using a selection Custom db finder using a selection --- .../cloud_manager.rb | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/app/models/manager_refresh/inventory_collection_default/cloud_manager.rb b/app/models/manager_refresh/inventory_collection_default/cloud_manager.rb index f5810788299..18624f66fc2 100644 --- a/app/models/manager_refresh/inventory_collection_default/cloud_manager.rb +++ b/app/models/manager_refresh/inventory_collection_default/cloud_manager.rb @@ -56,17 +56,16 @@ def hardwares(extra_attributes = {}) :association => :hardwares } - case extra_attributes[:strategy] - when :local_db_cache_all - attributes[:custom_manager_uuid] = lambda do |hardware| - [hardware.vm_or_template.ems_ref] - end - when :find_missing_in_local_db - attributes[:custom_db_finder] = lambda do |inventory_collection, hash_uuid_by_ref| - inventory_collection.parent.send(inventory_collection.association).references(:vm_or_template).where( - :vms => {:ems_ref => hash_uuid_by_ref[:vm_or_template]} - ).first - end + attributes[:custom_manager_uuid] = lambda do |hardware| + [hardware.vm_or_template.ems_ref] + end + + attributes[:custom_db_finder] = lambda do |inventory_collection, selection, _projection | + relation = inventory_collection.parent.send(inventory_collection.association). + includes(:vm_or_template). + references(:vm_or_template) + relation = relation.where(:vms => {:ems_ref => selection[:vm_or_template]}) unless selection.blank? + relation end attributes.merge!(extra_attributes) From 58364fe55f491d2ca449dfb0a024b8679cb56ffe Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Tue, 21 Feb 2017 11:22:54 +0100 Subject: [PATCH 07/11] Test that custom finder with scanned references is working Test that custom finder with scanned references is working --- ...ntory_collections_targeted_refresh_spec.rb | 97 +++++++------------ 1 file changed, 35 insertions(+), 62 deletions(-) diff --git a/spec/models/manager_refresh/save_inventory/graph_of_inventory_collections_targeted_refresh_spec.rb b/spec/models/manager_refresh/save_inventory/graph_of_inventory_collections_targeted_refresh_spec.rb index 8c341062cc5..703e4039879 100644 --- a/spec/models/manager_refresh/save_inventory/graph_of_inventory_collections_targeted_refresh_spec.rb +++ b/spec/models/manager_refresh/save_inventory/graph_of_inventory_collections_targeted_refresh_spec.rb @@ -310,18 +310,20 @@ vm_refs = ["vm_ems_ref_3", "vm_ems_ref_4"] @data[:vms] = ::ManagerRefresh::InventoryCollection.new( - :model_class => ManageIQ::Providers::CloudManager::Vm, - :arel => @ems.vms.where(:ems_ref => vm_refs) + vms_init_data( + :arel => @ems.vms.where(:ems_ref => vm_refs) + ) ) @data[:hardwares] = ::ManagerRefresh::InventoryCollection.new( - :model_class => Hardware, - :arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}), - :manager_ref => [:vm_or_template] + 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]) ) @data[:disks] = ::ManagerRefresh::InventoryCollection.new( - :model_class => Disk, - :arel => @ems.disks.joins(:hardware => :vm_or_template).where('hardware' => {'vms' => {'ems_ref' => vm_refs}}), - :manager_ref => [:hardware, :device_name] + disks_init_data( + :arel => @ems.disks.joins(:hardware => :vm_or_template).where('hardware' => {'vms' => {'ems_ref' => vm_refs}}), + ) ) @vm_data_3 = vm_data(3).merge( @@ -333,6 +335,7 @@ :guest_os => @data[:hardwares].lazy_find(image_data(2)[:ems_ref], :key => :guest_os), :vm_or_template => @data[:vms].lazy_find(vm_data(3)[:ems_ref]) ) + @disk_data_3 = disk_data(3).merge( :hardware => @data[:hardwares].lazy_find(vm_data(3)[:ems_ref]) ) @@ -378,7 +381,7 @@ :vm_or_template_id => @vm3.id, :bitness => 64, :virtualization_type => "virtualization_type_3", - :guest_os => nil, + :guest_os => "linux_generic_2", } ], :extra_disks => [ @@ -704,90 +707,60 @@ def initialize_inventory_collections(only_collections) @data = {} only_collections.each do |collection| @data[collection] = ::ManagerRefresh::InventoryCollection.new(send("#{collection}_init_data", - :extra_attributes => { + { :complete => false })) end (all_collections - only_collections).each do |collection| @data[collection] = ::ManagerRefresh::InventoryCollection.new(send("#{collection}_init_data", - :extra_attributes => { + { :complete => false, :strategy => :local_db_cache_all })) end end - def orchestration_stacks_init_data(extra_attributes: {}) - extra_attributes[:model_class] = ManageIQ::Providers::CloudManager::OrchestrationStack - - init_data(:orchestration_stacks, - extra_attributes) + 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: {}) - extra_attributes[:model_class] = OrchestrationStackResource - - init_data(:orchestration_stacks_resources, - extra_attributes) + 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: {}) - extra_attributes[:model_class] = ManageIQ::Providers::CloudManager::Vm - - init_data(:vms, - extra_attributes) + def vms_init_data(extra_attributes = {}) + init_data(cloud.vms(extra_attributes.merge(:attributes_blacklist => []))) end - def miq_templates_init_data(extra_attributes: {}) - extra_attributes[:model_class] = ManageIQ::Providers::CloudManager::Template - - init_data(:miq_templates, - extra_attributes) + def miq_templates_init_data(extra_attributes = {}) + init_data(cloud.miq_templates(extra_attributes)) end - def key_pairs_init_data(extra_attributes: {}) - extra_attributes[:manager_ref] = [:name] - extra_attributes[:model_class] = ManageIQ::Providers::CloudManager::AuthKeyPair - - init_data(:key_pairs, - extra_attributes) + def key_pairs_init_data(extra_attributes = {}) + init_data(cloud.key_pairs(extra_attributes)) end - def hardwares_init_data(extra_attributes: {}) - if extra_attributes[:strategy] == :local_db_cache_all - extra_attributes[:custom_manager_uuid] = lambda do |hardware| - [hardware.vm_or_template.ems_ref] - end - end - - extra_attributes[:manager_ref] = [:vm_or_template] - extra_attributes[:model_class] = Hardware - init_data(:hardwares, - extra_attributes) + def hardwares_init_data(extra_attributes = {}) + init_data(cloud.hardwares(extra_attributes)) end - def disks_init_data(extra_attributes: {}) - if extra_attributes[:strategy] == :local_db_cache_all - extra_attributes[:custom_manager_uuid] = lambda do |hardware| - [hardware.hardware.vm_or_template.ems_ref, hardware.device_name] - end - end + def disks_init_data(extra_attributes = {}) + init_data(cloud.disks(extra_attributes)) + end - extra_attributes[:manager_ref] = [:hardware, :device_name] - extra_attributes[:model_class] = Disk - init_data(:disks, - extra_attributes) + def cloud + ManagerRefresh::InventoryCollectionDefault::CloudManager end - def init_data(association, extra_attributes) + def init_data(extra_attributes) init_data = { - :parent => @ems, - :association => association + :parent => @ems, } init_data.merge!(extra_attributes) - init_data end def association_attributes(model_class) From 39c483b54790e5067a0b5190c5fc39df7c7faaa8 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Tue, 21 Feb 2017 11:24:17 +0100 Subject: [PATCH 08/11] Add an InventoryCollection for VMs crosslinks Add an InventoryCollection for VMs crosslinks --- .../inventory/persister/automation_manager.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/models/manageiq/providers/ansible_tower/inventory/persister/automation_manager.rb b/app/models/manageiq/providers/ansible_tower/inventory/persister/automation_manager.rb index e187ae7b968..3fc67bc4e65 100644 --- a/app/models/manageiq/providers/ansible_tower/inventory/persister/automation_manager.rb +++ b/app/models/manageiq/providers/ansible_tower/inventory/persister/automation_manager.rb @@ -15,5 +15,12 @@ def initialize_inventory_collections %i(credentials), :builder_params => {:resource => manager} ) + + collections[:vms] = ::ManagerRefresh::InventoryCollection.new( + :model_class => Vm, + :arel => Vm, + :strategy => :local_db_find_references, + :manager_ref => [:uid_ems] + ) end end From 4b4393b3d253bd9e5f3ee06a964027c0f2b4de39 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Tue, 21 Feb 2017 11:24:51 +0100 Subject: [PATCH 09/11] Use an InventoryCollection for VMs crosslinks Use an InventoryCollection for VMs crosslinks, this reduces number of DB queries from N to 1. --- .../ansible_tower/inventory/parser/automation_manager.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/app/models/manageiq/providers/ansible_tower/inventory/parser/automation_manager.rb b/app/models/manageiq/providers/ansible_tower/inventory/parser/automation_manager.rb index 94b9dacf967..45b32ee2ce5 100644 --- a/app/models/manageiq/providers/ansible_tower/inventory/parser/automation_manager.rb +++ b/app/models/manageiq/providers/ansible_tower/inventory/parser/automation_manager.rb @@ -20,8 +20,7 @@ def configured_systems inventory_object.hostname = host.name inventory_object.virtual_instance_ref = host.instance_id inventory_object.inventory_root_group = persister.inventory_root_groups.lazy_find(host.inventory_id.to_s) - # TODO(lsmola) get rid of direct DB access - inventory_object.counterpart = Vm.find_by(:uid_ems => host.instance_id) + inventory_object.counterpart = persister.vms.lazy_find(host.instance_id) end end @@ -34,15 +33,13 @@ def configuration_scripts inventory_object.variables = job_template.extra_vars_hash inventory_object.inventory_root_group = persister.inventory_root_groups.lazy_find(job_template.inventory_id.to_s) - authentications = [] + inventory_object.authentications = [] %w(credential_id cloud_credential_id network_credential_id).each do |credential_attr| next unless job_template.respond_to?(credential_attr) credential_id = job_template.public_send(credential_attr).to_s next if credential_id.blank? - authentications << persister.credentials.lazy_find(credential_id) + inventory_object.authentications << persister.credentials.lazy_find(credential_id) end - - inventory_object.authentications = authentications end end From c079e572645ff1f30be0fec6e0ea9e4692930da8 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Tue, 21 Feb 2017 13:36:11 +0100 Subject: [PATCH 10/11] Ancestry associations are not defined in models Ancestry associations are not defined in models, we will hardcode them, until there is a better way of a dependency scan. --- app/models/manager_refresh/inventory_object_lazy.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/models/manager_refresh/inventory_object_lazy.rb b/app/models/manager_refresh/inventory_object_lazy.rb index 1fa75d10ec0..dd749f28c5d 100644 --- a/app/models/manager_refresh/inventory_object_lazy.rb +++ b/app/models/manager_refresh/inventory_object_lazy.rb @@ -36,7 +36,16 @@ def transitive_dependency? # and a :stack is a relation to another object, in the InventoryObject object, # then this relation is considered transitive. !!(key && (inventory_collection.dependency_attributes.keys.include?(key) || - inventory_collection.model_class.reflect_on_association(key))) + association?(key))) + end + + def association?(key) + # TODO(lsmola) remove this if there will be better dependency scan, probably with transitive dependencies filled + # in a second pass + return true if [:parent, :genelogy_parent].include?(key) + + # Is the key an association on inventory_collection_scope model class? + !inventory_collection.association_to_foreign_key_mapping[key].nil? end private From 7912ea9369e7e4bfbf4ab86b9a572c03cc551d73 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Tue, 21 Feb 2017 13:37:57 +0100 Subject: [PATCH 11/11] Autofix rubocop issues Autofix rubocop issues --- app/models/manager_refresh/inventory_collection.rb | 2 +- .../inventory_collection_default/cloud_manager.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb index 644dc25cf49..097f74b4b42 100644 --- a/app/models/manager_refresh/inventory_collection.rb +++ b/app/models/manager_refresh/inventory_collection.rb @@ -417,7 +417,7 @@ def extract_references(new_references = []) # which is equivalent to "col1 IN [a,b] AND col2 IN [e,f]". The best would be to forcing rails to query # "(col1, col2) IN [(a,e), (b,f), (b,e)]" which would load exactly what we need. Postgree supports this, but rails # doesn't seem to. So for now, we can load a bit more from the DB than we need, in case of manager_ref.count > 1 - hash_uuids_by_ref = manager_ref.each_with_object({}) {|x, obj| obj[x] = [] } + hash_uuids_by_ref = manager_ref.each_with_object({}) { |x, obj| obj[x] = [] } # TODO(lsmola) hm, if we call find in the parser code, not all references will be here, so this will really work # only for lazy_find. So if we want to call find, I suppose we can cache all, possibly we could optimize this to diff --git a/app/models/manager_refresh/inventory_collection_default/cloud_manager.rb b/app/models/manager_refresh/inventory_collection_default/cloud_manager.rb index 18624f66fc2..a4dea997cec 100644 --- a/app/models/manager_refresh/inventory_collection_default/cloud_manager.rb +++ b/app/models/manager_refresh/inventory_collection_default/cloud_manager.rb @@ -60,10 +60,10 @@ def hardwares(extra_attributes = {}) [hardware.vm_or_template.ems_ref] end - attributes[:custom_db_finder] = lambda do |inventory_collection, selection, _projection | - relation = inventory_collection.parent.send(inventory_collection.association). - includes(:vm_or_template). - references(:vm_or_template) + attributes[:custom_db_finder] = lambda do |inventory_collection, selection, _projection| + relation = inventory_collection.parent.send(inventory_collection.association) + .includes(:vm_or_template) + .references(:vm_or_template) relation = relation.where(:vms => {:ems_ref => selection[:vm_or_template]}) unless selection.blank? relation end