19:14:03 <lbragstad> #startmeeting keystone-office-hours 19:14:04 <openstack> Meeting started Tue Jun 27 19:14:03 2017 UTC and is due to finish in 60 minutes. The chair is lbragstad. Information about MeetBot at http://wiki.debian.org/MeetBot. 19:14:06 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 19:14:08 <openstack> The meeting name has been set to 'keystone_office_hours' 19:14:45 <hrybacki> o/ 19:14:51 <gagehugo> o/ 19:14:53 <lbragstad> o/ 19:15:05 <hrybacki> cool. what now? 19:15:14 <lamt> o/ 19:16:06 <lbragstad> next - let's go through what we want to try and get done 19:16:10 <lbragstad> this can be patches in review 19:16:15 <lbragstad> or bugs in LP 19:16:34 <hrybacki> what needs help the most right now? 19:17:33 <lbragstad> i use https://goo.gl/ZgrrT7 19:17:35 <lbragstad> #link https://goo.gl/ZgrrT7 19:17:52 <gagehugo> lbragstad oh nice 19:18:03 <hrybacki> same 19:18:17 <lbragstad> the idea came from dstanek 19:18:35 <lbragstad> but the objective was to draw attention to the patches we have in review that already close bugs 19:18:49 <lbragstad> instead of automatically going to launchpad to find something to fix 19:19:26 <lbragstad> not to shamelessly plug my own patch but.. 19:19:33 <lbragstad> #link https://review.openstack.org/#/c/475472/ seems to be close 19:20:01 <lbragstad> #link https://review.openstack.org/#/c/475929/ is good 19:20:07 <lbragstad> i can review ^ 19:20:20 * lbragstad can't do much, be he can review 7 lines of code 19:20:30 <hrybacki> I'll peak at your commit now lbragstad 19:20:38 <hrybacki> /s/commit/review/ 19:20:45 <lbragstad> hrybacki: thanks! 19:20:47 <gagehugo> same 19:21:20 <gagehugo> lbragstad I went with a more generic message for https://review.openstack.org/#/c/475929/ 19:21:50 <gagehugo> not sure if we want to tell the user that the backend is ldap 19:24:02 <lbragstad> gagehugo: ok - i think that's fine 19:26:58 <lbragstad> #action review https://review.openstack.org/#/c/475472/ 19:27:06 <lbragstad> #action review https://review.openstack.org/#/c/475929/ 19:27:20 <lbragstad> #link https://review.openstack.org/#/c/475100/ 19:27:31 <lbragstad> ^ that's another one that's ready for review and closes a bug 19:27:54 <lbragstad> #undo 19:27:55 <openstack> Removing item from minutes: #link https://review.openstack.org/#/c/475100/ 19:28:09 <lbragstad> maybe not - triaging the bug 19:29:06 <hrybacki> lbragstad: gagehugo general question -- when do we add tests? 19:29:29 <lbragstad> hrybacki: unit tests? 19:29:33 <hrybacki> e.g. for gagehugo's review, would we want to add a test that checks the proper exception is caught? 19:29:35 * hrybacki nods 19:29:55 <gagehugo> hrybacki yeah should probably do that 19:30:28 <lbragstad> hrybacki: yeah - that'd be good 19:30:38 <hrybacki> do we have a general policy for that though? 19:30:39 <lbragstad> gagehugo: think you can find a seem to test that? 19:31:01 <lbragstad> hrybacki: typically every patch should be accompanied with tests 19:31:25 <hrybacki> ack 19:31:31 <lbragstad> some cases where that's not always the case is ldap stuff (because of integration issues and the slippery slope of mocking and cli stuff) 19:31:33 <gagehugo> I'll find where we are testing invalid credentials 19:42:16 <gagehugo> I don't think there are any current ldap unit tests for authing 19:42:44 <hrybacki> that is a bug in and of itself haha 19:43:00 <lamt> it might be in the functional test 19:43:00 <hrybacki> confused why steve gave +2 here: https://review.openstack.org/#/c/477638/ but left corrections in his comments 19:43:07 <lamt> if deals with ldap 19:46:28 <gagehugo> nope 19:46:43 <cmurphy> hey guys 19:46:47 <cmurphy> how is bug day going? 19:46:51 <lbragstad> cmurphy: o/ 19:47:01 <hrybacki> o/ 19:48:25 <hrybacki> lots of patches to review here cmurphy: https://goo.gl/H86e37 19:48:47 <lbragstad> #link https://review.openstack.org/#/c/473245/3 19:48:54 <lbragstad> ^ that's a good one, too 19:49:09 <cmurphy> hrybacki: lbragstad sweet 20:10:04 <lbragstad> #link https://review.openstack.org/#/c/473245/3 has some good oauth tests in it if anyone is interested 20:10:21 <hrybacki> lbragstad: reviewing that now 20:10:48 <lbragstad> hrybacki: awesome 20:14:26 <hrybacki> I love how documented these test are 20:19:40 <lbragstad> yeah - that's nice 20:20:43 <lbragstad> samueldmq: it'd be good to get your opinion on https://review.openstack.org/#/c/475472/ when you have a minute 20:46:49 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: Validate rolling upgrade is run in order https://review.openstack.org/437441 20:47:17 <lbragstad> cc cmurphy hrybacki gagehugo ^ 20:54:12 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: Validate rolling upgrade is run in order https://review.openstack.org/437441 21:00:54 * samueldmq is back 21:02:16 <samueldmq> lbragstad: looking 21:04:03 <hrybacki> Have to head to my next meeting -- will finish looking over ^ first thing in the morning! 21:09:02 <morgan> I saw a ping of my name in the meetings ng 21:09:08 <sjmc7> afternoon, folks. we’ve been doing some scale/stress testing and seen high request times listing projects (specifically, GET /users/<userid>/projects) when a user has access to a lot of them (0.1 seconds with a couple of assignments up to 4-5 seconds with 500). obviously there’s much more data being pulled out of the database, but flat out listing the projects is very fast, so my guess is it’s the role assignment calculations taking the time 21:09:15 <morgan> Something about options? 21:09:35 <sjmc7> does anyone have any suggestions if there’s something i can tweak or optimize? i know it’s not a common scenario 21:10:11 <morgan> sjmc7: yep it is the role calculation. Unfortunately that is going to be somewhat slow :( 21:10:34 <sjmc7> no magic SQL index i can add? :) 21:10:54 <morgan> Not really. We can't do joins because that information could be in different backends 21:11:28 <lbragstad> morgan: ah - we were wondering when we can remove https://github.com/openstack/keystone/blob/9070172084fe31c9564de38886662fb198de68cb/keystone/conf/eventlet_server.py 21:11:30 <morgan> User might be LDAP, project might be database, and roles could be something else. :( 21:11:37 <sjmc7> yeah, that makes sense 21:11:38 <morgan> lbragstad: remove it 21:11:46 <morgan> lbragstad: eventlet is dead 21:12:00 <morgan> For keystone * 21:12:04 <lbragstad> morgan: ok - so those weren't there for backwards compat? 21:12:35 <morgan> Might have been, we used eventlet for testing for a while after we stopped supporting it for deployment 21:12:44 <morgan> Might have been that. We don't do that anymore 21:12:49 <morgan> (or shouldn't) 21:12:53 <lbragstad> we still have the eventlet entry points 21:13:20 <morgan> Eventlet should be dead, no? 21:13:35 <morgan> We don't import it, we don't depend on ot 21:13:44 <lbragstad> morgan: well - i can run keystone locally using keystone-wsgi-admin 21:13:53 <morgan> Again, I say make the last vestige disappear. 21:15:06 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: Document and add release note for HEAD APIs https://review.openstack.org/478284 21:15:47 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: Document and add release note for HEAD APIs https://review.openstack.org/478284 21:16:03 <lbragstad> morgan: ok 21:18:07 <lbragstad> cc cmurphy samueldmq hrybacki gagehugo ^ that one closes another bug 21:19:34 <sjmc7> morgan: do you know where the time’s getting spent? querying role assignments is also very fast - is there a lot of manipulation of the result going on in code? 21:20:20 <lbragstad> sjmc7: do you happen to have caching enabled? 21:21:04 <sjmc7> no, we don’t 21:21:33 <lbragstad> sjmc7: that may have a significant impact on your results 21:22:13 <sjmc7> yeah, i imagine it would. i’ll give that a go, thanks 21:26:26 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: Remove duplicate list() call when list projects https://review.openstack.org/478286 21:26:30 <lbragstad> sjmc7: ^ looking at the code now 21:27:05 <sjmc7> lbragstad: thanks. i had a look, and it looked pretty straightforward (retrieve the assignments, retrieve all the projects by id) 21:28:01 <sjmc7> but there’s something goign on - listing assignments with GET /role_assignments?user.id takes around 0.1 seconds and GET /projects is about 0.2 (including outputting to the terminal) 21:28:33 <sjmc7> again, i know this isn’t exactly a common case that’d be optimized for 21:28:46 <lbragstad> we seem to jump from https://github.com/openstack/keystone/blob/a1976aa2c9b70de30ea6f646b430bd49f82dbcc1/keystone/assignment/core.py#L232 21:28:58 <lbragstad> right to the resource driver https://github.com/openstack/keystone/blob/a1976aa2c9b70de30ea6f646b430bd49f82dbcc1/keystone/resource/backends/sql.py#L85-L93 21:29:33 <sjmc7> yeah 21:29:46 <lbragstad> sjmc7: do you have any insight or ability to time each line in https://github.com/openstack/keystone/blob/a1976aa2c9b70de30ea6f646b430bd49f82dbcc1/keystone/assignment/core.py#L226 ? 21:30:56 <sjmc7> yeah, i can do that 21:31:56 <lbragstad> sjmc7: hmmmm 21:32:07 <sjmc7> i think this might be an old version (newton?) but it doesn’t look like it’s changed much 21:32:15 <lbragstad> sjmc7: since we don't define that method in the resource manager, i wonder if it's even possible to cache it ? 21:32:20 <samueldmq> lbragstad: done in the GET/HEAD reviews 21:32:38 <sjmc7> # TODO(henry-nash): We might want to consider list limiting this at some 21:32:39 <sjmc7> # point in the future. 21:32:40 <samueldmq> I will need a bit of time to test that one in the db_sync check in my environment 21:32:41 <sjmc7> someoen was here before me :) 21:32:53 <lbragstad> samueldmq: awesome - thanks for the feedback 21:32:59 <samueldmq> sure 21:34:23 <lbragstad> sjmc7: we typically implement caching based on method arguments 21:34:26 <openstackgerrit> Gage Hugo proposed openstack/keystone master: Clarify LDAP invalid credentials exception https://review.openstack.org/475929 21:34:27 <lbragstad> sjmc7: like this https://github.com/openstack/keystone/blob/a1976aa2c9b70de30ea6f646b430bd49f82dbcc1/keystone/token/provider.py#L180 21:35:09 <gagehugo> lbragstad hrybacki ^ 21:35:18 <lbragstad> gagehugo: running tests on it now 21:35:33 <gagehugo> also I noticed that one added test is being ran 8 times 21:36:10 <lbragstad> gagehugo: it's probably because that test module is being inherited in other places 21:36:17 <gagehugo> lbragstad yeah 21:36:40 <gagehugo> I wonder if that could be cleaned up? 21:36:42 <lbragstad> i have a hard time wrapping my head around the dependencies of ldap/sql test modules 21:36:49 <lbragstad> gagehugo: oh - i'm sure it could 21:37:23 <lbragstad> sjmc7: since we don't have those methods defined in the resource manager, i'm not sure it's possible to cache them 21:37:34 <sjmc7> that’s sad :( 21:37:39 <lbragstad> sjmc7: i'd be curious to see if your results change with caching enabled 21:37:47 <lbragstad> sjmc7: if not - that's certainly a bug 21:38:20 <lbragstad> sjmc7: and we can get a fix up easy enough and possibly backported to ocata (we won't be able to backport to newton though) 21:38:51 <lbragstad> sjmc7: it was likely missed because the method is defined in the driver and there isn't much business logic to move it up to the manager 21:40:56 <sjmc7> ok. just generating timing info now 21:42:53 <cmurphy> I don't really know what to make of https://review.openstack.org/#/c/466567 - I'm trying to curl those APIs and they're broken because there's no pki_setup 21:44:18 <sjmc7> lbragstad: looks like all the time’s taken inside list_role_assignments 21:44:21 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: Move list projects from ids to manager and cache https://review.openstack.org/478293 21:45:02 <lbragstad> sjmc7: that's a total wild guess 21:45:04 <lbragstad> ^ 21:45:55 <lbragstad> sjmc7: here - https://github.com/openstack/keystone/blob/a1976aa2c9b70de30ea6f646b430bd49f82dbcc1/keystone/assignment/core.py#L227 ? 21:46:05 <sjmc7> yep 21:46:11 <sjmc7> i’ll jam some timing info in there 21:47:02 <lbragstad> sjmc7: hmm - https://github.com/openstack/keystone/blob/a1976aa2c9b70de30ea6f646b430bd49f82dbcc1/keystone/assignment/backends/sql.py#L190-L247 doesn't look trivial 21:47:39 <sjmc7> no, but calling GET /role_assignments?user.id= *is* quick 21:50:55 <lbragstad> sjmc7: hmm - which eventually calls into https://github.com/openstack/keystone/blob/a1976aa2c9b70de30ea6f646b430bd49f82dbcc1/keystone/assignment/core.py#L882 21:53:13 <sjmc7> those role assignment functions do get complicated quick 21:53:45 <lbragstad> sjmc7: both of those API end up calling the same driver method 21:53:50 <sjmc7> yeah :( 21:54:32 <gagehugo> lbragstad I'm getting 2 failures, I wonder if I can move this test to another class where it won't be ran 8 times 21:54:45 <lbragstad> gagehugo: yeah - i got a couple, too 21:55:29 <lbragstad> sjmc7: i wonder if it's a difference of using `effective` or not 21:55:32 <lbragstad> sjmc7: https://github.com/openstack/keystone/blob/a1976aa2c9b70de30ea6f646b430bd49f82dbcc1/keystone/assignment/core.py#L228 21:55:56 <sjmc7> yeah, there’s a branch on that 21:56:00 <sjmc7> gonna put some more timing around it 21:56:09 <lbragstad> the GET /role_assignment?user.id= API doesn't calculate effective role assignments unless you ask it to 21:56:25 <lbragstad> sjmc7: but... list_projects_for_users does no matter what 21:56:32 <sjmc7> ah, just spits them out from the DB 21:57:19 <sjmc7> the list_effective function i can see would be expensive 21:58:37 <lbragstad> here - https://github.com/openstack/keystone/blob/a1976aa2c9b70de30ea6f646b430bd49f82dbcc1/keystone/assignment/core.py#L931-L935 21:58:49 <lbragstad> yeah - maybe time that? 21:59:07 <sjmc7> yep 21:59:44 <lbragstad> sjmc7: https://github.com/openstack/keystone/blob/a1976aa2c9b70de30ea6f646b430bd49f82dbcc1/keystone/assignment/core.py#L692 looks intense, too 22:03:12 <sjmc7> yep :) 22:05:12 <sjmc7> yeah, so calling /users/abc/projects list_effective is taking around 4 seconds. list_direct is very fast 22:05:39 <lbragstad> sjmc7: so - sounds like effectively role assignments could be improved 22:06:24 <sjmc7> yeah, looks like it. it seems it’s unlikely to be the database since wit a few hundred rows even a full table scan will be very quick 22:06:53 <lbragstad> yeah - effective role assignment probably has a lot of marshalling in python 22:07:02 <lbragstad> which is more likely the case 22:08:37 <lbragstad> sjmc7: would you be able to open a bug against keystone and include your timing? 22:08:55 <sjmc7> yeah, will do 22:09:05 <lbragstad> sjmc7: thanks 22:09:47 <lbragstad> hrybacki: gagehugo cmurphy lamt that about does it for office hours 22:09:50 <lbragstad> thanks for coming! 22:09:54 <lbragstad> #endmeeting