| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-09 | |||
| 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 | melwitt | dansmith: this is the quota_usages and reservations tables, so it's updating usages in cell0 | |
| 03:22:48 | Kevin_Zheng | We meet this in devstack env, didn't test in other cases yet | |
| 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 | mriedem | and we delete the instance from the cell0 db | |
| 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:27 | mriedem | but we don't decrement the quota | |
| 03:25:47 | melwitt | dansmith: (after the person tried to add the missing quota cleanup) | |
| 03:25:47 | mriedem | melwitt: well, w/o https://review.openstack.org/#/c/443395/ we don't even attempt to decrement quota | |
| 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 | for ocata | |
| 03:27:28 | mriedem | the set_target_cell one for master only, | |
| 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 | so we need 2 fixes | |
| 03:27:28 | openstackgerrit | Tony Breeds proposed openstack/os-vif master: Add Constraints support https://review.openstack.org/413325 | |
| 03:27:28 | mriedem | but we can't backprot that to ocata | |
| 03:27:28 | melwitt | dansmith: yeah. but we need a backportable fix | |
| 03:27:28 | dansmith | okay good | |
| 03:27:28 | mriedem | yes | |
| 03:27:28 | dansmith | has melwitt pointed out that counting fixes all this nonsense? | |
| 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: Temporarily untarget context when deleting from cell0 https://review.openstack.org/443403 | |
| 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: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 | |
| 03:57:20 | mriedem_away | melwitt: one last thing that i thought of while looking through all of this ^ | |