These are chat archives for ManageIQ/manageiq/performance

25th
Sep 2015
Matthew Draper
@matthewd
Sep 25 2015 18:50
@dmetzger57 got a full backtrace?
Dennis Metzger
@dmetzger57
Sep 25 2015 18:52
@matthewd
`` [----] E, [2015-09-25T11:36:49.555356 #20506:9ef994] ERROR -- : MIQ(MiqQueue#deliver) Message id: [24842], Error: [Association named 'hosts' was not found on MiqRegion; perhaps you misspelled it?] [----] E, [2015-09-25T11:36:49.555525 #20506:9ef994] ERROR -- : [ActiveRecord::AssociationNotFoundError]: Association named 'hosts' was not found on MiqRegion; perhaps you misspelled it? Method:[rescue in deliver] [----] E, [2015-09-25T11:36:49.555667 #20506:9ef994] ERROR -- : /opt/rubies/ruby-2.2.3/lib/ruby/gems/2.2.0/gems/activerecord-4.2.4/lib/active_record/associations.rb:161:inassociation'
/opt/rubies/ruby-2.2.3/lib/ruby/gems/2.2.0/gems/activerecord-4.2.4/lib/active_record/associations/builder/collection_association.rb:66:in host_ids' /var/www/miq/vmdb/app/models/mixins/aggregation_mixin.rb:100:inaggregate_hardware'
/var/www/miq/vmdb/app/models/mixins/aggregation_mixin.rb:30:in aggregate_cpu_speed' /var/www/miq/vmdb/app/models/vim_performance_state.rb:117:incapture_total'
/var/www/miq/vmdb/app/models/vim_performance_state.rb:55:in capture' /var/www/miq/vmdb/app/models/metric/ci_mixin/capture.rb:211:inperf_capture_state'
/var/www/miq/vmdb/app/models/metric/ci_mixin/state_finders.rb:21:in vim_performance_state_for_ts' /var/www/miq/vmdb/app/models/metric/ci_mixin/state_finders.rb:41:inext_management_systems_from_vim_performance_state_for_ts'
/var/www/miq/vmdb/app/models/metric/rollup.rb:189:in rollup_child_metrics' /var/www/miq/vmdb/app/models/metric/rollup.rb:131:inblock in rollup_hourly'
/var/www/miq/vmdb/app/models/metric/rollup.rb:131:in each' /var/www/miq/vmdb/app/models/metric/rollup.rb:131:inrollup_hourly'
/var/www/miq/vmdb/app/models/metric/ci_mixin/rollup.rb:91:in block (2 levels) in perf_rollup' /var/www/miq/vmdb/gems/pending/util/extensions/miq-benchmark.rb:12:inrealtime_store'
/var/www/miq/vmdb/gems/pending/util/extensions/miq-benchmark.rb:31:in realtime_block' /var/www/miq/vmdb/app/models/metric/ci_mixin/rollup.rb:90:inblock in perf_rollup'
/var/www/miq/vmdb/gems/pending/util/extensions/miq-benchmark.rb:12:in realtime_store' /var/www/miq/vmdb/gems/pending/util/extensions/miq-benchmark.rb:31:inrealtime_block'
/var/www/miq/vmdb/app/models/metric/ci_mixin/rollup.rb:77:in perf_rollup' /var/www/miq/vmdb/app/models/miq_queue.rb:345:inblock in deliver'
/opt/rubies/ruby-2.2.3/lib/ruby/2.2.0/timeout.rb:88:in block in timeout' /opt/rubies/ruby-2.2.3/lib/ruby/2.2.0/timeout.rb:32:inblock in catch'
/opt/rubies/ruby-2.2.3/lib/ruby/2.2.0/timeout.rb:32:in catch' /opt/rubies/ruby-2.2.3/lib/ruby/2.2.0/timeout.rb:32:incatch'
/opt/rubies/ruby-2.2.3/lib/ruby/2.2.0/timeout.rb:103:in timeout' /var/www/miq/vmdb/app/models/miq_queue.rb:341:indeliver'
/var/www/miq/vmdb/app/models/miq_queue_worker_base/runner.rb:106:in deliver_queue_message' /var/www/miq/vmdb/app/models/miq_queue_worker_base/runner.rb:134:indeliver_message'
/var/www/miq/vmdb/app/models/miq_queue_worker_base/runner.rb:151:in block in do_work' /var/www/miq/vmdb/app/models/miq_queue_worker_base/runner.rb:145:inloop'
/var/www/miq/vmdb/app/models/miq_queue_worker_base/runner.rb:145:in do_work' /var/www/miq/vmdb/app/models/miq_worker/runner.rb:325:inblock in do_work_loop'
/var/www/miq/vmdb/app/models/miq_worker/runner.rb:322:in loop' /var/www/miq/vmdb/app/models/miq_worker/runner.rb:322:indo_work_loop'
/var/www/miq/vmdb/app/models/miq_worker/runner.rb:142:in run' /var/www/miq/vmdb/app/models/miq_worker/runner.rb:123:instart'
/var/www/miq/vmdb/app/models/miq_worker/runner.rb:24:in start_worker' /var/www/miq/vmdb/lib/workers/bin/worker.rb:2:in<top (required)>'
/opt/rubies/ruby-2.2.3/lib/ruby/gems/2.2.0/gems/railties-4.2.4/lib/rails/commands/runner.rb:60:in load' /opt/rubies/ruby-2.2.3/lib/ruby/gems/2.2.0/gems/railties-4.2.4/lib/rails/commands/runner.rb:60:in<top (required)>'
/opt/rubies/ruby-2.2.3/lib/ruby/gems/2.2.0/gems/railties-4.2.4/lib/rails/commands/commands_tasks.rb:123:in require' /opt/rubies/ruby-2.2.3/lib/ruby/gems/2.2.0/gems/railties-4.2.4/lib/rails/commands/commands_tasks.rb:123:inrequir
Matthew Draper
@matthewd
Sep 25 2015 18:55
:+1:
Looks like our virtual associations don't support the foo_ids method
irb(main):002:0> MiqRegion.first.hosts
  MiqRegion Load (0.6ms)  SELECT  "miq_regions".* FROM "miq_regions"  ORDER BY "miq_regions"."id" ASC LIMIT 1
  MiqRegion Inst Including Associations (0.1ms - 1rows)
  Host Load (12.7ms)  SELECT "hosts".* FROM "hosts" WHERE ("hosts"."id" BETWEEN 0 AND 999999999999)
  Host Inst Including Associations (0.0ms - 0rows)
=> #<ActiveRecord::Relation []>
irb(main):003:0> MiqRegion.first.host_ids
  MiqRegion Load (0.6ms)  SELECT  "miq_regions".* FROM "miq_regions"  ORDER BY "miq_regions"."id" ASC LIMIT 1
  MiqRegion Inst Including Associations (0.1ms - 1rows)
ActiveRecord::AssociationNotFoundError: Association named 'hosts' was not found on MiqRegion; perhaps you misspelled it?
Jason Frey
@Fryguy
Sep 25 2015 18:57
Actually, @dmetzger57 found this issue yesterday and has a fix
Dennis Metzger
@dmetzger57
Sep 25 2015 18:57
Moving the alias did not resolve the issue
Jason Frey
@Fryguy
Sep 25 2015 18:57
no?
Dennis Metzger
@dmetzger57
Sep 25 2015 18:57
nope
Matthew Draper
@matthewd
Sep 25 2015 18:57
@Fryguy note I'm not using an alias there… I'm calling the actual association methods
So it's getting defined, but I think we're just not hijacking it, so it gets too far through to the "real" association-stuff
Jason Frey
@Fryguy
Sep 25 2015 18:58
there shouldn't be a real association on that mode though
what is calling host_ids, I guess is the question?
Matthew Draper
@matthewd
Sep 25 2015 18:59
Does it matter? Seems to be a bug that it exists but raises when called
Jason Frey
@Fryguy
Sep 25 2015 18:59
Oh...wait...we used to have real associations on the Region record
with finder_sql...but you can't write them with real associations anymore, hence the issue
so they were switched to virtual associations...seems like we need to add *_ids method for everything :(
Matthew Draper
@matthewd
Sep 25 2015 18:59
Yeah, so this has possibly always been a shortcoming of virtual associations, and we just haven't used the _ids method before
Jason Frey
@Fryguy
Sep 25 2015 18:59
yeah
Matthew Draper
@matthewd
Sep 25 2015 19:00
Well apparently we're already adding it
We just need it to work ;)
Jason Frey
@Fryguy
Sep 25 2015 19:00
heh
Dennis Metzger
@dmetzger57
Sep 25 2015 19:04
ok, so the fix is to .....
Jason Frey
@Fryguy
Sep 25 2015 19:05
add a host_ids method
it can probably just call hosts.collect(&:id)
Dennis Metzger
@dmetzger57
Sep 25 2015 19:05
i'll give it a try
Jason Frey
@Fryguy
Sep 25 2015 19:06
you should probably write some specs for all of the aggregations on a Region...since region is so different that the others
you might uncover more
Matthew Draper
@matthewd
Sep 25 2015 19:10
I think the other methods are all mutative, which doesn't really make sense for a virtual association
@dmetzger57 you want x = send(name); x.respond_to?(:ids) ? x.ids : x.collect(&:id)
If the virtual association method happens to return a Relation, we can use it to be much more efficient… if not, the query's already happened, and it doesn't matter.
Jason Frey
@Fryguy
Sep 25 2015 19:15
oh good point
is .ids in Rails 4.2?
I can't keep track :)
Matthew Draper
@matthewd
Sep 25 2015 19:15
Yep
Jason Frey
@Fryguy
Sep 25 2015 19:15
:+1:
Matthew Draper
@matthewd
Sep 25 2015 19:16
The one that's not is Array#pluck, which would've saved us the conditional
Jason Frey
@Fryguy
Sep 25 2015 19:16
ah ok...good to know
Rails 5, I presume?
Matthew Draper
@matthewd
Sep 25 2015 19:18
Yep
Matthew Draper
@matthewd
Sep 25 2015 20:42
@dmetzger57 have you got ids behaving itself now?
Dennis Metzger
@dmetzger57
Sep 25 2015 20:45
@matthewd no , i created the host_ids method, still seeing the host method called and getting the same error.
Matthew Draper
@matthewd
Sep 25 2015 20:46
Wanna push up a branch so I can see what you've got?
Keenan Brock
@kbrock
Sep 25 2015 20:46
@dmetzger57 I really like pluck. But I think it is my new shiny toy
Dennis Metzger
@dmetzger57
Sep 25 2015 20:50
@matthewd will push a branch is a bit, I took a break looking at another issue
Dennis Metzger
@dmetzger57
Sep 25 2015 21:10
@matthewd my master https://github.com/dmetzger57/manageiq has the last thing I tried, which was Jason's initial suggestion, only change is added a method host_ids to miq_region.rb
Jason Frey
@Fryguy
Sep 25 2015 21:11
you definitely want to go through the hosts method, so your first commit is the most accurate
also the aliases are still wrong
Matthew Draper
@matthewd
Sep 25 2015 21:12
Yeah, so the problem there was that we're not actually calling host_ids — we're calling all_host_ids… and that gets aliased to the old host_ids method, further up in the file
Jason Frey
@Fryguy
Sep 25 2015 21:13
right
the aliases must move down past the methods that they are aliasing
Matthew Draper
@matthewd
Sep 25 2015 21:13
But instead of that, try this:
Jason Frey
@Fryguy
Sep 25 2015 21:13
I don't understand why they don't blow up
Matthew Draper
@matthewd
Sep 25 2015 21:13
diff --git a/lib/extensions/ar_virtual.rb b/lib/extensions/ar_virtual.rb
index e762f85..7ea1cff 100644
--- a/lib/extensions/ar_virtual.rb
+++ b/lib/extensions/ar_virtual.rb
@@ -162,6 +162,11 @@ def virtual_has_many(name, options = {})
     uses = options.delete :uses
     reflection = ActiveRecord::Associations::Builder::HasMany.build(self, name, nil, options)
     add_virtual_reflection(reflection, name, uses, options)
+
+    define_method("#{name.to_s.singularize}_ids") do
+      records = send(name)
+      records.respond_to?(:ids) ? records.ids : records.collect(&:id)
+    end
   end

   def virtual_belongs_to(name, options = {})
Jason Frey
@Fryguy
Sep 25 2015 21:13
oh that's sweet
but you should only define it if the corresponding method doesn't already exist
Matthew Draper
@matthewd
Sep 25 2015 21:14
@Fryguy that shouldn't be a problem… we normally define the virtual associations at the top of the class, before the methods
Jason Frey
@Fryguy
Sep 25 2015 21:15
yeah, but I always had the intention of moving them after the method in question (kind of like you might do with private_class_method :blah)
Dennis Metzger
@dmetzger57
Sep 25 2015 21:15
So take the change to ar_virtual.rb instead of tweaking mi_region.rb?
the generic solution
Jason Frey
@Fryguy
Sep 25 2015 21:16
I guess if I ever got around to implementing that I would change virtual_has_many to ensure the corresponding method exists
Matthew Draper
@matthewd
Sep 25 2015 21:17
@Fryguy also, somewhere along the way, host_ids is apparently getting defined (because the alias is working, and the error isn't a NoMethodError)… so I imagine if we asked, it would already be there
Jason Frey
@Fryguy
Sep 25 2015 21:17
maybe from AggregationMixin, but I was pretty sure that module only defined all_host_ids
I guess it's not hard to check :D'
Matthew Draper
@matthewd
Sep 25 2015 21:19
The short answer is it's getting defined by AR, as it would for a real association. I'm just not sure how that comes about.
Jason Frey
@Fryguy
Sep 25 2015 21:19
yeah
Matthew Draper
@matthewd
Sep 25 2015 21:19
The builder call above, I guess
Jason Frey
@Fryguy
Sep 25 2015 21:20
I think @tenderlove added that
Matthew Draper
@matthewd
Sep 25 2015 21:21
So, we could check method_defined?("#{name.to_s.singularize}_ids") before we build the reflection
Jason Frey
@Fryguy
Sep 25 2015 21:21
yeah that's what I was thinking
Matthew Draper
@matthewd
Sep 25 2015 21:22
Though I suspect if it is defined, the reflection's going to override it anyway
Jason Frey
@Fryguy
Sep 25 2015 21:22
I think I'm just surprised that ActiveRecord::Associations::Builder::HasMany.build has a side effect of putting it in the GeneratedMethods
I feel like that's the real bug deep down
not a Rails bug...perhaps there's a way to call Builder.new instead and not have it created a generated method
(I'm assuming that's what does it...I can't figure it out otherwise)
Jason Frey
@Fryguy
Sep 25 2015 21:32
ugh, yeah, the build does it
Matthew Draper
@matthewd
Sep 25 2015 21:33
Yeah, I guess it's a case of us only wanting 80% of a piece of functionality
To me, it'd be ideal if we did actually use the auto-defined method, by virtue of registering a proper reflection for our association
But that's a more involved journey in trying to get virtual associations upstreamable
Jason Frey
@Fryguy
Sep 25 2015 21:35
yeah
just as an FYI, here's what I wrote to verify:
class Blah < ActiveRecord::Base
  self.table_name = "vms"
  def dangerous_attribute_method?(_); false; end
  def generated_association_methods; self; end
  def add_autosave_association_callbacks(*args); self; end
  def self.generate_reflection(on)
    ActiveRecord::Associations::Builder::HasMany.build(self, on, nil, {})
  end
end

> Blah.instance_methods - Object.methods - ActiveRecord::Base.instance_methods
# => [:dangerous_attribute_method?, :generated_association_methods, :add_autosave_association_callbacks]
> Blah.generate_reflection(:things)
> Blah.instance_methods - Object.methods - ActiveRecord::Base.instance_methods
# => [:dangerous_attribute_method?, :generated_association_methods, :add_autosave_association_callbacks, :autosave_associated_records_for_things, :validate_associated_records_for_things, :before_add_for_things, :before_add_for_things?, :before_add_for_things=, :after_add_for_things, :after_add_for_things?, :after_add_for_things=, :before_remove_for_things, :before_remove_for_things?, :before_remove_for_things=, :after_remove_for_things, :after_remove_for_things?, :after_remove_for_things=, :things, :thing_ids, :things=, :thing_ids=]
Lines 3-5 there are taken from the stuff in ArVirtual's NonPrModels (probably don't need them, but whatevs)
the thing is, they aren't proper associations
you can't update them or manipulate them in most cases
they are more like computed values
Matthew Draper
@matthewd
Sep 25 2015 21:39
Yeah.. internalizing the strictly-readonly nature would be a necessary step along the way, I think