Skip to content

Commit

Permalink
specify virtual_delegate types to avoid deadlock
Browse files Browse the repository at this point in the history
deriving the attribute type for delegates required the target
class to be loaded.
This forced a cascade of load_schema calls that end up
introducing a race condition.

This PR explicitly declares the attribute type so the target class
no longer needs to be loaded and the race condition
(and subsequent deadlocks) are avoided.

https://bugzilla.redhat.com/show_bug.cgi?id=1700378
  • Loading branch information
kbrock committed May 23, 2019
1 parent 69f3c97 commit 168f3ad
Show file tree
Hide file tree
Showing 14 changed files with 35 additions and 32 deletions.
2 changes: 1 addition & 1 deletion Gemfile
Expand Up @@ -24,7 +24,7 @@ manageiq_plugin "manageiq-providers-ansible_tower" # can't move this down yet, b
manageiq_plugin "manageiq-schema"

# Unmodified gems
gem "activerecord-virtual_attributes", "~>1.1.0"
gem "activerecord-virtual_attributes", "~>1.2.0"
gem "activerecord-session_store", "~>1.1"
gem "acts_as_tree", "~>2.7" # acts_as_tree needs to be required so that it loads before ancestry
gem "ancestry", "~>3.0.4", :require => false
Expand Down
2 changes: 1 addition & 1 deletion app/models/ems_cluster.rb
Expand Up @@ -44,7 +44,7 @@ class EmsCluster < ApplicationRecord
include FilterableMixin

include DriftStateMixin
virtual_delegate :last_scan_on, :to => "last_drift_state_timestamp_rec.timestamp", :allow_nil => true
virtual_delegate :last_scan_on, :to => "last_drift_state_timestamp_rec.timestamp", :allow_nil => true, :type => :datetime

include RelationshipMixin
self.default_relationship_type = "ems_metadata"
Expand Down
2 changes: 1 addition & 1 deletion app/models/entitlement.rb
Expand Up @@ -7,7 +7,7 @@ class Entitlement < ApplicationRecord

validate :one_kind_of_managed_filter

virtual_delegate :name, :to => :miq_user_role, :allow_nil => true, :prefix => true
virtual_delegate :name, :to => :miq_user_role, :allow_nil => true, :prefix => true, :type => :string

def self.valid_filters?(filters_hash)
return true unless filters_hash # nil ok
Expand Down
14 changes: 7 additions & 7 deletions app/models/host.rb
Expand Up @@ -117,13 +117,13 @@ class Host < ApplicationRecord

virtual_column :os_image_name, :type => :string, :uses => [:operating_system, :hardware]
virtual_column :platform, :type => :string, :uses => [:operating_system, :hardware]
virtual_delegate :v_owning_cluster, :to => "ems_cluster.name", :allow_nil => true, :default => ""
virtual_delegate :v_owning_cluster, :to => "ems_cluster.name", :allow_nil => true, :default => "", :type => :string
virtual_column :v_owning_datacenter, :type => :string, :uses => :all_relationships
virtual_column :v_owning_folder, :type => :string, :uses => :all_relationships
virtual_delegate :cpu_total_cores, :cpu_cores_per_socket, :to => :hardware, :allow_nil => true, :default => 0
virtual_delegate :num_cpu, :to => "hardware.cpu_sockets", :allow_nil => true, :default => 0
virtual_delegate :total_vcpus, :to => "hardware.cpu_total_cores", :allow_nil => true, :default => 0
virtual_delegate :ram_size, :to => "hardware.memory_mb", :allow_nil => true, :default => 0
virtual_delegate :cpu_total_cores, :cpu_cores_per_socket, :to => :hardware, :allow_nil => true, :default => 0, :type => :integer
virtual_delegate :num_cpu, :to => "hardware.cpu_sockets", :allow_nil => true, :default => 0, :type => :integer
virtual_delegate :total_vcpus, :to => "hardware.cpu_total_cores", :allow_nil => true, :default => 0, :type => :integer
virtual_delegate :ram_size, :to => "hardware.memory_mb", :allow_nil => true, :default => 0, :type => :integer
virtual_column :enabled_inbound_ports, :type => :numeric_set # The following are not set to use anything
virtual_column :enabled_outbound_ports, :type => :numeric_set # because get_ports ends up re-querying the
virtual_column :enabled_udp_inbound_ports, :type => :numeric_set # database anyway.
Expand All @@ -139,7 +139,7 @@ class Host < ApplicationRecord
virtual_column :enabled_run_level_4_services, :type => :string_set, :uses => :host_services
virtual_column :enabled_run_level_5_services, :type => :string_set, :uses => :host_services
virtual_column :enabled_run_level_6_services, :type => :string_set, :uses => :host_services
virtual_delegate :annotation, :to => :hardware, :prefix => "v", :allow_nil => true
virtual_delegate :annotation, :to => :hardware, :prefix => "v", :allow_nil => true, :type => :string
virtual_column :vmm_vendor_display, :type => :string
virtual_column :ipmi_enabled, :type => :boolean
virtual_column :archived, :type => :boolean
Expand All @@ -163,7 +163,7 @@ class Host < ApplicationRecord
self.default_relationship_type = "ems_metadata"

include DriftStateMixin
virtual_delegate :last_scan_on, :to => "last_drift_state_timestamp_rec.timestamp", :allow_nil => true
virtual_delegate :last_scan_on, :to => "last_drift_state_timestamp_rec.timestamp", :allow_nil => true, :type => :datetime

include UuidMixin
include MiqPolicyMixin
Expand Down
2 changes: 1 addition & 1 deletion app/models/miq_group.rb
Expand Up @@ -16,7 +16,7 @@ class MiqGroup < ApplicationRecord
has_many :miq_product_features, :through => :miq_user_role
has_many :authentications, :dependent => :nullify

virtual_delegate :miq_user_role_name, :to => :entitlement, :allow_nil => true
virtual_delegate :miq_user_role_name, :to => :entitlement, :allow_nil => true, :type => :string
virtual_column :read_only, :type => :boolean
virtual_has_one :sui_product_features, :class_name => "Array"

Expand Down
2 changes: 1 addition & 1 deletion app/models/miq_product_feature.rb
Expand Up @@ -13,7 +13,7 @@ class MiqProductFeature < ApplicationRecord
has_many :shares, :through => :miq_product_features_shares
belongs_to :tenant

virtual_delegate :identifier, :to => :parent, :prefix => true, :allow_nil => true
virtual_delegate :identifier, :to => :parent, :prefix => true, :allow_nil => true, :type => :string

validates_presence_of :identifier
validates_uniqueness_of :identifier
Expand Down
2 changes: 1 addition & 1 deletion app/models/miq_report_result.rb
Expand Up @@ -12,7 +12,7 @@ class MiqReportResult < ApplicationRecord

serialize :report

virtual_delegate :description, :to => :miq_group, :prefix => true, :allow_nil => true
virtual_delegate :description, :to => :miq_group, :prefix => true, :allow_nil => true, :type => :string
virtual_attribute :status, :string
virtual_column :status_message, :type => :string, :uses => :miq_task
virtual_has_one :result_set, :class_name => "Hash"
Expand Down
2 changes: 1 addition & 1 deletion app/models/miq_server.rb
Expand Up @@ -37,7 +37,7 @@ class MiqServer < ApplicationRecord
scope :active_miq_servers, -> { where(:status => STATUSES_ACTIVE) }
scope :recently_active, -> { where(:last_heartbeat => 10.minutes.ago.utc...Time.now.utc) }
scope :with_zone_id, ->(zone_id) { where(:zone_id => zone_id) }
virtual_delegate :description, :to => :zone, :prefix => true, :allow_nil => true
virtual_delegate :description, :to => :zone, :prefix => true, :allow_nil => true, :type => :string

validate :validate_zone_not_maintenance?

Expand Down
4 changes: 2 additions & 2 deletions app/models/miq_widget.rb
Expand Up @@ -42,8 +42,8 @@ def destroy_schedule
end

virtual_column :status, :type => :string, :uses => :miq_task
virtual_delegate :status_message, :to => "miq_task.message", :allow_nil => true, :default => "Unknown"
virtual_delegate :queued_at, :to => "miq_task.created_on", :allow_nil => true
virtual_delegate :status_message, :to => "miq_task.message", :allow_nil => true, :default => "Unknown", :type => :string
virtual_delegate :queued_at, :to => "miq_task.created_on", :allow_nil => true, :type => :datetime
virtual_column :last_run_on, :type => :datetime, :uses => :miq_schedule

def row_count(row_count_param = nil)
Expand Down
1 change: 1 addition & 0 deletions app/models/mixins/authentication_mixin.rb
Expand Up @@ -13,6 +13,7 @@ module AuthenticationMixin
virtual_delegate :authentication_status,
:to => "authentication_status_severity_level.status",
:default => "None",
:type => :string,
:allow_nil => true

def self.authentication_check_schedule
Expand Down
2 changes: 2 additions & 0 deletions app/models/mixins/compliance_mixin.rb
Expand Up @@ -11,10 +11,12 @@ module ComplianceMixin

virtual_delegate :last_compliance_status,
:to => "last_compliance.compliant",
:type => :string,
:allow_nil => true
virtual_delegate :timestamp,
:to => :last_compliance,
:allow_nil => true,
:type => :datetime,
:prefix => true
end

Expand Down
4 changes: 2 additions & 2 deletions app/models/mixins/drift_state_mixin.rb
Expand Up @@ -9,8 +9,8 @@ module DriftStateMixin
has_one :first_drift_state_timestamp_rec, -> { select("id, timestamp, resource_type, resource_id").order("timestamp") }, :as => :resource, :class_name => 'DriftState'
has_one :last_drift_state_timestamp_rec, -> { order("timestamp DESC").select("id, timestamp, resource_type, resource_id") }, :as => :resource, :class_name => 'DriftState'

virtual_delegate :first_drift_state_timestamp, :to => "first_drift_state_timestamp_rec.timestamp", :allow_nil => true
virtual_delegate :last_drift_state_timestamp, :to => "last_drift_state_timestamp_rec.timestamp", :allow_nil => true
virtual_delegate :first_drift_state_timestamp, :to => "first_drift_state_timestamp_rec.timestamp", :allow_nil => true, :type => :datetime
virtual_delegate :last_drift_state_timestamp, :to => "last_drift_state_timestamp_rec.timestamp", :allow_nil => true, :type => :datetime
end

def drift_state_timestamps
Expand Down
4 changes: 2 additions & 2 deletions app/models/mixins/ownership_mixin.rb
Expand Up @@ -5,7 +5,7 @@ module OwnershipMixin
belongs_to :evm_owner, :class_name => "User"
belongs_to :miq_group

virtual_delegate :email, :name, :userid, :to => :evm_owner, :prefix => true, :allow_nil => true
virtual_delegate :email, :name, :userid, :to => :evm_owner, :prefix => true, :allow_nil => true, :type => :string

# Determine whether the selected object is owned by the current user
# Resulting SQL:
Expand All @@ -25,7 +25,7 @@ module OwnershipMixin
t.grouping(Arel::Nodes::NamedFunction.new("LOWER", [arel_attribute(:evm_owner_userid)]).eq(userid))
end)

virtual_delegate :owning_ldap_group, :to => "miq_group.description", :allow_nil => true
virtual_delegate :owning_ldap_group, :to => "miq_group.description", :allow_nil => true, :type => :string

# Determine whether to return objects owned by the current user's miq_group
# or not.
Expand Down
24 changes: 12 additions & 12 deletions app/models/vm_or_template.rb
Expand Up @@ -156,18 +156,18 @@ class VmOrTemplate < ApplicationRecord
virtual_column :used_storage, :type => :integer, :uses => [:used_disk_storage, :mem_cpu]
virtual_column :used_storage_by_state, :type => :integer, :uses => :used_storage
virtual_column :uncommitted_storage, :type => :integer, :uses => [:provisioned_storage, :used_storage_by_state]
virtual_delegate :ram_size_in_bytes, :to => :hardware, :allow_nil => true, :default => 0
virtual_delegate :mem_cpu, :to => "hardware.memory_mb", :allow_nil => true, :default => 0
virtual_delegate :ram_size, :to => "hardware.memory_mb", :allow_nil => true, :default => 0
virtual_delegate :ram_size_in_bytes, :to => :hardware, :allow_nil => true, :default => 0, :type => :integer
virtual_delegate :mem_cpu, :to => "hardware.memory_mb", :allow_nil => true, :default => 0, :type => :integer
virtual_delegate :ram_size, :to => "hardware.memory_mb", :allow_nil => true, :default => 0, :type => :integer
virtual_column :ipaddresses, :type => :string_set, :uses => {:hardware => :ipaddresses}
virtual_column :hostnames, :type => :string_set, :uses => {:hardware => :hostnames}
virtual_column :mac_addresses, :type => :string_set, :uses => {:hardware => :mac_addresses}
virtual_column :memory_exceeds_current_host_headroom, :type => :string, :uses => [:mem_cpu, {:host => [:hardware, :ext_management_system]}]
virtual_column :num_hard_disks, :type => :integer, :uses => {:hardware => :hard_disks}
virtual_column :num_disks, :type => :integer, :uses => {:hardware => :disks}
virtual_column :num_cpu, :type => :integer, :uses => :hardware
virtual_delegate :cpu_total_cores, :cpu_cores_per_socket, :to => :hardware, :allow_nil => true, :default => 0
virtual_delegate :annotation, :to => :hardware, :prefix => "v", :allow_nil => true
virtual_delegate :cpu_total_cores, :cpu_cores_per_socket, :to => :hardware, :allow_nil => true, :default => 0, :type => :integer
virtual_delegate :annotation, :to => :hardware, :prefix => "v", :allow_nil => true, :type => :string
virtual_column :has_rdm_disk, :type => :boolean, :uses => {:hardware => :disks}
virtual_column :disks_aligned, :type => :string, :uses => {:hardware => {:hard_disks => :partitions_aligned}}

Expand All @@ -183,11 +183,11 @@ class VmOrTemplate < ApplicationRecord
virtual_has_one :service, :class_name => 'Service'
virtual_has_one :parent_resource, :class_name => "VmOrTemplate"

virtual_delegate :name, :to => :host, :prefix => true, :allow_nil => true
virtual_delegate :name, :to => :storage, :prefix => true, :allow_nil => true
virtual_delegate :name, :to => :ems_cluster, :prefix => true, :allow_nil => true
virtual_delegate :vmm_product, :to => :host, :prefix => :v_host, :allow_nil => true
virtual_delegate :v_pct_free_disk_space, :v_pct_used_disk_space, :to => :hardware, :allow_nil => true
virtual_delegate :name, :to => :host, :prefix => true, :allow_nil => true, :type => :string
virtual_delegate :name, :to => :storage, :prefix => true, :allow_nil => true, :type => :string
virtual_delegate :name, :to => :ems_cluster, :prefix => true, :allow_nil => true, :type => :string
virtual_delegate :vmm_product, :to => :host, :prefix => :v_host, :allow_nil => true, :type => :string
virtual_delegate :v_pct_free_disk_space, :v_pct_used_disk_space, :to => :hardware, :allow_nil => true, :type => :float
delegate :connect_lans, :disconnect_lans, :to => :hardware, :allow_nil => true

before_validation :set_tenant_from_group
Expand Down Expand Up @@ -1519,9 +1519,9 @@ def changed_vm_value?(options)
#

virtual_delegate :allocated_disk_storage, :used_disk_storage,
:to => :hardware, :allow_nil => true, :uses => {:hardware => :disks}
:to => :hardware, :allow_nil => true, :uses => {:hardware => :disks}, :type => :integer

virtual_delegate :provisioned_storage, :to => :hardware, :allow_nil => true, :default => 0
virtual_delegate :provisioned_storage, :to => :hardware, :allow_nil => true, :default => 0, :type => :integer

def used_storage
used_disk_storage.to_i + ram_size_in_bytes
Expand Down

0 comments on commit 168f3ad

Please sign in to comment.