| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-09 | |||
| 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 ^ | |
| 03:57:37 | mriedem_away | we can start unwinding the super complicated spaghetti code that was there for the transition | |
| 03:57:55 | melwitt | okay, cool | |
| 03:58:17 | mriedem_away | now i'm really leaving | |
| 03:58:20 | mriedem_away | o/ | |
| 03:58:36 | melwitt | gnight o/ | |
| 05:21:42 | openstackgerrit | ChangBo Guo(gcb) proposed openstack/nova master: Enable more virt.xenapi tests on Python 3 https://review.openstack.org/414421 | |
| 05:45:46 | openstackgerrit | Edu Alcaniz proposed openstack/nova master: follow-up https://review.openstack.org/443424 | |
| 05:46:16 | openstackgerrit | jichenjc proposed openstack/nova master: Trivial Fix https://review.openstack.org/442310 | |
| 06:06:25 | openstackgerrit | jichenjc proposed openstack/nova master: Add description for Image location in snapshot https://review.openstack.org/440379 | |
| 06:14:01 | openstackgerrit | jichenjc proposed openstack/nova master: check hw:mem_page_size when change extra_spec https://review.openstack.org/408610 | |
| 06:14:32 | openstackgerrit | jichenjc proposed openstack/nova master: no instance info cache update if instance deleted https://review.openstack.org/363585 | |
| 06:53:46 | openstackgerrit | ChangBo Guo(gcb) proposed openstack/nova master: Add volume status check when deleting instances https://review.openstack.org/260339 | |
| 07:49:00 | arne_r | markus_z: Hi, care to give this https://review.openstack.org/#/c/436868/ your blessing? | |
| 07:50:23 | markus_z | arne_r: I'm going to check it within the next hours | |
| 07:52:45 | arne_r | thanks =) | |
| 08:26:36 | Kevin_Zheng | melwitt, mriedem maybe it is good to add tempest test for this: create instance -> error -> delete -> check quota after this fixed, to avoid same thing happen again | |
| 08:26:39 | Kevin_Zheng | ? | |
| 08:33:22 | markus_z | arne_r: It needs a small change, IMO | |
| 08:41:32 | openstackgerrit | melanie witt proposed openstack/nova master: Make Quotas object favor the API database https://review.openstack.org/410945 | |
| 08:41:32 | openstackgerrit | melanie witt proposed openstack/nova master: Count resources to check quota for cells https://review.openstack.org/416521 | |