Earlier  
Posted Nick Remark
#openstack-nova - 2017-03-09
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
03:28:41 mriedem like, can't we just do the context manager there?
03:28:53 dansmith mriedem: correct,
03:29:07 dansmith mriedem: because the caller will need to do things, like later it might need to lazy-load fault for example
03:29:10 dansmith which happens deep in the view
03:29:22 dansmith long after we've returned from here and have lost all cell scope
03:29:48 dansmith or instance_info_cache sometimes
03:30:14 mriedem ok, so i guess we have to hacky the quota decrement in the delete path
03:30:18 mriedem such that it's untargeted
03:30:31 dansmith and get is used to look up a thing which is later passed to something else that will make an rpc call, etc
03:30:34 dansmith anyway
03:30:36 dansmith yeah,
03:30:46 dansmith I have a hard stop now, but happy to work on that thing tomorrow
03:31:00 mriedem i'll roll something on top of the original fix and he can see if it works
03:31:04 mriedem because i want to stop too
03:31:53 dansmith melwitt: this is a reason to unblock your bottom two patches right?
03:32:16 melwitt dansmith: yes, I think so
03:32:21 melwitt well, no
03:32:37 melwitt dansmith: my patches don't move the usages and reservations tables. just leaves them out
03:32:43 dansmith oh right
03:32:46 melwitt dansmith: sorry, doesn't move the data
03:33:00 dansmith okay, anyway .. tomorrow
03:33:51 melwitt it might still work because we have the tables and the usage might be addressed by the quota resyncing stuff
03:35:59 melwitt like, data migration was not supposed to be needed for the usages. have to read over the original spec again
03:45:56 melwitt yeah, nevermind. that wouldn't work at all bc no methods added to read/write usages and reservations in api db
03:48:28 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Decrement quota when deleting an instance in cell0 https://review.openstack.org/443395
03:48:28 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Temporarily untarget context when deleting from cell0 https://review.openstack.org/443403
03:48:29 mriedem Kevin_Zheng: melwitt: ^ this is pretty ugly, but it's what i'm thinking before going to bed
03:50:20 Kevin_Zheng mriedem: Thanks I will test it :)
03:50:30 Kevin_Zheng mriedem: have a nice sleep
03:50:46 mriedem o/
03:52:06 melwitt mriedem_away: okay, looking
03:57:19 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Start removing _lookup_instance https://review.openstack.org/443405

Earlier   Later