| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-14 | |||
| 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 | mriedem | the resource tracker doing resource tracking still ... :( | |
| 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: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 | 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:19 | sean-k-mooney | put this in memory in the driver is worring | |
| 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 | dansmith | right, so we have the mapping between instances and pmem devices stored in the libvirt xml | |
| 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: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 | 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:25 | tssurya | left* | |
| 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 | |
| 15:49:23 | dansmith | when Ioriginally suggested this, I was imaging the driver wholly owning the "what do we do" | |
| 15:49:51 | tssurya | dansmith: yea I remember you telling it in the spec design phase | |
| 15:49:52 | dansmith | so only ironic will be anything other than a no-op in this case, and all it needs to do is do its poweroff | |
| 15:50:00 | dansmith | tssurya: the lock only works within a compute node, | |
| 15:50:04 | tssurya | its just that the notifications/action stuff happens in the upper level | |
| 15:50:25 | dansmith | tssurya: so you can still race with the compute node processing this and something else trying to take action on the instance | |
| 15:50:37 | mriedem | something needs to handle UnexpectedTaskStateError for power-update otherwise we fail to process all of the events in the same request on the same host | |
| 15:50:38 | dansmith | tssurya: meaning ironic may have sent that event, and meanwhile some user tried to reboot the instance at the same time | |
| 15:50:38 | tssurya | dansmith: oh yea true | |
| 15:51:08 | mriedem | and you could lose a race with the bug you're trying to fix with this, i think, right? | |
| 15:51:09 | tssurya | so what's the point of moving it to the manager again ? | |
| 15:51:12 | dansmith | mriedem: all we need to do is handle, in the ironic case, what happens if I do instance.save(expected_Task_state=None) right? | |
| 15:51:19 | mriedem | tssurya: rolling upgrades for one | |
| 15:51:34 | tssurya | ah yes | |
| 15:51:36 | dansmith | tssurya: and it makes it so we don't touch the instance at all for any drivers that don't care about this | |
| 15:52:10 | tssurya | mriedem: and yea the downside point is valid | |
| 15:52:13 | mriedem | dansmith: i think so, but if the driver raises the nthe compute manager code has to handle it and not barf for the other events in the same request | |
| 15:52:53 | dansmith | mriedem: I think the driver should just not raise, I think the driver should handle all of this, because it's only one | |
| 15:53:29 | mriedem | so if the driver gets UnexpectedTaskStateError, it just logs and returns? | |