These are chat archives for ManageIQ/manageiq/performance

9th
Feb 2016
Jason Frey
@Fryguy
Feb 09 2016 02:02
Updated the PR with cleaner specs...it's ready for merge now
Keenan Brock
@kbrock
Feb 09 2016 02:02
k
first push travis is almost done - checking on next
any further thoughts on stefankroes/ancestry#262 ?
I'm thinking we could add a dot delimiter, and use postgres ltree
may be a good win
Jason Frey
@Fryguy
Feb 09 2016 02:10
wait, so the PR is to allow for setting the delimiter, but then you also changed the id format? why is that in the same PR?
Keenan Brock
@kbrock
Feb 09 2016 02:11
:(
well, the first was 1 PR
the second was requested by tons of people
"and it was just a little more"
should I remove?
Jason Frey
@Fryguy
Feb 09 2016 02:11
it's just confusing from a review perspective because more is happening than what's being described
(via the PR title)
Keenan Brock
@kbrock
Feb 09 2016 02:12
ok, change title /desc or remove?
Jason Frey
@Fryguy
Feb 09 2016 02:12
just change PR title desc I guess right now
Keenan Brock
@kbrock
Feb 09 2016 02:13
thnx
much better
typically it works in the end an all. but spending 2 hours to rebase old PRs and have their name on the commit is bitter sweet
what do you think about [Vm.find(1),Vm.find(2),Vm.find(3)].where(:id => [1,2])?
thinking about "ActiveArray"
Jason Frey
@Fryguy
Feb 09 2016 02:17
I've always wanted something like that...basically the where interface but executing in Ruby not SQL
Keenan Brock
@kbrock
Feb 09 2016 02:18
close to having it implemented in ActiveHash
Jason Frey
@Fryguy
Feb 09 2016 02:18
reminds me of the LINQ queries that can be run no regular Arrays
Keenan Brock
@kbrock
Feb 09 2016 02:18
well, it is working
but want to have where().order().where() chaining
I'll have to share when it gets closer
one of these weekends...
Jason Frey
@Fryguy
Feb 09 2016 02:20
ugh stupid Ruby 1.9.3
Keenan Brock
@kbrock
Feb 09 2016 02:21
darn - previous run worked fine too
Jason Frey
@Fryguy
Feb 09 2016 02:25
so I typoed something, and all the specs should have failed
the fact that 3/4 of them passed is bad
I'm going to look into why
Jason Frey
@Fryguy
Feb 09 2016 02:36
so here's an example of a run that did not run that should have: https://travis-ci.org/stefankroes/ancestry/jobs/107927798
Keenan Brock
@kbrock
Feb 09 2016 02:37
yea
I've had all sorts of issues running it locally
it was so easy before
but they are using thoughtbot's apprasials to handle multiple version of gems
very confusing
wwtd locally seems to be able to run most of them though
Jason Frey
@Fryguy
Feb 09 2016 04:52
ok, I fixed the tests on ancestry: stefankroes/ancestry#264
Keenan Brock
@kbrock
Feb 09 2016 04:52
k
Jason Frey
@Fryguy
Feb 09 2016 04:53
would prefer to get that in first so I can rebase my PR on top of it
Keenan Brock
@kbrock
Feb 09 2016 04:59
thanks
revbase away
Jason Frey
@Fryguy
Feb 09 2016 05:00
done
alright...I'm heading to bed
night :moon: :wave:
Keenan Brock
@kbrock
Feb 09 2016 05:01
already?
night ;)
Jason Frey
@Fryguy
Feb 09 2016 05:02
:P
Alex Krzos
@akrzos
Feb 09 2016 13:58
If you have a few more improvements to vm_infra/explorer and need me to run the numbers through my testbed I have the appliances ready to go,(Just FYI guys)
Joe Rafaniello
@jrafanie
Feb 09 2016 14:04
@akrzos @Fryguy has an arrange_nodes improvement (7-9% improvement) on 5.4.z and master... I have a master only improvement
@Fryguy talked about another possible improvement but we need to research if it's possible
@akrzos it's basically this PR: stefankroes/ancestry#261
Keenan Brock
@kbrock
Feb 09 2016 14:06
@jrafanie what do you think about ltree ?
Joe Rafaniello
@jrafanie
Feb 09 2016 14:07
I don't know much about it @kbrock
is that a pg specific function?
Keenan Brock
@kbrock
Feb 09 2016 14:07
yea, neither do I, but lots of ancestry people talk about it
Alex Krzos
@akrzos
Feb 09 2016 14:07
@jrafanie @Fryguy If you guys can point me at the commit I can patch the appliances and run it
Keenan Brock
@kbrock
Feb 09 2016 14:07
think so. but mysql and oracle have similar provisions
Joe Rafaniello
@jrafanie
Feb 09 2016 14:09
I'm not opposed to doing closer to the metal improvements where DB agnostic stuff is too slow but we'd have to see how much work it is
Dennis Metzger
@dmetzger57
Feb 09 2016 14:09
@akrzos have you put the two official 5.4 MRs (769 and 770) in your environment?
Alex Krzos
@akrzos
Feb 09 2016 14:11
I have @kbrock sort only right now
Joe Rafaniello
@jrafanie
Feb 09 2016 14:12
good question @dmetzger57, it's easy to forget what we're measuring
Keenan Brock
@kbrock
Feb 09 2016 14:12
@akrzos there are 2 patches 769 and 770
the 769 that got sent out may be a little big
one sec
Joe Rafaniello
@jrafanie
Feb 09 2016 14:13
@akrzos I can get 5.4.z enabled with the ancestry improvement stefankroes/ancestry#261 via a monkey patch initializer
I"m concerned with using ancestry master branch with 5.4.z
too much change
Alex Krzos
@akrzos
Feb 09 2016 14:15
@jrafanie did you want to test that separately or in conjunction with 769 & 770?
Joe Rafaniello
@jrafanie
Feb 09 2016 14:16
@akrzos I don't care as long as we have good before/after numbers of just a single change
Keenan Brock
@kbrock
Feb 09 2016 14:17
yay
Joe Rafaniello
@jrafanie
Feb 09 2016 14:17
let me switch to 5.4.z and get that ready
Alex Krzos
@akrzos
Feb 09 2016 14:21
Yeah I have to pull 770 in and test that
Jason Frey
@Fryguy
Feb 09 2016 16:11
@kbrock what will it take to get a new version of ancestry gem?
and how much do we have to drag in with a new version off of master
Keenan Brock
@kbrock
Feb 09 2016 16:12
@Fryguy no idea. I don't think I have rubygems access yet. I'll ping
Joe Rafaniello
@jrafanie
Feb 09 2016 16:12
@Fryguy not seeing gains with the arrange_nodes on 5.4.z /ruby 2.0.0
:-(
looking at something else right now, will come back to it after lunch
Keenan Brock
@kbrock
Feb 09 2016 16:12
@jrafanie do keep in mind, that the data is very temperamental for showing gains here
Jason Frey
@Fryguy
Feb 09 2016 16:13
ugh :(
Joe Rafaniello
@jrafanie
Feb 09 2016 16:13
@Fryguy my assumption is the ancestor_ids but I haven't profiled it on 5.4.z with the change
Jason Frey
@Fryguy
Feb 09 2016 16:15
ok yeah
I can look into that, or the other idea I had with the columns
@kbrock There's also a 2-1-stable branch...perhaps we could release a 2.1.1 with the tests and performance improvements pulled over
Keenan Brock
@kbrock
Feb 09 2016 16:20
yea, I created that branch before bringing stuff in
master may be 2.2
Jason Frey
@Fryguy
Feb 09 2016 18:04
@matthewd When you catch up, is there a way to preload a relation, but only load specific columns?
Something like, MiqPreloader.preload(hosts, :vms, :select => [:id, :name])
Alex Krzos
@akrzos
Feb 09 2016 18:43
for the 770 patch can the spec file be ignored?
Jason Frey
@Fryguy
Feb 09 2016 18:43
yes
Alex Krzos
@akrzos
Feb 09 2016 18:43
thanks
Jason Frey
@Fryguy
Feb 09 2016 18:44
So, this actually works MiqPreloader.preload(hosts, :vms, :select => [:id, :name]) :grin: it just doesn't work well with polymorphic relationships :(
Alex Krzos
@akrzos
Feb 09 2016 19:06
About how much % improvement should I see with 770 alone? or is it meant in conjunction with 769?
Jason Frey
@Fryguy
Feb 09 2016 19:06
which one is 770?
Jason Frey
@Fryguy
Feb 09 2016 19:08
(I'm not on VPN hence why I asked :) )
Alex Krzos
@akrzos
Feb 09 2016 19:08
Simplify archived? and orphaned? to not use associations
Jason Frey
@Fryguy
Feb 09 2016 19:09
achived/orphaned is not much...maybe 1-2%
the rbac sort performance one was a bit more but I forget how much
they are independent
Alex Krzos
@akrzos
Feb 09 2016 19:09
gotcha, ok yeah I'm not seeing much improvement with 770 vs 769
Dennis Metzger
@dmetzger57
Feb 09 2016 19:10
@akrzos which environment are you running the tests against?
Jason Frey
@Fryguy
Feb 09 2016 19:10
770 was such low risk that I threw it in
Alex Krzos
@akrzos
Feb 09 2016 19:10
@dmetzger57 I have three appliances each with 5.4.4.2, 5.4.5.0 and 5.5.0.13 respectively loaded with the customer db
trying the patchs on each
Dennis Metzger
@dmetzger57
Feb 09 2016 19:11
perfect test setup
Alex Krzos
@akrzos
Feb 09 2016 19:11
the previous version of the sort patch i tested was not the same as the one in 769
so re-testing it
Also I turned off some vms off a host and placed those appliances on that specific host
before the appliances were on a host with 3 rhevm simulators sitting next to them
that unfortunately added some noise to my previous test results
So trying 770 on 5.5 I'm seeing a slower timing, not sure it was ok to use that patch on 5.5
Is there a third patch? (If not I'll try 769+770 together)
Dennis Metzger
@dmetzger57
Feb 09 2016 19:15
This message was deleted
the third patch is something @Fryguy is still actively working on, so i say go for 769 + 770
Jason Frey
@Fryguy
Feb 09 2016 19:27
770 should not be slower on 5.5
but since it introduces such little gain, perhaps it was within the standard deviation
the third patch is "done" development wise, but @jrafanie was looking info finding simper ways to backport since we can't release a new gem
Alex Krzos
@akrzos
Feb 09 2016 19:35
This message was deleted
This message was deleted
not sure how to format that correctly as a table
This message was deleted
Alex Krzos
@akrzos
Feb 09 2016 19:39
5.4.5.0 Baseline 769 770 769&770
1 59964.6 46694 54275.7 39564.5
2 43487.2 36159.7 41447.9 29081.4
3 42248.1 33571.4 40347.4 27746.3
4 43146.8 29058.6 39473.1 27187.9
5 41446.8 28897.2 38674.4 24855.1
There we go
Jason Frey
@Fryguy
Feb 09 2016 19:40
is the number on the left the pass#?
Alex Krzos
@akrzos
Feb 09 2016 19:40
Yes or iteration
Jason Frey
@Fryguy
Feb 09 2016 19:40
ok, yeah, first run is always slower
Alex Krzos
@akrzos
Feb 09 2016 19:40
yup reboots inbetween patchs too
Jason Frey
@Fryguy
Feb 09 2016 19:40
wow...we got a lot more out of those 2 patches than I expected
Alex Krzos
@akrzos
Feb 09 2016 19:41
I need to examine 5.5 since the behavior is not consistent with the behavior on 5.4
i had to manually apply 769 though as patch fails to run on 5.5
Jason Frey
@Fryguy
Feb 09 2016 19:42
ok, I'm not sure we've backport those 2 patches to 5.5 anyway, yet, so that's a TODO (cc @kbrock)
Joe Rafaniello
@jrafanie
Feb 09 2016 19:44
5.5 probably is faster than those without the patches
Alex Krzos
@akrzos
Feb 09 2016 19:46
5.5.0.13 Baseline 769 770 769&770
1 22475 19857 29821 26974
2 17754 14901 24563 25241
3 16646 15081 24910 22937
4 18866 14470 24582 23469
5 18343 14479 23805 21715
So there is 5.5 and you can see 770 slowed it down, but perhaps it didn't apply correctly
769 on 5.5 is great
Jason Frey
@Fryguy
Feb 09 2016 19:47
there must be something else on 5.5 (aside from draper) that the removal of the prealoding in 770 is too aggressive
so, then it has to go back to the DB again
Joe Rafaniello
@jrafanie
Feb 09 2016 19:53
@akrzos can you run a rack ruby prof run of the baseline vs. 770?
oh wait
5.5, I don't think you can bundle
:cry:
does 5.5 have ruby-prof in the gemset?
Alex Krzos
@akrzos
Feb 09 2016 19:54
I think 5.5 has an issue because a hunk failed, let me see exactly what occured
Joe Rafaniello
@jrafanie
Feb 09 2016 19:54
ok
Alex Krzos
@akrzos
Feb 09 2016 19:56
yeah this failed to apply:
--- app/models/vm_or_template.rb
+++ app/models/vm_or_template.rb
@@ -1371,12 +1371,12 @@
   end

   def archived?
-    self.ext_management_system.nil? && self.storage.nil?
+    ems_id.nil? && storage_id.nil?
   end
   alias archived archived?

   def orphaned?
-    self.ext_management_system.nil? && !self.storage.nil?
+    ems_id.nil? && !storage_id.nil?
   end
   alias orphaned orphaned?
retrying with the changes
Also I should mention all of those tests were with admin user, I should retest with rbac
Joe Rafaniello
@jrafanie
Feb 09 2016 20:01
ah, cool
Jason Frey
@Fryguy
Feb 09 2016 20:03
@akrzos Yeah, that is the meat of that patch :)
Keenan Brock
@kbrock
Feb 09 2016 20:10
phew - without those changes would definitely make that slower
Alex Krzos
@akrzos
Feb 09 2016 20:10
5.5.0.13 Baseline 769 770 769&770
1 22475 19857 21682 19661
2 17754 14901 16430 14236
3 16646 15081 16272 14332
4 18866 14470 16052 14782
5 18343 14479 16467 14409
Jason Frey
@Fryguy
Feb 09 2016 20:10
oh yeah, because the preload got removed
Alex Krzos
@akrzos
Feb 09 2016 20:10
ok patched correctly now
Keenan Brock
@kbrock
Feb 09 2016 20:10
yea, 770 is minor
Jason Frey
@Fryguy
Feb 09 2016 20:10
yay..that's much better :)
Alex Krzos
@akrzos
Feb 09 2016 20:10
well now you know how much preload helps
Keenan Brock
@kbrock
Feb 09 2016 20:10
heh
Joe Rafaniello
@jrafanie
Feb 09 2016 20:12
:clap:
much better
Joe Rafaniello
@jrafanie
Feb 09 2016 20:49
@dmetzger57 are we good enough on 5.4 based on @akrzos's results above?
I was about to fire up 5.4.z with rubyprof again hoping I'll see something I missed before
Dennis Metzger
@dmetzger57
Feb 09 2016 20:51
@jrafanie the final answer will come from (or at least be heavily influenced) by what the customer sees in their environment with the change
Joe Rafaniello
@jrafanie
Feb 09 2016 20:52
can we install ruby 2.2 in their environment ;-)
Jason Frey
@Fryguy
Feb 09 2016 20:52
@jrafanie Can you throw together an intializer so @akrzos can test with that third patch?
Dennis Metzger
@dmetzger57
Feb 09 2016 20:52
I think we at the point (unless someone has another hot spot in sight) to wait for customer feedback
Joe Rafaniello
@jrafanie
Feb 09 2016 21:19
is there a reason we're not merging the 5.4 MRs?
I profiled without them again, waste of time
Jason Frey
@Fryguy
Feb 09 2016 21:20
¯\_(ツ)_/¯
Joe Rafaniello
@jrafanie
Feb 09 2016 21:20
specifically the sort and archived/orphaned ones
Oleg Barenboim
@chessbyte
Feb 09 2016 21:21
@dmetzger57 why not merge the performance MRs for 5.4?
Joe Rafaniello
@jrafanie
Feb 09 2016 21:21
I can do it, just wanted to ask before I did it
Jason Frey
@Fryguy
Feb 09 2016 21:21
we also need them ported to 5.5...not sure who opened the MRs for 5.4, but it should be the same person
Joe Rafaniello
@jrafanie
Feb 09 2016 21:22
looks like @kbrock did the work on 5.4 ;-)
Dennis Metzger
@dmetzger57
Feb 09 2016 21:24
769 and 770 are good to go for merging
Joe Rafaniello
@jrafanie
Feb 09 2016 21:24
ok, i'll do it, thanks!
ugh, @kbrock 770 failed the tests, please rebase
that should pick up the fix from MartinH
Keenan Brock
@kbrock
Feb 09 2016 22:05
rebasing
Joe Rafaniello
@jrafanie
Feb 09 2016 22:41
@Fryguy arrange_nodes with profiling on 5.4.z before and after
Screen Shot 2016-02-09 at 5.40.36 PM.png
Keenan Brock
@kbrock
Feb 09 2016 22:43
it introduced 23k calls to parent_id? that seems a bit much
from User#get_timezone
Keenan Brock
@kbrock
Feb 09 2016 22:44
for my stuff, that would not do much, but that seems to be a great option
Joe Rafaniello
@jrafanie
Feb 09 2016 22:45
yeah, @kbrock I've seen it creep into some, not all, of my rubyprof runs
Keenan Brock
@kbrock
Feb 09 2016 22:45
wait - you are ok caching this (which can change), but you are not ok caching region? (which will never change)
Joe Rafaniello
@jrafanie
Feb 09 2016 22:45
:laughing:
wise guy
Keenan Brock
@kbrock
Feb 09 2016 22:45
you're killing me
it is a good find though
and since MiqServer.my_server is cached, this can be a trivial caching
Joe Rafaniello
@jrafanie
Feb 09 2016 22:46
well, I need a more consistent set of ui screens to measure it's impact
Keenan Brock
@kbrock
Feb 09 2016 22:46
I'm finding get_config("vmdb").config.fetch_path showing up a lot in my profiles
looking forward to the config revamp
Joe Rafaniello
@jrafanie
Feb 09 2016 22:47
yeah, I don't see it all the time, otherwise I'd try it
but, I think we're asking what the server timezone is a lot
regardless of config revamp, I don't think that's needed
just ask once
but, it's not impacting the problematic page every time so I'm not really going after it
Oleg Barenboim
@chessbyte
Feb 09 2016 22:51
are you guys suggesting caching as in memo-izing or caching as in that timed cache way?
Joe Rafaniello
@jrafanie
Feb 09 2016 22:51
timed cache might be ok
Keenan Brock
@kbrock
Feb 09 2016 22:51
timed cache
Joe Rafaniello
@jrafanie
Feb 09 2016 22:52
it's an easy way to avoid cache invalidation
Keenan Brock
@kbrock
Feb 09 2016 22:52
what, not caching at all?
Oleg Barenboim
@chessbyte
Feb 09 2016 22:52
man I hate that hack
Keenan Brock
@kbrock
Feb 09 2016 22:52
or you mean timed cache?
Oleg Barenboim
@chessbyte
Feb 09 2016 22:52
the timed cache is what I hate
feels so dirty and imprecise
Joe Rafaniello
@jrafanie
Feb 09 2016 22:53
@chessbyte hehe, of all the hacks... that's the one you hate ;-)
Oleg Barenboim
@chessbyte
Feb 09 2016 22:53
always have hated it
because it is non-deterministic as to the record may change under it
there has to be a better way via memcache
with timestamps as part of the key or something like that
Joe Rafaniello
@jrafanie
Feb 09 2016 22:54
I'd like to revisit those cache_with_timeout (s) once config revamp lands
Jason Frey
@Fryguy
Feb 09 2016 22:54
I hate it too, but it is a decent tool for certain problems
Oleg Barenboim
@chessbyte
Feb 09 2016 22:55
fine, fine - agree to hate and fix later
Jason Frey
@Fryguy
Feb 09 2016 22:55
:+1:
collective hate...the darths would be proud
Joe Rafaniello
@jrafanie
Feb 09 2016 22:55
I think some of the caching went hand-in-hand with the ugliness in the VMDB::Config
Oleg Barenboim
@chessbyte
Feb 09 2016 22:55
and also with the non-use or under-use of memcache
Joe Rafaniello
@jrafanie
Feb 09 2016 22:56
yeah, single-use memcache
Jason Frey
@Fryguy
Feb 09 2016 22:56
@jrafanie the caching was borne out of not slamming the database with calls to MiqServer.my_server with workers :)
Oleg Barenboim
@chessbyte
Feb 09 2016 22:57
Jason Frey
@Fryguy
Feb 09 2016 22:57
cool