Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-09
02:27:04 Kevin_Zheng will it fix the problem we meet?
02:27:16 melwitt ah, cool. yeah, we didn't know about that one yet and I agree it sounds critical. our intention was that things should be okay in the single cell deployment
02:28:21 mriedem Kevin_Zheng: we'll have to fix the bug separately
02:28:24 melwitt yes, it will fix that problem. but I think we'll have to do a small different fix for now, that we can backport
02:28:26 mriedem because we have to backport the fix
02:28:29 mriedem jinx
02:28:45 Kevin_Zheng yeah sounds reasonable
02:28:45 mriedem so when we bury the instance in cell0,
02:28:46 melwitt hah, yeah
02:28:56 mriedem we have to remove the reservation from cell0
02:28:59 mriedem i think?
02:29:06 mriedem we must be making a claim in cell0 or how does that work?
02:29:14 mriedem s/claim/reservation/
02:30:09 melwitt thinking ... we're not doing anything to target the reservation/usage/etc
02:30:41 melwitt so it should be in cell0 I think. and that's not getting removed ... so I think you're right
02:33:24 mriedem the quota reservation happens in nova-api here https://github.com/openstack/nova/blob/master/nova/compute/api.py#L312
02:33:50 mriedem but since that's not targeted to cell0, wouldn't that be in the main cell, i.e. nova database?
02:34:21 mriedem the commit is here https://github.com/openstack/nova/blob/master/nova/compute/api.py#L1059
02:35:22 mriedem there is no rollback of the quota in _bury_in_cell0
02:35:22 mriedem https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L836
02:35:50 mriedem which probably makes sense because don't we want to keep the reservation until the instance is actually deleted?
02:35:53 melwitt there shouldn't be because ERROR instances occupy quota
02:36:23 melwitt but when the user deletes it, we need to do a reverse reserve and commit and somehow that's going to the wrong db
02:37:18 mriedem we put the instance in cell0 here https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L881
02:37:19 melwitt it's probably something like, we target cell0 to delete the instance and the quota reserve/commit for the freeing of quota goes to cell0 in the same target and we didn't mean to
02:37:31 melwitt or something
02:38:19 mriedem yeah, so we made the reservation in the nova db, but put the instance in the nova_cell0 db, and right when we delete i bet we find it in cell0 via the instance mapping
02:40:11 mriedem https://github.com/openstack/nova/blob/master/nova/compute/api.py#L1708
02:40:12 melwitt yeah, I'm looking in compute/api.py and I'm not seeing any freeing of quota for local deletes
02:40:15 mriedem i think deleted would be False there
02:40:19 mriedem because conductor deleted the build request
02:40:40 mriedem we delete the build request in _bury_in_cell0 here https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L888
02:41:09 melwitt yeah. so far it's looking to me like we're missing a quotas.commit() for the local delete cases
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

Earlier   Later