These are chat archives for ManageIQ/manageiq/performance

25th
Sep 2015
Matthew Draper
@matthewd
Sep 25 2015 18:50 UTC
@dmetzger57 got a full backtrace?
Dennis Metzger
@dmetzger57
Sep 25 2015 18:52 UTC
@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 UTC
:+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 UTC
Actually, @dmetzger57 found this issue yesterday and has a fix
Dennis Metzger
@dmetzger57
Sep 25 2015 18:57 UTC
Moving the alias did not resolve the issue
Jason Frey
@Fryguy
Sep 25 2015 18:57 UTC
no?
Dennis Metzger
@dmetzger57
Sep 25 2015 18:57 UTC
nope
Matthew Draper
@matthewd
Sep 25 2015 18:57 UTC
@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 UTC
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 UTC
Does it matter? Seems to be a bug that it exists but raises when called
Jason Frey
@Fryguy
Sep 25 2015 18:59 UTC
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 UTC
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 UTC
yeah
Matthew Draper
@matthewd
Sep 25 2015 19:00 UTC
Well apparently we're already adding it
We just need it to work ;)
Jason Frey
@Fryguy
Sep 25 2015 19:00 UTC
heh
Dennis Metzger
@dmetzger57
Sep 25 2015 19:04 UTC
ok, so the fix is to .....
Jason Frey
@Fryguy
Sep 25 2015 19:05 UTC
add a host_ids method
it can probably just call hosts.collect(&:id)
Dennis Metzger
@dmetzger57
Sep 25 2015 19:05 UTC
i'll give it a try
Jason Frey
@Fryguy
Sep 25 2015 19:06 UTC
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 UTC
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 UTC
oh good point
is .ids in Rails 4.2?
I can't keep track :)
Matthew Draper
@matthewd
Sep 25 2015 19:15 UTC
Yep
Jason Frey
@Fryguy
Sep 25 2015 19:15 UTC
:+1:
Matthew Draper
@matthewd
Sep 25 2015 19:16 UTC
The one that's not is Array#pluck, which would've saved us the conditional
Jason Frey
@Fryguy
Sep 25 2015 19:16 UTC
ah ok...good to know
Rails 5, I presume?
Matthew Draper
@matthewd
Sep 25 2015 19:18 UTC
Yep
Matthew Draper
@matthewd
Sep 25 2015 20:42 UTC
@dmetzger57 have you got ids behaving itself now?
Dennis Metzger
@dmetzger57
Sep 25 2015 20:45 UTC
@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 UTC
Wanna push up a branch so I can see what you've got?
Keenan Brock
@kbrock
Sep 25 2015 20:46 UTC
@dmetzger57 I really like pluck. But I think it is my new shiny toy
Dennis Metzger
@dmetzger57
Sep 25 2015 20:50 UTC
@matthewd will push a branch is a bit, I took a break looking at another issue
Dennis Metzger
@dmetzger57
Sep 25 2015 21:10 UTC
@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 UTC
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 UTC
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 UTC
right
the aliases must move down past the methods that they are aliasing
Matthew Draper
@matthewd
Sep 25 2015 21:13 UTC
But instead of that, try this:
Jason Frey
@Fryguy
Sep 25 2015 21:13 UTC
I don't understand why they don't blow up
Matthew Draper
@matthewd
Sep 25 2015 21:13 UTC
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 UTC
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 UTC
@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 UTC
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 UTC
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 UTC
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 UTC
@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 UTC
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 UTC
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 UTC
yeah
Matthew Draper
@matthewd
Sep 25 2015 21:19 UTC
The builder call above, I guess
Jason Frey
@Fryguy
Sep 25 2015 21:20 UTC
I think @tenderlove added that
Matthew Draper
@matthewd
Sep 25 2015 21:21 UTC
So, we could check method_defined?("#{name.to_s.singularize}_ids") before we build the reflection
Jason Frey
@Fryguy
Sep 25 2015 21:21 UTC
yeah that's what I was thinking
Matthew Draper
@matthewd
Sep 25 2015 21:22 UTC
Though I suspect if it is defined, the reflection's going to override it anyway
Jason Frey
@Fryguy
Sep 25 2015 21:22 UTC
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 UTC
ugh, yeah, the build does it
Matthew Draper
@matthewd
Sep 25 2015 21:33 UTC
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 UTC
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 UTC
Yeah.. internalizing the strictly-readonly nature would be a necessary step along the way, I think