Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-05-15
19:28:34 mnaser specifically here https://github.com/openstack/nova/blob/575a91ff5be79ac35aef4b61d84c78c693693304/nova/quota.py#L1194-L1195
19:28:55 dansmith apparently mysql always indexes foreign key columns, so cell_id should be indexed
19:29:08 mnaser https://www.irccloud.com/pastebin/MftvYe3g/
19:29:10 mnaser thats the EXPLAIN
19:29:23 dansmith mnaser: you specifically have the subset conf turned on, is that intentional?
19:29:53 mnaser dansmith: sorry, nto following, subset conf?
19:30:09 dansmith mnaser: see the comment just above the code in instance_list ?
19:30:16 dansmith query_cell_subset = CONF.api.instance_list_per_project_cells
19:30:25 dansmith if that is off then it won't do this
19:31:01 dansmith defaults to false
19:31:36 dansmith heh, the conf text (written by me I'm sure) says turning that on "causes one more (fast) query to the API database"
19:31:48 mnaser heh, let me check if its enabled
19:31:55 dansmith maybe I should edit that comment or say "assuming you're archiving regularly" :)
19:32:08 mnaser instance_list_per_project_cells is not set to anything so its probably set to default False
19:32:26 dansmith mnaser: okay well you see the code, not sure how you could be runningthat
19:32:41 mnaser dansmith: https://github.com/openstack/nova/blob/575a91ff5be79ac35aef4b61d84c78c693693304/nova/quota.py#L1194-L1195 ?
19:32:55 mnaser that doesnt seem to respect it
19:33:04 dansmith mnaser: no, the instance list one
19:33:17 dansmith maybe you're not actually hitting the instance_list code, just the quota one?
19:33:29 mnaser dansmith: yes, sorry, should have clarified, i think that's the running theory
19:33:36 dansmith okay I thought you assumed both
19:33:44 dansmith I wrote instance_list so was focusing there
19:33:44 mnaser i'm looking at logs and os_volume-boot is taking some ~45-60s-ish to respond
19:33:56 mnaser yeah, initially that was my two findings, but i agree, it seems quota code might be what is hurting
19:34:03 dansmith gotcha
19:34:05 dansmith melwitt: ^
19:34:33 dansmith mnaser: maybe we could make the quota code check that same conf, as for <=1 real cell, it's probably quicker not to do the query anywa
19:35:21 mnaser dansmith: yes, given we want to backport though, i'd be tempted to actually change get_by_project_id behaviour to respect that config value so its not possible for someone to miss it later
19:35:39 dansmith no, we definitely don't want that
19:35:40 mnaser (for the future, but backport just fixing the behaviour in quota.py)
19:36:03 mnaser im sure there's details im missing why its not a good idea :)
19:36:07 dansmith if a caller is looking for a set of instance mappings, you want it to get what it asked for, not altered by some random config flag
19:37:54 mnaser yep makes sense
19:38:12 mnaser the patch to fix this seems trivial, so ill push something up
19:38:17 dansmith that's a very low-level database query routine.. the "should we optimize for lots of cells or not" is a very high-level decision
19:38:38 dansmith mnaser: cool, add me and melwitt, but the latter is the important review I thnk
19:39:50 mnaser dansmith: will do. i'll write up a bug right now too
19:40:02 dansmith thanks
19:40:06 mnaser dansmith: i could also workaronud this by enabling quota from placement, but yeah
19:40:08 mnaser let's get it fixed
19:40:22 dansmith mnaser: you could also archive your damned database now and then :D
19:40:32 mnaser dansmith: haha, always me bringing up those massive db issues :p
19:40:57 dansmith I guess the benefit of not archiving is giving us some data on these things, so ... your choice :D
19:53:27 mnaser it's friday and i did a bad job at writing this probably: https://bugs.launchpad.net/nova/+bug/1878979
19:53:27 openstack Launchpad bug 1878979 in OpenStack Compute (nova) "Quota code does not respect [api]/instance_list_per_project_cells" [Undecided,New]
20:08:59 mriedem mnaser: related https://bugs.launchpad.net/nova/+bug/1857306
20:08:59 openstack Launchpad bug 1857306 in OpenStack Compute (nova) "_bury_in_cell0 could not handle instance duplicate exception" [Low,Triaged]
20:09:10 mriedem wait a minute, that's not right
20:09:34 mriedem https://review.opendev.org/#/c/700456/
20:12:38 mnaser mriedem: ehhhh, that's a lot to think about for friday :(
20:13:46 mriedem you're stuck inside, what else do you have to do
20:37:23 mnaser welp, _instances_cores_ram_count_legacy has zero tests
20:47:03 mriedem i think those are covered with functional tests
20:47:11 mriedem depending on if the placement flag is configured to use placement for quota or not
20:47:28 mnaser mriedem: yeah i found some unit test ones but they mostly mock that function out
20:47:45 mriedem yeah it's functional tests i'm pretty sure
20:47:53 mriedem more reliable for those kind of tests hitting the dbs
20:47:57 mnaser mriedem: ok thanks ill go dig there
20:48:24 mnaser mriedem: my test is mostly "if flag X is enabled then make sure you call with all cells otherwise make sure you retrieve list of cells
20:49:25 openstackgerrit Mohammed Naser proposed openstack/nova master: WIP: Make quotas respect instance_list_per_project_cells https://review.opendev.org/728575
20:49:26 mnaser mriedem: ^ for context
20:49:54 mnaser yeah that code is broken now that i look at it again
20:50:23 mnaser ah no nvm, python lets vars defined in if get accessed outside
20:52:42 openstackgerrit Lin Yang proposed openstack/os-traits master: CPU: add traits for new AVX512 support https://review.opendev.org/728576
21:15:29 openstackgerrit Lin Yang proposed openstack/os-traits master: CPU: add traits for new AVX512 support https://review.opendev.org/728576
21:24:06 openstackgerrit Mohammed Naser proposed openstack/nova master: Make quotas respect instance_list_per_project_cells https://review.opendev.org/728575
21:24:16 mnaser dansmith, melwitt: ^ cooked up and tested locally :) thanks for the hints
21:28:36 openstackgerrit Gage Hugo proposed openstack/nova stable/stein: [DNM] - testing numa placement https://review.opendev.org/728579
22:32:48 melwitt mnaser: here's the func test for cores ram count https://github.com/openstack/nova/blob/master/nova/tests/functional/db/test_quota.py#L142
22:33:21 melwitt oh you found it. nevermind
22:35:52 mnaser melwitt: :D
22:36:25 mnaser I tried to make it as backportable as possible
22:37:00 melwitt good call. this doesn't look too bad at all, so thank you for that
22:38:09 melwitt mnaser: I assume you tried out this patch? what was the perf result, just curious
22:39:50 ikla I'm trying to get pci_passthrough to work and followed the documents. I keep seeing Pci claimed failed. Anyway to drill this down. Config looks perfect.
22:40:34 mnaser melwitt: I have not yet actually! It’s Friday and the load dropped so it wasn’t hitting as hard
22:41:07 mnaser I can try on Monday though but I imagine it’ll drop scanning instance mappings and I only have 2 cells really (0 and 1)
22:41:10 melwitt mnaser: I guess, this seems like the opposite of what you wrote in the bug report. it's always limiting to project_id and you're adding some that would look at all cells?
22:42:04 mnaser melwitt: sorry I’ve been pretty slow today. But yes before it was limited to project ID and this just doesn’t limit and checks all cells anywyas
22:42:27 melwitt like before your patch it's always only looking at a particular project_id and quota check is always scoped to a project, so there's no need to look at all cells
22:43:23 melwitt mnaser: if that's the case, that would mean there's a bug in objects.CellMappingList.get_by_project_id if it's not actually scoping down to a project
22:44:28 mnaser melwitt: that function does actually scope by project. The only problem is that to do that, it needs to scan all instance mappings to find all cells for that project which is where I took the performance hit
22:44:55 mnaser After my patch, nova checks all cells, regardless if a user has instances in it or not
22:45:00 melwitt and no worry, I've been slow today too. spent all day/night yesterday dealing with fallout from a nova gate breakage and trying to get it fixed. so exhausted
22:45:18 mnaser Sorry if my commit message or bug aren’t clear, I’m just feeling bleeeh and dazed all day lol
22:47:49 melwitt mnaser: sorry this just doesn't seem to make sense. before your patch, it's always limiting to one project cells. after you patch, it's capable of not limiting. I don't see how that would help with any instance mapping scanning?
22:48:48 melwitt oh hang on
22:49:08 mnaser melwitt: I think we may be talking about two scans. One for the quota calculation and one to find which cells the user has instances in
22:49:19 melwitt ok so you're saying when you can look at all cells, you actually go _faster_ because you don't do the instance mapping thing? wow ok
22:49:39 melwitt yeah ok it's a tradeoff, slowly remembering now
22:49:49 mnaser melwitt: https://github.com/openstack/nova/blob/master/nova/objects/cell_mapping.py#L275-L285
22:49:57 melwitt the project limiting behavior is actually worse for single cell-ers
22:50:00 melwitt yeah, I see now
22:50:01 melwitt sorry
22:50:11 mnaser All good! Yeah, it hurt me more in this case
22:50:24 melwitt yeah, I see
22:50:28 melwitt nice catch
22:50:47 mnaser not the catch I wanted on a Friday but it is what it is :P
22:50:54 melwitt haha right
22:51:13 melwitt ok, I'll review this properly a bit later. thanks for opening the bug and doing the patch

Earlier   Later