| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-20 | |||
| 21:16:02 | efried | right, but the distinction for 'offload' (at least the one that I care about) is that it calls destroy() | |
| 21:16:06 | efried | whereas non-offload doesn't. | |
| 21:16:16 | efried | uh, unless I got confused. | |
| 21:16:34 | efried | yeah | |
| 21:16:47 | mriedem | yeah shelve without offload powers off the guest and creates a snapshot | |
| 21:17:33 | efried | ack | |
| 21:18:17 | mriedem | i'm drawing a blank on why we shelve offload immediately for volume-backed servers | |
| 21:19:19 | mriedem | just ask one of these guys from 2013 https://review.opendev.org/#/c/34032/ | |
| 21:19:46 | efried | was gonna say, seems like it's been that way from the get-go. dansmith co-authored, so... | |
| 21:19:51 | efried | he'll surely know. | |
| 21:20:31 | mriedem | i guess this comment https://review.opendev.org/#/c/34032/21/nova/compute/manager.py@3099 | |
| 21:20:44 | dansmith | mriedem: because there's no reason not to offload immediately | |
| 21:20:48 | efried | probably because waaay back then, the distinction with offload was only whether snapshot was necessary, which it isn't for volume (right?) | |
| 21:20:50 | dansmith | mriedem: there's no point in maintaining affinity | |
| 21:21:18 | dansmith | efried: not just no snapshot required, but there's no benefit to unshelve to the same host before offload | |
| 21:21:32 | dansmith | once offloaded, it's might as well go anywhere, but before offload, it's much quicker if you stay put | |
| 21:21:45 | dansmith | but with volume-backed, that distinction does not exist | |
| 21:21:49 | efried | got it. | |
| 21:22:02 | dansmith | efried: also, did you say I co-authored something? | |
| 21:22:08 | mriedem | i didn't realize the offload periodic came later https://review.opendev.org/#/c/35361/ | |
| 21:22:25 | dansmith | oh that shelve patch | |
| 21:22:32 | dansmith | I dunno what I co-authored about it | |
| 21:22:36 | dansmith | not much | |
| 21:22:37 | mriedem | not that one | |
| 21:22:41 | mriedem | https://review.opendev.org/#/c/34032/ | |
| 21:22:41 | mriedem | this | |
| 21:23:09 | dansmith | right | |
| 21:23:12 | efried | co-authored *and* +2ed, tsk | |
| 21:23:22 | efried | those were rowdy times | |
| 21:23:24 | eandersson | Anyone got any experience with the openstacksdk interacting with nova. Trying to figure out how to fix a memory leak in Senlin. | |
| 21:23:32 | dansmith | efried: never uploaded a patch set, | |
| 21:23:46 | efried | eandersson: you mean nova talking through sdk, or using sdk to talk to nova? | |
| 21:23:47 | dansmith | efried: so that was probably honorary because I helped figure something out | |
| 21:23:54 | eandersson | using the sdk to talk to nova | |
| 21:24:51 | efried | eandersson: mmph. Not so hot there. Given you already met crickets in -sdks, mriedem and I might between us be able to figure things out... | |
| 21:25:28 | eandersson | Senlin seems to be creating a new client for everytime it makes a call to nova. Which isn't ideal. | |
| 21:25:51 | efried | and you think that's because of the way it's talking to sdk? | |
| 21:25:53 | eandersson | I wanted to make it a singleton, but not sure how to do that when the user/project might differ. | |
| 21:26:07 | eandersson | https://review.opendev.org/#/c/695139/4/senlin/profiles/base.py | |
| 21:26:18 | eandersson | If you look at like 489 there | |
| 21:26:22 | eandersson | *389 | |
| 21:26:52 | eandersson | So I know for sure the leak is caused by the openstacksdk | |
| 21:26:56 | mriedem | so create a singleton map per user/project hash? | |
| 21:27:12 | eandersson | Yea - was going to go that route, but seems crazy to me hehe | |
| 21:27:22 | mriedem | obviously it's just masking a leak | |
| 21:27:25 | eandersson | Isn't there a way I can just create one openstacksdk object and re-use it | |
| 21:27:28 | mriedem | but idk why the sdk would be leaking | |
| 21:27:39 | mriedem | i'm not the guy to ask about that... | |
| 21:27:42 | eandersson | It could be some reference that keeps it alive | |
| 21:28:32 | eandersson | I poked mordred so maybe he'll get back to me tomorrow | |
| 21:28:40 | efried | I am *sure* I remember mordred telling me sdk already does local singleton-ing | |
| 21:28:48 | efried | but that doesn't mean it isn't buggy. | |
| 21:29:58 | mriedem | dansmith: this is random, but i've noticed that during a resize_claim we make at least 3 lazy-load roundtrip calls to the db to load fields that in most cases probably aren't even populated http://paste.openstack.org/show/786457/ | |
| 21:30:13 | dansmith | sweet | |
| 21:30:15 | mriedem | was wondering what you thought about adding something to Instance.refresh() to specify additional fields to load up | |
| 21:30:27 | mriedem | refresh today just pulls the instance with it's existing additional joined fields, | |
| 21:30:29 | dansmith | lazy load pci_Devices twice? | |
| 21:30:35 | dansmith | resources is the new vpmems stuff right? | |
| 21:30:43 | mriedem | pci_requests, pci_devices and resources | |
| 21:30:52 | dansmith | ah right | |
| 21:31:13 | dansmith | but sure, fresh with a extra_attrs optional param makes sense | |
| 21:31:21 | mriedem | so was thinking something like Instance.refresh(..., extra_attrs=None) yeah | |
| 21:31:27 | mriedem | it's a version bump since refresh is remotable | |
| 21:31:29 | dansmith | so you can fault them all in in a single go if you know you're going to hit them | |
| 21:31:31 | mriedem | but that's probably not a big deal | |
| 21:31:34 | mriedem | right | |
| 21:31:43 | dansmith | mriedem: or if you don't need a refresh, you could just do a "load_if_not_present()" | |
| 21:31:49 | mriedem | and i want to use refresh here since the instance in the resize_claim is what's being used in the compute manager as well | |
| 21:33:34 | dansmith | then sure | |
| 21:35:37 | openstack | Launchpad bug 1853370 in OpenStack Compute (nova) "resize_claim lazy-loads at least 3 joined fields in separate DB calls" [Low,Confirmed] | |
| 21:35:37 | mriedem | https://bugs.launchpad.net/nova/+bug/1853370 if you want to hack on it | |
| 21:35:43 | mriedem | i just wanted to dump thoughts before moving on | |
| 21:37:46 | sean-k-mooney | artom: ya ill take a look ill be around for about half an hour so ill review that before i finish up today | |
| 21:42:31 | efried | sigh, I need a fresher on rebuild again. | |
| 21:42:49 | efried | this is where you give an instance a new image but keep its network etc. | |
| 21:43:06 | efried | does vTPM fall into "etc"? | |
| 21:43:12 | efried | I would kinda think not. | |
| 21:43:24 | efried | dansmith: ^ | |
| 21:43:49 | dansmith | efried: this? | |
| 21:43:51 | dansmith | shelve? | |
| 21:43:55 | dansmith | oh on rebuild | |
| 21:44:06 | dansmith | rebuild the instance cannot move, just rebuild its root from a new image | |
| 21:44:23 | efried | Yeah, so the question is whether we should keep the vTPM | |
| 21:44:39 | dansmith | except when it's evacuate | |
| 21:44:40 | dansmith | we should, I would think | |
| 21:44:44 | dansmith | rebuild is akin to putting the Windows 98 disc in the drive and rebooting | |
| 21:44:51 | sean-k-mooney | artom: so ya im happy with those one thing i notices is we shoudl update teh job templates to drop python 2 and 35 for the tempest plugin and just use the ussuri template instead | |
| 21:45:11 | sean-k-mooney | that can be in a follow up patch | |
| 21:45:15 | efried | I kinda want to say the vTPM should ride with the image | |
| 21:45:55 | efried | that would make for consistency with the backup/restore and shelve/unshelve type cases. | |
| 21:46:09 | sean-k-mooney | we do not nuke addtional ephemeral disks on rebuild right? | |
| 21:46:15 | sean-k-mooney | just the root disk | |
| 21:46:23 | dansmith | vtpm should survive a rebuild | |
| 21:46:25 | dansmith | for sure | |
| 21:46:31 | sean-k-mooney | you could look at the vtpm the same way | |
| 21:46:48 | efried | i.e. any time you create an instance from an image, we check the image for the metadata that points to the vtpm swift store. | |
| 21:47:07 | efried | dansmith: so what if your instance has a vTPM, and then you rebuild with an image that has ^ metadata? | |
| 21:47:09 | mriedem | efried: https://docs.openstack.org/nova/latest/contributor/evacuate-vs-rebuild.html ?!!??! :) | |
| 21:47:12 | sean-k-mooney | efried: i dont think that is right | |
| 21:47:26 | dansmith | efried: instance create is like creating a new server box, rebuild is like reinstalling the OS | |