| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-08-24 | |||
| 14:06:06 | melwitt | . | |
| 14:06:35 | mriedem | mdbooth: btw yes i'm also interested in simply have regression testing for evac in a real env | |
| 14:06:40 | mriedem | *having | |
| 14:07:28 | mriedem | melwitt: that last rocky change is in the gate | |
| 14:09:51 | melwitt | mriedem: I saw, thank you for getting that. the master change didn't land until late last night after I went to sleep | |
| 14:13:59 | mriedem | mdbooth: btw do you still care about this? https://review.openstack.org/#/c/572790/ | |
| 14:38:15 | mriedem | dansmith: a couple of thoughts on batch size floor in https://review.openstack.org/#/c/593131/ | |
| 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. | |