Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-08-24
14:41:48 dansmith mriedem: thanks will look shortly
14:57:22 mriedem and i'm done with that stack \o/
15:02:35 mdbooth mriedem: I do. Came up yesterday, in fact.
15:02:54 mdbooth mriedem: I don't think I had any strong objections there?
15:03:47 mriedem i'm pretty sure you wanted to +1 it
15:04:28 mdbooth Just re-reading my last review. Sounds like I was prevaricating rather than objecting.
15:06:12 mdbooth Ah... the attach_mode thing.
15:13:46 mdbooth mriedem: So there's attach_mode and access_mode
15:14:23 mdbooth It seems like we only *use* access_mode, but that code is checking attach_mode
15:23:34 openstackgerrit Claudiu Belu proposed openstack/nova master: hyper-v: autospec classes before they are instantiated https://review.openstack.org/342211
15:23:42 openstackgerrit Claudiu Belu proposed openstack/nova master: WIP: replace spec with autospec https://review.openstack.org/557299
15:29:11 mdbooth mriedem: Who's a friendly cinder person?
15:31:34 mriedem for attachment record stuff it would be jgriffith
15:33:54 mdbooth I've hopefully added the right one.
15:34:24 giblet_off I will be back on Tuesday
15:38:56 mdbooth dansmith: Is your no timeouts messaging thing in Queens, btw? Or backportable to Queens?
15:46:23 mriedem it is not in queens
15:46:27 mriedem and not really backportable
15:46:35 mriedem as that would require a min required version of oslo.messaging
15:46:39 mriedem and we don't bump mins on libs in stable
15:47:21 mdbooth mriedem: ack, thanks.
15:47:58 mdbooth mriedem: Means I can't promise a fix to a customer in our next stable release :/
15:48:16 sean-k-mooney mdbooth: without it could you just set the time out really high to get a similar effect?
15:48:25 mdbooth sean-k-mooney: Yeah.
15:49:53 mdbooth sean-k-mooney: I still need to look way harder at the impact, anyway.
15:50:09 mriedem mdbooth: you can fork whatever you want
15:50:35 mdbooth mriedem: Well I would, but dansmith won't let me.
15:51:06 mdbooth I was just going to rewrite in Go.
15:53:16 mdbooth In all seriousness, though: pretty sure you were joking, but in case not we don't do that. At least not in any incompatible way.
15:54:06 mdbooth If we can't do it, that's the way it is.
16:14:51 openstackgerrit Stephen Finucane proposed openstack/nova master: doc: Note NUMA topology requirements for numa-aware-vswitches https://review.openstack.org/596393
16:16:24 mdbooth melwitt: Do you remember anything about either adding or removing refresh_conn_info from hard reboot?
16:16:54 mdbooth It's vaguely ringing a bell, but I can't find any evidence of it.
16:17:13 melwitt mdbooth: no, I never understood much around that. lyarwood would be the one to ask
16:17:49 melwitt might be a WIP patch from him or something? not sure
16:18:23 openstackgerrit Merged openstack/nova stable/rocky: Correct the release notes related to nova-consoleauth https://review.openstack.org/595890
16:18:32 openstackgerrit Merged openstack/nova master: tests: Move mocking to setUp https://review.openstack.org/595802
16:20:50 mdbooth melwitt: I think the change I'm thinking of is when we added the call to destroy() which calls cleanup() and therefore does other stuff like detaching volumes. We don't seem to refresh_conn_info, though
16:21:21 mdbooth mriedem: Is refresh_conn_info still relevant in the new world order?
16:21:43 melwitt mdbooth: ah yeah, the "redo all the things" change
16:21:50 mdbooth melwitt: Yeah.
16:22:02 melwitt didn't have anything to do with refresh_conn_info tho (as you saw)
16:22:38 mdbooth I'm trying to come up with a recovery recommendation for this pre_live_migration timeout.
16:23:43 mdbooth The bug is: we called cleanup on the source when the dest timed out, even though it continued to run. The result is that the instance still has all volumes connected on the source where it continues to run, but some bdms have conn_info for source and others for dest.
16:24:05 mdbooth So really we want some operation which would cause a refresh_conn_info.
16:24:29 mdbooth Options seem to be cold and live migrations.
16:24:38 mdbooth Wonder if reboot ought to be another.
16:25:09 mriedem mdbooth: refresh_conn_info with the new attachment record flow isn't the same as how things worked with the old flow
16:25:50 smcginnis melwitt: Looks like that last stable/rocky patch merged, and you were right that the hash needs to be updated.
16:26:02 mdbooth mriedem: I'll take a look.
16:26:03 melwitt smcginnis: doing it right now, thanks
16:26:35 mriedem mdbooth: https://review.openstack.org/#/c/584648/20/nova/scheduler/client/report.py@2055 the difference is in the old flow, we'd always create a new connection to the storage backend which gives us a fresh connection_info from cinder,
16:26:48 mriedem with the new flow, we are getting the attachment record which has a stashed connection_info, which might be potentially stale
16:26:58 mriedem which is what i think you're worried about
16:27:28 mriedem btw this is related https://review.openstack.org/#/c/579004/
16:27:34 mdbooth mriedem: Ah, good point. So in that case attachment_id also be out of date.
16:28:07 mdbooth From that commit message: Note: Only hard reboots will currently update connection_info.
16:28:16 mdbooth I wonder if I missed something
16:28:47 mriedem that's just a result of the patch
16:29:02 mriedem but i think the comment is wrong
16:30:10 mdbooth mriedem: That's super-useful, thanks.
16:30:22 mriedem i am both very useful and very handsome
16:30:27 mriedem i think we can all agree
16:31:47 mdbooth mriedem: Your modesty is also unparalleled in all of history ;)
16:32:22 mriedem that's very kind of you
16:32:24 mriedem so,
16:32:28 mriedem without digging into the logs,
16:32:40 mriedem i expect this is blowing up b/c attachment_update isn't meant to be used this way once it's already got a connector
16:32:55 mriedem likely should be creating a new attachment record with new connector (and id) and deleting the old one
16:38:29 mriedem commented and pinged jgriffith
16:38:39 mdbooth Oh, the irony!
16:39:09 mdbooth So, if connection_info is wrong that's obviously going to break any attempt to attach
16:39:32 mdbooth But in this particular case because the volumes are already attached they're already working
16:40:04 mdbooth But since the robustification patch added detach/attach in hard reboot, we actually broke it
16:40:21 mdbooth It needs to be combined with refresh_conn_info
16:40:59 openstackgerrit Dan Smith proposed openstack/nova master: Batch results per cell when doing cross-cell listing https://review.openstack.org/592698
16:41:00 openstackgerrit Dan Smith proposed openstack/nova master: List instances from all cells explicitly https://review.openstack.org/593717
16:41:01 openstackgerrit Dan Smith proposed openstack/nova master: Make instance_list perform per-cell batching https://review.openstack.org/593131
16:41:02 openstackgerrit Dan Smith proposed openstack/nova master: Record cell success/failure/timeout in CrossCellLister https://review.openstack.org/594265
16:41:03 openstackgerrit Dan Smith proposed openstack/nova master: Optimize global marker re-lookup in multi_cell_list https://review.openstack.org/594577
16:41:28 mriedem umm,
16:41:31 mriedem well,
16:42:00 mriedem we've talked about this hard refresh thing for a long time b/c of the ceph ip or auth changing and not wanting to live migrate your entire cloud to update the attachments
16:44:35 mriedem i think coreycb is trying to fix this one case for reboot, and then see if we can extend to other operations
16:44:54 mriedem "This only includes a fix for the 'openstack server reboot --hard' path as I didn't want to get too deep before knowing if this is a proper fix. A complete fix would also provide soft reboot, stop/start support, perhaps more?"
16:46:02 coreycb mriedem: thanks for your comments. i dropped that patch for a bit but will get back to it once we get rocky done.
16:47:05 mdbooth mriedem: I'm coming from a different angle. A weird unfixable bug means I've now got inconsistent state, and I want a thing to make everything right again.
16:48:11 mriedem mdbooth: is that related to your other live migration patch?
16:48:27 mriedem https://review.openstack.org/#/c/551302/
16:49:01 mriedem bw, you probably need some more spinning plates
16:50:43 mdbooth mriedem: No.
16:51:02 openstackgerrit Dan Smith proposed openstack/nova master: Optimize global marker re-lookup in multi_cell_list https://review.openstack.org/594577
16:51:07 mdbooth mriedem: I thought about that one, but I don't think it would fix this.
16:51:59 mdbooth I should resurrect that patch as being just a generally good idea, even if the original motivation for it has gone. It would have fixed a couple of other things since iirc.
16:53:54 stephenfin mriedem: I'm not sure what I can do about this https://review.openstack.org/#/c/588570/
16:54:54 stephenfin mriedem: Have tried drafting a functional test but it looks like it's going to require a good deal of changes to the fake libvirt driver, assuming it's even possible to validate
16:55:01 mriedem mdbooth: fwiw i think i was ok with that change but wanted to see the same rollback stuff done in a follow up first
16:56:50 mriedem stephenfin: ok...i'm about out of here for lunch and errands, so probably need to hit me next week when i'm in more of a "f it" kind of mood
16:57:19 stephenfin mriedem: Heh, fair
16:57:22 mriedem because this definitely seems like new feature capability

Earlier   Later