| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-09 | |||
| 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 | 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 | |