Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-09
02:41:48 melwitt like if we're deleting from here https://github.com/openstack/nova/blob/master/nova/compute/api.py#L1799
02:42:10 melwitt this would be if it never landed on a host
02:43:14 melwitt we're not doing anything with quotas until L1842 but you have to have reserve and commit for a delete too
02:43:18 mriedem yeah i was looking at _lookup_instance https://github.com/openstack/nova/blob/master/nova/compute/api.py#L1660
02:43:56 mriedem and we pull it from cell0 here https://github.com/openstack/nova/blob/master/nova/compute/api.py#L1690
02:44:47 mriedem looks like we need something like this https://github.com/openstack/nova/blob/master/nova/compute/api.py#L1729-L1756
02:44:56 melwitt I see that _delete_while_booting takes care of quotas correctly, it's just we're missing any quota handling at all at L1799
02:45:12 melwitt yes, agree
02:47:51 mriedem should be relatively straightforward
02:48:43 melwitt yeah, I think so too
02:50:42 melwitt thanks Kevin_Zheng for alerting about that
02:51:27 Kevin_Zheng melwitt: your are welcome, still try to catch all you guys discussed :)
02:52:02 melwitt Kevin_Zheng: mriedem added a comment to the lp bug to explain what was concluded
02:52:20 melwitt it turned out to be something pretty simple
02:53:07 melwitt that particular local delete case is missing quota cleanup
02:53:40 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Decrement quota when deleting an instance in cell0 https://review.openstack.org/443395
02:53:41 mriedem melwitt: Kevin_Zheng: ^ w/o any tests run or anything
02:56:38 Kevin_Zheng https://review.openstack.org/#/c/443003
02:56:54 Kevin_Zheng ^ seems we have already tried this but didnt work?
02:58:02 melwitt gah, I didn't look at the patch first
02:59:02 Kevin_Zheng we tested manually
02:59:30 Kevin_Zheng fix like this actually decrease usage in cell0
02:59:36 melwitt I'm trying to understand why that code is going to cell0
03:01:20 melwitt mriedem: ^
03:01:42 mriedem yeah idk
03:01:51 mriedem are we mutating the context somewhere?
03:03:24 melwitt I thought probably not. I'm not sure if we yet merged the thing method that targets a context and lets it continue on targeted
03:03:32 melwitt and also I didn't think it would be used in this case
03:08:04 mriedem that definitely came later
03:08:07 mriedem set_target_cell
03:09:02 melwitt grr this doesn't make sense
03:12:12 mriedem i would have to recreate locally and tinker with this,
03:12:15 mriedem which i don't have time tonight
03:12:29 mriedem Kevin_Zheng: the instance is in cell0 though correct?
03:12:33 mriedem and the build_request is deleted?
03:15:30 Kevin_Zheng yes, we are have meeting now, but I will try to dig more if you guys didn't have tien
03:15:32 Kevin_Zheng Time
03:15:57 mriedem melwitt: i think i found the problem
03:16:06 melwitt do tell
03:16:18 mriedem gather round children
03:16:24 melwitt heh
03:16:26 mriedem so when the REST API looks up the instance via id
03:16:34 mriedem it goes down to compute.API.get()
03:16:59 mriedem eventually to _get_instance
03:17:12 mriedem we get the cell0 instance mapping here https://github.com/openstack/nova/blob/fa2b4a82648101826566da68dd56d204e269853f/nova/compute/api.py#L2292
03:17:14 Kevin_Zheng Ah
03:17:22 mriedem and then we mutate the context here https://github.com/openstack/nova/blob/fa2b4a82648101826566da68dd56d204e269853f/nova/compute/api.py#L2294
03:17:33 mriedem and forever after, that context object is targeted to cell0
03:17:59 melwitt argh, so we are already using set_target_cell
03:18:43 Kevin_Zheng Seems right
03:20:06 mriedem dansmith: ^ set_target_cell might be screwing us here
03:20:15 mriedem melwitt: on master yes
03:20:17 mriedem but not ocata
03:20:58 melwitt I wonder if we should avoid using it until everything that needs to move to api db moves, or try to just patch around it for now?
03:21:00 mriedem we still have the bug in ocata
03:21:11 mriedem but i think that's the reason why the guys patch didn't work
03:21:17 mriedem and decremented from cell0
03:21:34 dansmith the only time I've experienced this is if the main db for the control services is set to cell0
03:21:51 dansmith it shouldn't happen with a single cell and database set to that db right?
03:22:47 mriedem i believe this is single node devstack
03:22:48 Kevin_Zheng We meet this in devstack env, didn't test in other cases yet
03:22:48 melwitt dansmith: this is the quota_usages and reservations tables, so it's updating usages in cell0
03:23:05 mriedem dansmith: yeah see https://review.openstack.org/443395
03:23:08 melwitt after set_target_cell targets the context to cell0
03:23:25 mriedem which doesn't work because it tries to decrement from cell0 because the context is mutated to use cell0 when we lookup the instance here https://github.com/openstack/nova/blob/fa2b4a82648101826566da68dd56d204e269853f/nova/compute/api.py#L2294
03:24:47 dansmith I'm confused, nothing ever moves between cell0 and another cell..
03:24:48 mriedem we make the reservation on quota in the nova db in the api
03:25:00 mriedem then conductor fails to schedule and creates the instance in cell0
03:25:03 mriedem and deletes the build request
03:25:10 dansmith ah
03:25:11 mriedem when you go to delete the instance, we dont find the build request,
03:25:18 mriedem but we find the instance mapping to cell0
03:25:22 melwitt dansmith: this is a delete of an ERROR instance in cell0. the REST API does a get of the instance, which set_target_cell to cell0 on the context, then that same context is passed along to quotas.commit() and it decrements usage in the wrong db
03:25:22 mriedem and we delete the instance from the cell0 db
03:25:27 mriedem but we don't decrement the quota
03:25:47 mriedem melwitt: well, w/o https://review.openstack.org/#/c/443395/ we don't even attempt to decrement quota
03:25:47 melwitt dansmith: (after the person tried to add the missing quota cleanup)
03:25:49 dansmith and this is because we don't have the reservations in the api I guess?
03:25:53 mriedem dansmith: yup
03:26:18 dansmith awesome
03:27:27 melwitt mriedem, dansmith: right. so 1) we're missing quota cleanup 2) when we add quota cleanup, it cleans the wrong db because the context was targeted to cell0 early on
03:27:28 dansmith has melwitt pointed out that counting fixes all this nonsense?
03:27:28 mriedem yes
03:27:28 dansmith okay good
03:27:28 melwitt dansmith: yeah. but we need a backportable fix
03:27:28 mriedem but we can't backprot that to ocata
03:27:28 openstackgerrit Tony Breeds proposed openstack/os-vif master: Add Constraints support https://review.openstack.org/413325
03:27:28 mriedem so we need 2 fixes
03:27:28 dansmith yeah, so we probably need a hacky thing to just go find the cell a thing is in (which will only ever be two places to check) right?
03:27:28 mriedem the set_target_cell one for master only,
03:27:28 dansmith for ocata
03:27:31 mriedem and then https://review.openstack.org/#/c/443395/ or the other patch for ocata
03:27:43 mriedem we know what cell it's in already
03:27:52 mriedem and we delete the instance, that's all fine
03:27:58 mriedem we just never decrement quota in that path, which https://review.openstack.org/#/c/443395/ does
03:28:06 mriedem it just doesn't work on master b/c of set_target_cell
03:28:12 dansmith okay
03:28:15 dansmith yeah
03:28:27 mriedem was there a reason we needed https://github.com/openstack/nova/blob/fa2b4a82648101826566da68dd56d204e269853f/nova/compute/api.py#L2294 ?
03:28:35 melwitt mriedem: yeah, I was thinking consider taking the first patch for ocata https://review.openstack.org/#/c/443003

Earlier   Later