These are chat archives for ManageIQ/manageiq/performance

19th
Aug 2016
Jason Frey
@Fryguy
Aug 19 2016 15:29
@kbrock I've thought about it but not often :)
I'm actually thikning switching to closure_tree eventually might be a better path forward, but ¯\_(ツ)_/¯
Keenan Brock
@kbrock
Aug 19 2016 15:30
:)
yea, closure trees are nice
but here, I really like ancestry and []
Jason Frey
@Fryguy
Aug 19 2016 15:30
yeah
Keenan Brock
@kbrock
Aug 19 2016 15:30
every few months I go back to the presentation
think I'll have to get a sample metrics together
if closure worked with different objects.....
then it would be a no brainer
Jason Frey
@Fryguy
Aug 19 2016 15:31
well, it can if we keep the relationships table :)
Keenan Brock
@kbrock
Aug 19 2016 15:31
right
ugh
relationships are hard
Jason Frey
@Fryguy
Aug 19 2016 15:31
no way around that...trees and relational DBs were not meant for each other
graph db is the way to go if we really really care
Keenan Brock
@kbrock
Aug 19 2016 15:32
yup
ooh
drew's problem
Jason Frey
@Fryguy
Aug 19 2016 15:32
I expermineted with Neo4j locally with Ruby...it was fun :)
Keenan Brock
@kbrock
Aug 19 2016 15:32
I think there was a race condition around the setting of the tree name stuff
but not sure
nice
Jason Frey
@Fryguy
Aug 19 2016 15:33
interesting
Keenan Brock
@kbrock
Aug 19 2016 15:33
I'm going to revert the relation changes we made
and see if the sporadic test still shows up
Jason Frey
@Fryguy
Aug 19 2016 15:33
do you have a link to the sproadic failure?
or an open issue?
Keenan Brock
@kbrock
Aug 19 2016 15:34
ManageIQ/manageiq#10534
the value coming back from path is rarely wrong
I'm going in and running other methods to try and find the culpret
it may be a scope being cached too
Jason Frey
@Fryguy
Aug 19 2016 15:34
very interesting
Keenan Brock
@kbrock
Aug 19 2016 15:35
values change for dest_host_nested.path
that is why I took
I felt responsible
but to be honest, I think the issue existed before
Jason Frey
@Fryguy
Aug 19 2016 15:35
maybe
Keenan Brock
@kbrock
Aug 19 2016 15:35
it is showing up now b/c a new test was introduced...
well, I'm not sure
that is why I'm spending so much time here :)
Jason Frey
@Fryguy
Aug 19 2016 15:36
what was the PR where you introducedthe change to the parents?
I want to link in that ticket
Keenan Brock
@kbrock
Aug 19 2016 15:36
ManageIQ/manageiq#10299
Jason Frey
@Fryguy
Aug 19 2016 15:36
(Not trying to be an I-told-you-so, but this yak shaving is what I was afraid of :( )
Keenan Brock
@kbrock
Aug 19 2016 15:37
yes
it cut out >50% of the time
I just ran into an issue with relationships in rbac again
Jason Frey
@Fryguy
Aug 19 2016 15:38
ok I assigned ManageIQ/manageiq#10534 to you since it seems related
Keenan Brock
@kbrock
Aug 19 2016 15:38
they really hurt our performance
+1
Jason Frey
@Fryguy
Aug 19 2016 15:38
you can always reassign back if not
Keenan Brock
@kbrock
Aug 19 2016 15:39
heh - beat me to the comment re ManageIQ/manageiq#10299
Drew Bomhof
@syncrou
Aug 19 2016 15:39
@kbrock, @Fryguy - I did run my tests after reverting ManageIQ/manageiq#10299 - And still ran into the failures with path.
Jason Frey
@Fryguy
Aug 19 2016 15:39
oh interesting
Keenan Brock
@kbrock
Aug 19 2016 15:39
@syncrou thanks - you just saved me a bunch of work
Jason Frey
@Fryguy
Aug 19 2016 15:39
maybe you guys can pair up to solve
Drew Bomhof
@syncrou
Aug 19 2016 15:40
Sounds good to me - In a meeting now however.
Jason Frey
@Fryguy
Aug 19 2016 15:40
weird coincidence that a bug was uncovered in OLD code that was changed in the past week and it's not really because of the change
those coincidences are the WORST kind of red herring
Dennis Metzger
@dmetzger57
Aug 19 2016 15:41
is it a new test that’s failing?
Jason Frey
@Fryguy
Aug 19 2016 15:41
yes, I believe the test was just recently added
Drew Bomhof
@syncrou
Aug 19 2016 15:41
@dmetzger57 - It was a test I added to test the nested datacenter changes
Jason Frey
@Fryguy
Aug 19 2016 15:41
and it was added over in provisioning, so it's sort of tangential
right @syncrou ?
Dennis Metzger
@dmetzger57
Aug 19 2016 15:42
just pondering the priority of this compared to the UI perf work queued up for @kbrock
Jason Frey
@Fryguy
Aug 19 2016 15:42
oh i see @dmetzger57
Dennis Metzger
@dmetzger57
Aug 19 2016 15:43
it’s a real squirrel, but still possibly a squirrel
:smile:
Jason Frey
@Fryguy
Aug 19 2016 15:43
yeah...given @kbrock was literally just in that code, he probably is the most knowledgeable to help
I guess after that would be me :)
Drew Bomhof
@syncrou
Aug 19 2016 15:44
@Fryguy - Yes.
Jason Frey
@Fryguy
Aug 19 2016 15:44
I'll reassign back to @syncrou
@syncrou I've known for a while that there is a nesting problem with with_relationship_type
in that it doesn't really support it :P haha
Drew Bomhof
@syncrou
Aug 19 2016 15:46
:laughing:
Jason Frey
@Fryguy
Aug 19 2016 15:46
but it has always worked in the past because typically once you start nesting into a tree, you don't change trees along the way
@dmetzger57 Yeah it's up to you if you want @kbrock to keep looking at this squirrel or not since there are bigger priorities...I can help out in his place if needed
Dennis Metzger
@dmetzger57
Aug 19 2016 15:53
@Fryguy I lean toward @kbrock focusing on the comitted UI improvements
Jason Frey
@Fryguy
Aug 19 2016 15:53
:+1:
Yeah, @kbrock I think you got into relationships because of the perf_capture_timer speed-ups, right?
or more specifically, the way perf_capture_timer needs to check if realtime-alerting is enabled, which needs to look at parent relationships
Keenan Brock
@kbrock
Aug 19 2016 16:03
yes, and also for rbac
Joe Rafaniello
@jrafanie
Aug 19 2016 16:10
please don't say rbac ;-)
we're going to make it worse
Jason Frey
@Fryguy
Aug 19 2016 16:12
BREAK ALL THE THINGS!
Joe Rafaniello
@jrafanie
Aug 19 2016 16:13
@chrisarcand and I want to totally write a new thing... rbac accepts way too many things
alas, we probably won't
Chris Arcand
@chrisarcand
Aug 19 2016 16:18
Sad but true
Keenan Brock
@kbrock
Aug 19 2016 16:18
@jrafanie I'm changing the api
and getting there
s/changing/reducing/
Chris Arcand
@chrisarcand
Aug 19 2016 16:18
:+1:
Joe Rafaniello
@jrafanie
Aug 19 2016 16:23
yeah
@kbrock we'll cc you on our changes to multiple entitlements
please cc us so we don't cross streams
Keenan Brock
@kbrock
Aug 19 2016 17:42
@jrafanie every change I make is going through GT and reviewed on Wednesdays :)
Joe Rafaniello
@jrafanie
Aug 19 2016 17:43
ok, just keep in mind we're changing rbac for entitlements, yet another filter type :cry:
Keenan Brock
@kbrock
Aug 19 2016 17:47
well, I have it down to only 1 place where we fetch ids for the records