Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-02-01
15:31:43 dansmith vsaienk0: can you do a canary change against ironic, depending on this? https://review.openstack.org/427772
15:32:51 vsaienk0 dansmith: done https://review.openstack.org/#/c/427675/ waiting for results, thanks!
15:33:25 dansmith vsaienk0: okay, this is the thing I was pointing to yesterday, which I suspect you're hitting. if this fixes it, we'll need to put it in a proper place of course
15:34:25 openstackgerrit Christopher Martin proposed openstack/nova: Stop using mox in test_compute_cells.py https://review.openstack.org/389750
15:34:54 dansmith vsaienk0: oh you guys are a devstack plugin, well that's why I couldn't find stuff... going forward we can just put this in your lib/ironic thing, so .. easier
15:36:35 sfinucan bauzas: Think there's a very minor logging mistake in https://review.openstack.org/#/c/427200, though otherwise LGTM
15:37:10 bauzas sfinucan: you can hurt me with a -1, I won't hit you :p
15:37:41 sfinucan bauzas: I don't usually -1 for things I'm not 100% sure are an issue, but you asked for it ;)
15:37:44 bauzas and I even prefer or I could miss the comments :)
15:38:11 bauzas sfinucan: well, I'm French so arguing is like my 2nd nature
15:39:01 openstackgerrit Matt Riedemann proposed openstack/nova: Ensure build request exists before creating instance https://review.openstack.org/427775
15:39:02 mriedem dansmith: melwitt: bauzas: ^ i'm assuming some sort of transactional lock would be best there, but not sure how to do it
15:40:18 dansmith mriedem: you can't, they're in separate databases
15:40:25 mriedem thought about that
15:40:26 mriedem yeah
15:40:28 dansmith mriedem: you should create the instance, then try to delete the buildreq, and if that fails, delete the instance
15:40:31 dansmith like we do below
15:42:04 mriedem we have a window either way
15:42:16 mriedem i'd rather not create the instance in the first place
15:42:44 mriedem if the build request doesn't exist before we create the instance, this catches it, if the build request doesn't exist after we create the instance, the existing check at the bottom handles it
15:43:20 dansmith mriedem: there's no window this way,
15:43:40 dansmith mriedem: and the check at the bottom will have to undo a bunch of other stuff, and will have done actions, notifications, etc
15:43:44 dansmith mriedem: but your call
15:44:14 dansmith mriedem: actually, yeah, that's fine
15:44:34 mriedem huh? why my change we don't create the instance, and continue, so the actions and notifications don't happen
15:44:54 dansmith mriedem: I meant if you delay the delete until below
15:44:58 mriedem s/why/with/
15:45:42 openstackgerrit Andy McCrae proposed openstack/nova: Allow placement endpoint interface to be set https://review.openstack.org/426163
15:45:43 dansmith since we don't have the instance mapping until a little later, we can't delete the build request yet up there, which means we have to wait
15:47:38 openstackgerrit Lee Yarwood proposed openstack/nova: libvirt: Remove redundant bdm serial mangling and saving during swap_volume https://review.openstack.org/427364
15:47:38 bauzas mriedem: dansmith: catching up your convo
15:47:57 dansmith mriedem: let me try to linearize the mess that came out of my head above
15:48:26 dansmith mriedem: when we create the instance where we do today, it's not find-able as an instance by the api until the instance mapping is created below
15:48:32 bauzas dansmith: mriedem: I think most of the corner cases about synchronizing have been written by alaski in its conductor method
15:48:38 bauzas lemme find it
15:48:54 dansmith mriedem: so if we delete the buildreq due to an external delete,
15:49:23 dansmith mriedem: and then create the instance, it's still not find-able until we add the instance mapping and then delete the buildreq
15:49:55 dansmith mriedem: so there might be a race between creating the mapping and noticing the buildreq is gone, but that is an extremely short time period
15:49:56 dansmith make sense?
15:51:41 mriedem dansmith: yeah, and my patch makes that window even smaller yes?
15:52:30 dansmith mriedem: hang on, I'm reading _get_instance()
15:52:47 mriedem dansmith: are you suggesting we move all of https://github.com/openstack/nova/blob/d308d1f70e7e840b1b0c8f4307998d89f9a5ddff/nova/conductor/manager.py#L925-L949 to right after the instance is created, before the notification and action stuff?
15:53:37 mriedem if we moved that up, we wouldn't need to keep track of bdms to cleanup...
15:54:09 mriedem i'm going to start on fixing the return/continue bug
15:54:48 dansmith mriedem: check this out: https://github.com/openstack/nova/blob/master/nova/compute/api.py#L2295-L2302
15:55:03 dansmith mriedem: that will short-circuit the logic we put in to make this atomic across both dbs
15:55:20 dansmith mriedem: and it says we should remove it once people have to use cellsv2
15:55:39 dansmith that will effectively go straight to the default cell without requiring the buildreq/instmap lock that we have in place
15:55:43 mriedem yeah i saw that last night, and remember specifically asking alaski to put that TODO in there
15:56:07 dansmith so, can we (a) remove that and see how it goes, (b) merge it if it looks good and see if the fails go away?
15:58:43 openstackgerrit Dan Smith proposed openstack/nova: WIP: Remove pre-cellsv2 short circuit in instance get https://review.openstack.org/427782
15:59:17 mriedem dansmith: would we get into that else in this case though? the inst_map could exist and have the cell mapping set
15:59:45 mriedem i guess if that's the case, the cleanup routine in conductor should have already deleted the instance
16:00:13 mriedem crap i havea 1x1, bbiab
16:04:11 dansmith hmm
16:08:01 bauzas dansmith: mriedem: so, I actually wonder why we wouldn't reproduce the logic in build_instances() where when refreshing the instance, we accept that there could be an InstanceNotFound ?
16:08:19 bauzas dansmith: mriedem: but here, we would just access the BuildRequest record instead
16:08:43 bauzas when we get_new_instance()
16:08:57 dansmith bauzas: until we have updated the instance mapping the instance itself could not be deleted, and the buildreq is how that happens
16:09:23 bauzas I'm missing a race condition here then
16:10:20 dansmith the one on L2311 also introduces a race between deleting the map and the instance, so I'll remove that one too
16:11:18 openstackgerrit Ildiko Vancsa proposed openstack/nova: Implement new attach/detach Cinder flow https://review.openstack.org/330285
16:11:19 openstackgerrit Ildiko Vancsa proposed openstack/nova: Remove check_attach https://review.openstack.org/335358
16:13:40 bauzas dansmith: both conditions are for existing instances that needed to have instance mappings, right?
16:14:03 bauzas dansmith: for a fresh new ocata instance, we suppose the mapping is created, right?
16:14:40 dansmith bauzas: as of ocata, all instances old and new have said mappings, yes
16:15:13 bauzas okay, so I agree with you that we should wipe those conditionals
16:15:22 bauzas because it's confusing
16:16:45 dansmith s/confusing/wrong.
16:25:08 bauzas dansmith: now, say that we remove those conditionals, a GET /servers?uuid=<> would return a 404 in case the BuildRequest is deleted, right?
16:25:48 bauzas dansmith: if so, I think you found the best approach for fixing the issue, kudos
16:25:52 dansmith bauzas: if it's unmapped
16:26:04 bauzas dansmith: but this is mapped only after scheduling, right?
16:26:54 bauzas I mean
16:26:54 dansmith bauzas: yes
16:27:01 bauzas someone deletes an instance
16:27:07 bauzas then it deletes the mappings
16:27:17 bauzas so, bim
16:27:43 bauzas we go to the mappings equal None and then baaam, BuildRequestNotFound, pif paf 404
16:28:04 bauzas unless I missed that
16:28:11 bauzas s/that/something
16:32:07 openstackgerrit Lee Yarwood proposed openstack/nova: libvirt: Remove redundant bdm serial mangling and saving during swap_volume https://review.openstack.org/427364
16:35:23 kashyap Hi folks, has anyone seen this error with today's Git?
16:35:29 kashyap 017-02-01 15:55:46.453 | No hosts found to map to cell, exiting.
16:35:33 mriedem when conductor cleans up the build request, if it's already gone and it deletes the instance, why doesn't it also clean up the instance mapping?
16:35:38 mriedem dansmith: ^?
16:35:56 kashyap My current DevStack instance is failing with the above. /me looks at the logs
16:36:08 mriedem kashyap: you need to have the placement-api enabled
16:36:19 mriedem kashyap: http://lists.openstack.org/pipermail/openstack-dev/2017-January/111295.html
16:36:21 kashyap Ah!
16:36:23 kashyap That's it
16:37:01 bauzas mriedem: kashyap: not sure this error is related
16:37:11 bauzas but /me shrugs
16:37:18 kashyap bauzas: But mriedem is right that I don't have the placement API
16:37:23 bauzas sure
16:37:28 mriedem there won't be a host if n-cpu fails to start
16:37:39 bauzas ah true
16:37:40 mriedem which kills map_cell_and_hosts
16:37:41 mriedem or whatever
16:37:45 bauzas okat

Earlier   Later