| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-20 | |||
| 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 | |
| 21:47:30 | sean-k-mooney | efried: i think the vtpm lifetime shoudl be tied to the instance not the image | |
| 21:47:33 | dansmith | TPM is hardware, therefore it stays during a rebuild, IMHO | |
| 21:47:38 | dansmith | sean-k-mooney: ++ | |
| 21:47:56 | efried | So what if your instance has a vTPM, and then you rebuild with an image that has pointer-to-vtpm-in-swift metadata? | |
| 21:48:09 | efried | "they better match, or we punt"? | |
| 21:48:20 | dansmith | efried: you mean if you rebuild from a snapshot? | |
| 21:48:29 | dansmith | remember the pointer to swift stays with the instance not the snapshot | |
| 21:48:43 | dansmith | oh you mean for the backup case.. that's why it doesn't work for the backup case :) | |
| 21:48:45 | efried | no, you convinced me earlier that snapshot and restore should work. | |
| 21:48:58 | dansmith | no, I didn't, I said "you're effed on one of these two things depending on what you decide :) | |
| 21:49:15 | sean-k-mooney | yes and the point would be stored in the nova system meatadat not the snapshot metadata | |
| 21:49:32 | efried | but that doesn't work for backup/restore | |
| 21:49:35 | sean-k-mooney | *pointer to the tpm snapshot | |
| 21:49:38 | efried | because there's no instance meta | |
| 21:49:46 | efried | there is only image meta. | |
| 21:50:10 | sean-k-mooney | oh right we were talking about shelve and unshelve before | |
| 21:50:32 | efried | if we go on the philosophy that TPM is like hardware, it's not unreasonable that backup/restore would lose it. | |
| 21:50:45 | sean-k-mooney | well for vpmem and ephermeral disk we dont snapshot them currenlty | |
| 21:51:01 | efried | if I do a backup of my laptop and then restore that backup on a different shell, my hardware changed. | |
| 21:51:36 | sean-k-mooney | and if you reinstall the os on the same laptop the tpm data is preserved | |
| 21:51:42 | dansmith | efried: but that's why VMs are better than hardware | |
| 21:51:45 | efried | heh | |
| 21:52:22 | efried | Okay, does this sound reasonable: | |
| 21:53:04 | efried | On rebuild, if your instance had a vTPM before, and the image specifies a vTPM also, they must be the same vTPM, or we fail. | |
| 21:53:37 | sean-k-mooney | not really | |
| 21:53:45 | efried | what should we do in that case? | |
| 21:54:02 | sean-k-mooney | im not sure the image shoudl be able to reference a vtpm | |
| 21:54:11 | efried | then we can't do backup/restore. | |
| 21:54:19 | sean-k-mooney | of the tpm | |