Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-12-06
15:49:00 dansmith we do have some "if cellsv1: then do_old_thing()" escapes in that listing logic
15:49:01 dansmith so maybe that's it
15:49:07 mriedem oh wait,
15:49:08 mriedem yup
15:49:15 dansmith yeah, okay
15:49:16 mriedem when listing instances, if cellsv1 we do the old thing
15:49:26 mriedem we do look in cell0 though
15:49:30 dansmith yeah
15:49:31 mriedem so you'll need the cell0 mapping at the top yes?
15:49:32 mriedem ok
15:49:34 dansmith yes
15:49:38 mriedem belmoreira: ^
15:49:46 dansmith belmoreira: you will want the record there anyway, at some point
15:49:47 mriedem scheduler and api don't use cell mappings until pike
15:50:02 dansmith belmoreira: also, while you're here.. this fix is tanking the cellsv1 job: https://review.openstack.org/#/c/523187/
15:50:06 dansmith which is, like, not a good sign,
15:50:11 belmoreira yeah, cell0 I defined
15:50:13 dansmith but I'm not sure why, especially since you said it works
15:50:36 dansmith like, 100% fail on anything that hits nova
15:50:40 dansmith which seems hard to believe
15:55:22 belmoreira dansmith: I'm running this patch already. Haven't detected any issue yet. However, the functionality that we expose is very limited. Maybe that's why.
15:55:24 huanxie Hi jaypipes, I have updated the reno to make it precise and please help review it again https://review.openstack.org/#/c/451641/ Thanks a lot :)
15:55:45 mriedem belmoreira: can't even build an instance it looks like
15:55:47 jaypipes huanxie: already +2d.
15:55:48 dansmith belmoreira: I dunno, this seems to fail everything all the time, but I can't really see any errors in the logs
15:55:49 mriedem times out waiting to go to ACTIVE
15:56:17 huanxie Many thanks jaypipes :)
15:56:21 belmoreira dansmith: but, as you said, not a good sign :) Will keep eyes open on this
15:56:58 dansmith well, anyway, I don't want to spend too much time on it, but unless we figure that out, we can't merge it
15:57:09 belmoreira dansmith: definitely create/delete instances is working for us with this patch
15:57:44 dansmith although that's ocata I guess
15:57:45 belmoreira but I'm running newton
15:57:49 dansmith oh, newton
15:58:07 dansmith well, still, I'd expect to see an error somewhere if this was actually blowing something up
15:58:19 belmoreira in pike I will not need this patch
15:59:12 openstackgerrit Merged openstack/nova master: Add a new check to volume attach https://review.openstack.org/525622
15:59:43 mriedem dansmith: looking at the instance create flow, if cellsv1 the api will create the instance before casting to build_instances in conductor,
15:59:49 mriedem build_instances in conductor creates the instance mapping
16:00:13 mriedem well, api creates the instance mapping
16:00:38 mriedem conductor gets the host mapping for the chosen host and pulls the cell mapping from that host mapping to set on the instance mapping
16:00:41 mriedem then deletes the build request
16:00:53 mriedem ah,
16:01:03 mriedem and if the build request is already destroyed when conductor tries to do it,
16:01:07 mriedem conductor deletes the instance mapping
16:01:11 mriedem which the api relies on i think
16:01:40 mriedem https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L599-L605
16:01:43 dansmith oh you think we're deleting the build request early enough that it just nukes the instance when it tries because it assumes the user did it?
16:02:34 mriedem maybe
16:02:46 mriedem and when getting the instance from the api, we'll pull it from the top cell https://github.com/openstack/nova/blob/master/nova/compute/api.py#L2221
16:02:52 dansmith the update at top thing should really happen pretty late, after the cell sync, so I didn't think that was a problem
16:03:06 mriedem update at top happens for any state change doesn't it?
16:03:23 mriedem https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L573
16:03:41 mriedem so ^ should trigger an update at the top
16:03:44 mriedem which will delete the build request
16:04:01 mriedem and then later we'll get build request not found which will make conductor delete the instance mapping https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L604
16:04:07 mriedem thinking the instance was deleted by the user during build
16:04:42 mriedem yup, and https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L605 means we don't cast to the compute to build the instance
16:04:43 dansmith yeah, but I expected it to happen late enough
16:05:52 mriedem i think i get a cookie now
16:06:00 mriedem frosted sugar with sprinkles please
16:06:01 dansmith weyl.. I'm not sure what to do, other than to weaken conductor's interpretation of the BR being missing if cellsv1
16:06:21 mriedem can we conditionally delete the build request in cells/messaging based on the instance state?
16:07:00 dansmith well, which state?
16:07:06 ildikov mriedem: I'm at a place selling cupcakes one of which is called 'Uniporn & Rainho', sucks I can't send over one through IRC :/
16:07:21 mriedem dansmith: is ACTIVE too late?
16:07:47 dansmith I would think,
16:07:53 dansmith since we might not even make it to ACTIVE
16:08:10 mriedem i'm not even sure why we need this
16:08:14 dansmith what if we do it only if save() succeeded
16:08:15 mriedem given https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L597
16:08:50 dansmith mriedem: I think it's because that might've happened in a separate api db
16:08:57 mriedem we could alternatively check if we've set the cell mapping on the instance mapping, meaning we've picked a host, https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L509 but that's racy
16:09:19 mriedem because then conductor and cells/messaging are racing to delete the build request
16:09:30 mriedem but cells/messaging will only do it on the next update at top
16:09:40 mriedem which presumably would come after conductor has set the mapping and already deleted the build request
16:09:57 dansmith if they're sharing an api db, but not the cell db,
16:10:13 mriedem hmm, so the thing conductor deletes isn't necessarily the same thing that cells/messaging deletes?
16:10:24 dansmith then an update will delete the BR in the shared api db, which will then make conductor delete from the cell db, which will sync back up as a delete to the top level db
16:10:44 dansmith but if they're separate api dbs,
16:10:45 dansmith then you have to sync the BR delete up or you shadow the actual instance
16:10:52 dansmith since we were in the cell db when we did the BR delete
16:11:27 dansmith mriedem: you wanna borrow my rusty spoon when I'm done with it?
16:11:35 mriedem for your eyeballs?
16:11:37 dansmith yes
16:11:43 mriedem sure
16:11:46 mriedem send'er over
16:11:48 dansmith heh
16:11:55 mriedem pre-gooped please
16:14:10 dansmith mriedem: anyway, I want to fix this, but I can't really justify spending time on it ahead of other stuff on my plate
16:14:37 dansmith the patch is there for people to apply if they need it for that scenario, and maybe I can circle back after FF or something
16:17:05 belmoreira dansmith: +1, for me this patch makes sense in older versions (newton, ocata). Not sure how useful it will be in Pike, Queens.
16:17:10 belmoreira dansmith mriedem thanks. I will keep you posted
16:19:02 mriedem dansmith: edleafe: so after careful contemplation in the last 5 minutes,
16:19:08 mriedem i think we can unpin the Selection object patch
16:19:21 dansmith okay
16:19:31 mriedem my concerns in the top patch in the series about filtering out the hosts that have already been tried can be done in other ways using the filter_properties retry list of hosts
16:19:39 mriedem i.e. knowing what's already been claims
16:19:41 mriedem *claimed
16:20:22 bauzas I need to look at those Selection changes me too
16:20:55 dansmith gdi gerrit

Earlier   Later