| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-14 | |||
| 15:20:29 | mriedem | will flavour work? | |
| 15:20:31 | dansmith | no | |
| 15:21:13 | dansmith | I think building more things using the existing pattern of using migration uuid to reserve the old resources is a good idea | |
| 15:21:28 | alex_xu | emm...I try to remember which case we said no for this in the beginning | |
| 15:21:33 | alex_xu | luyao: ^ help me | |
| 15:22:44 | luyao | alex_xu: I'm always trying... | |
| 15:25:39 | tssurya | mriedem: do you also prefer to push all the instance state checking and updates into the manager like dansmith ? since we have a lock there | |
| 15:26:01 | alex_xu | mriedem: dansmith the first rpc call to the comptute node for the resize is send to the dest src. so you can do map 'migration = old flavor' first | |
| 15:26:54 | dansmith | alex_xu: you can't, but you don't have to do the migration mapping until you hit the source for the first time | |
| 15:27:08 | dansmith | alex_xu: isn't this the exact same set of steps for allocations in placement? so the ordering should work the same way | |
| 15:27:53 | openstackgerrit | YAMAMOTO Takashi proposed openstack/nova master: Revert "Revert resize: wait for events according to hybrid plug" https://review.opendev.org/675021 | |
| 15:27:54 | openstackgerrit | YAMAMOTO Takashi proposed openstack/nova master: Revert "Pass migration to finish_revert_migration()" https://review.opendev.org/676442 | |
| 15:28:10 | mriedem | well, it's a little different with placement allocations since we can manage those at the top in conductor | |
| 15:28:15 | alex_xu | dansmith: we switch instance_uuid to migration_uuid in the conductor, right? | |
| 15:28:47 | dansmith | okay, that's fair | |
| 15:29:06 | mriedem | for example, if we claim the new devices on the dest in prep_resize, cast to resize_instance on the source, swap the old devices to the migration, and then something fails during the disk transfer or whatever, we won't go back to the dest to cleanup that old claim - but the RT should fix that up in a periodic | |
| 15:29:46 | mriedem | i don't know where these "claims" are stored in memory though - in the compute manager? RT? driver? | |
| 15:30:09 | mriedem | i'll just say, the RT logic is already super complex, and now it sounds like we're going to be duplicating parts of it elsewhere... | |
| 15:30:11 | alex_xu | mriedem: swap should be happened first for same host resize | |
| 15:30:32 | alex_xu | mriedem: claim store in driver | |
| 15:30:54 | mriedem | cleaning up from a failed same host resize is simpler since yo'ure on the same host, but not for different hosts | |
| 15:31:18 | sean-k-mooney | alex_xu: this is diferent then how we claim pci devices and cpu/hugepage right? | |
| 15:31:53 | mriedem | the driver doing resource tracking now.... :( | |
| 15:31:56 | sean-k-mooney | because for those i thought we stored the claims in the db via the RT rather then in memeroy | |
| 15:32:02 | alex_xu | sean-k-mooney: yes, at least the vpmem and vgpu is managed by virt driver. pci and cpu managed by resource tracker | |
| 15:32:02 | mriedem | the resource tracker doing resource tracking still ... :( | |
| 15:32:19 | mriedem | what a mess | |
| 15:32:31 | mriedem | some things in placement, some things in legacy nova tables and the RT, some things now in the driver | |
| 15:32:58 | sean-k-mooney | we should really be keepign all this in the RT untill its in placment | |
| 15:33:04 | sean-k-mooney | if its ever in placment | |
| 15:33:19 | sean-k-mooney | put this in memory in the driver is worring | |
| 15:33:19 | alex_xu | sean-k-mooney: mriedem in the future, when pci and numa move to placement, then we needn't store in RT, right? then also managing in the virt driver? | |
| 15:33:24 | sean-k-mooney | or at least complex | |
| 15:33:41 | sean-k-mooney | alex_xu: it will still be needed in some cases | |
| 15:33:42 | alex_xu | sean-k-mooney: hah, you say different with dansmith :) | |
| 15:34:03 | sean-k-mooney | we placement wont be tracking indeivusal device assignment | |
| 15:34:18 | sean-k-mooney | e.g which vf(pci addres) the vm is using | |
| 15:34:38 | sean-k-mooney | to do that we would need to create a RP per vf which we are not going to do | |
| 15:34:42 | alex_xu | sean-k-mooney: yea, that is what happen for vgpu and vpmem | |
| 15:35:15 | dansmith | I'm not sure what sean-k-mooney said that is different than me | |
| 15:35:24 | sean-k-mooney | so the "assignment" infomation will always need to live in nova | |
| 15:35:47 | sean-k-mooney | the tally count of how many are avaliable wil be in placment | |
| 15:36:02 | dansmith | sean-k-mooney: when we previously discussed this, I wanted to avoid nova storing a mapping between the actual pmem device and the instance in our database, for a specific reason | |
| 15:36:08 | alex_xu | I think dansmith said we use libvirt to persistent the assigment of devices, not DB. sean-k-mooney is talk about we still need the DB | |
| 15:37:04 | sean-k-mooney | well currently we regenerate teh xmls on lots of operations so storign the mapping in the xml will be invasive | |
| 15:37:04 | dansmith | right, so we have the mapping between instances and pmem devices stored in the libvirt xml | |
| 15:37:28 | dansmith | whatever, I give up, do whatever ya | |
| 15:37:31 | dansmith | 'll want | |
| 15:37:35 | sean-k-mooney | it will be there implictly i guess | |
| 15:37:55 | alex_xu | dansmith: no... | |
| 15:38:06 | sean-k-mooney | dansmith: we dont need to store it in the db provide we will never use it in a filter | |
| 15:38:26 | sean-k-mooney | we only need to store the pci info in the db to use it with the numa/pci passthough filters | |
| 15:38:41 | sean-k-mooney | same for the numa toplogy blob | |
| 15:39:05 | sean-k-mooney | we could caluate them locally on the host and keep it in meory otherwise | |
| 15:39:40 | mriedem | what happens when i need a weigher to pick hosts with more or less allocated pmems?! | |
| 15:39:47 | sean-k-mooney | so if the only schduling for vpmem is done via placmeent then the assignment could be track via the xml | |
| 15:40:13 | mriedem | or pmem affinity | |
| 15:40:18 | sean-k-mooney | mriedem: we would either need to call placement for the data or we cant | |
| 15:40:55 | mriedem | btw, are there a fair number of rhosp users using vgpus now that you're on queens? | |
| 15:41:02 | sean-k-mooney | pmem affinity(i assume numa affinity) could be modeled in the RP tree | |
| 15:41:19 | sean-k-mooney | mriedem: not that im aware of | |
| 15:41:41 | sean-k-mooney | most are using full GPU passthough when they need gpus | |
| 15:41:46 | mriedem | was just going to say that | |
| 15:41:58 | sean-k-mooney | nvida licening is $$$$ | |
| 15:42:00 | dansmith | I'll be really honest here, I think this is a very niche, very libvirt-specific, very unlikely-to-be-widely-used feature, and I think that adding a bunch of nova persistence for these things brings more impact to operations and upgrades than we need, | |
| 15:42:24 | dansmith | and storing this information purely in the place where it matters (in libvirt) limits that impact and scope a lot | |
| 15:43:10 | sean-k-mooney | dansmith: im not against that just wanted to point out we have always assumed the xmls are not required until now | |
| 15:43:11 | dansmith | if an operator changes hardware after a maint cycle that changes the ordering of these devices or something, I worry about handing persistent devices to the wrong instances, and I think keeping the mapping(s) in one place that is visible and accessible to the operators if they need to remap is also a good idea | |
| 15:43:20 | sean-k-mooney | e.g. that we can jsut regenerate them | |
| 15:43:29 | dansmith | sean-k-mooney: no, that's not true I don't think | |
| 15:43:43 | dansmith | sean-k-mooney: if we delete the instance from a guest and we restart nova I think it will freak | |
| 15:44:10 | sean-k-mooney | yes but if an operator change the xml with virsh and we do a hard rebot we jsut regenerate it | |
| 15:44:11 | dansmith | sean-k-mooney: regenerating the xml all the time does not mean that the xml is not useful data.. we use it to determine which instances are actually on this host, vs just assigned | |
| 15:44:22 | sean-k-mooney | if the domain is missing i dont knwo what happens | |
| 15:44:25 | dansmith | sean-k-mooney: not if we don't store that detail ourselves | |
| 15:44:48 | dansmith | anyway, I think I've already spent way more time on this than this feature is worth, | |
| 15:45:11 | dansmith | and the column in the db to just dump a blob of data into instance_extra was already merged before this was all discussed, | |
| 15:45:38 | dansmith | so the easiest thing is to just let that become a dumping ground for all this stuff, regardless | |
| 15:46:11 | dansmith | alex_xu: really sorry for ever even involving myself in this, my apologies | |
| 15:46:37 | mriedem | onto tssurya's problem! | |
| 15:46:47 | tssurya | yayy | |
| 15:46:48 | dansmith | I was just goign to say | |
| 15:46:55 | dansmith | tssurya: I missed if there was a reply on the plan | |
| 15:47:10 | tssurya | not yet waiting for mriedem's opinion | |
| 15:47:13 | mriedem | she's just asking if i agree with changing task_state in hte api | |
| 15:47:15 | dansmith | yeah | |
| 15:47:20 | dansmith | didn't see a response on that | |
| 15:47:22 | tssurya | let a comment on the patch: https://review.opendev.org/#/c/645611/ | |
| 15:47:25 | tssurya | left* | |
| 15:47:25 | alex_xu | dansmith: sorry, I'm trying my best make it simple and easy. but yea, i still found those issue need help | |
| 15:47:54 | tssurya | also efried ^ in case you have an opinion | |
| 15:47:59 | mriedem | so if on power-update we don't check or set task_state in the api, we avoid the "instance is stuck witk with non-none task_state b/c it's on a stein compute" issue | |
| 15:48:18 | tssurya | right only to do the same in the manager | |
| 15:48:28 | mriedem | and thhe driver / compute manager would need to handle the UnexpectedTaskStateError | |
| 15:48:36 | dansmith | yep, and it moves the "do we do anything about this" closer to the thing that makes that decision | |
| 15:49:01 | tssurya | why does the driver have to handle UnexpectedTaskStateError ? | |
| 15:49:09 | dansmith | tssurya: it doesn't I don't think | |
| 15:49:13 | tssurya | I would be moving the task_state saving part into the manager | |
| 15:49:14 | mriedem | the downside is losing some race between the api and the compute manager where the sync power states task has turned off your bm instance b/c the nova db said it should be off but it's actually on again in ironic, right? | |
| 15:49:20 | tssurya | and since it has a lock it should be fine | |