| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-20 | |||
| 21:04:12 | efried | guess it really just makes sense to make it part of snapshot and have a crisp line | |
| 21:13:47 | mriedem | efried: shelve works for volume-backed and non-volume-backed | |
| 21:14:23 | efried | mriedem: In the compute API I see that volume-backed calls the shelve-offload code path. | |
| 21:14:29 | mriedem | if you shelve but don't offload (which offloading is automatic by default config) then you unshelve within the offload window you do end up on the same host (conductor just starts the server) | |
| 21:15:29 | efried | https://opendev.org/openstack/nova/src/branch/master/nova/compute/api.py#L3867-L3873 ? | |
| 21:15:36 | mriedem | yeah so in the case of volume-backed shelve there is no snapshot image | |
| 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 | |