| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-14 | |||
| 15:52:38 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix deps for api-samples tox env https://review.openstack.org/633620 | |
| 15:52:39 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add microversion to expose virtual device tags https://review.openstack.org/631948 | |
| 15:56:08 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove duplicate cleanup in functional tests https://review.openstack.org/636996 | |
| 16:11:21 | bauzas | efried: thanks for commenting https://review.openstack.org/#/c/636591/1/nova/virt/libvirt/driver.py | |
| 16:11:49 | efried | fo sho, look forward to seeing that patch. | |
| 16:11:52 | stephenfin | mriedem: Said it to lyarwood but I've backported that PYTHONDONTWRITEBYTECODE setting all the way back to queens. If you could take a look, I'd appreciate it https://review.openstack.org/#/q/topic:PYTHONDONTWRITEBYTECODE+status:open | |
| 16:12:21 | bauzas | efried: given we update the provider tree at compute startup, I just wonder if it's safe enough to assume that the libvirt in-memory cached attribute for the tree would be there anyway when an instance is spawned ? | |
| 16:12:29 | bauzas | mriedem: thoughts on that too ? | |
| 16:12:49 | bauzas | mriedem: context is https://review.openstack.org/#/c/636591/1/nova/virt/libvirt/driver.py@6347 | |
| 16:13:57 | efried | bauzas: We're pretty much *relying* on there being a match between upt and spawn | |
| 16:14:26 | bauzas | efried: there could be a race tho | |
| 16:14:28 | efried | bauzas: Also, pretty sure we run upt right before spawn, not just in the periodic, I could be wrong. So it should be pretty durn recent by then. | |
| 16:14:46 | bauzas | efried: where spawn() could run *before* upt | |
| 16:14:48 | bauzas | yeah | |
| 16:14:56 | efried | bauzas: But like I've said already (uh, possibly about something else): if there's a race, it's going to f up more than just this. | |
| 16:15:06 | mriedem | we run update_available_resource on compute start | |
| 16:15:08 | mriedem | which calls upt | |
| 16:15:11 | bauzas | i know | |
| 16:15:13 | mriedem | that's before the service is ready for spawn | |
| 16:15:15 | mriedem | so what's the issue | |
| 16:15:18 | bauzas | okay | |
| 16:15:33 | bauzas | I thought we were allowing instances to spawn, while compute was starting | |
| 16:15:41 | mriedem | f i hope not | |
| 16:15:46 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1259 | |
| 16:15:57 | mriedem | https://github.com/openstack/nova/blob/master/nova/service.py#L180 | |
| 16:16:08 | bauzas | oh good catch this is pre hook | |
| 16:16:12 | bauzas | perfect | |
| 16:16:35 | bauzas | mriedem: you're not worried by libvirt keeping a copy of the provider tree structure ? | |
| 16:16:47 | mriedem | i haven't been following along | |
| 16:17:07 | bauzas | mriedem: nah, it's just because we need to find the name of a RP by its uuid | |
| 16:17:08 | mriedem | sounds like more cache-tastic fun | |
| 16:17:24 | bauzas | mriedem: as we only get RP UUID from the allocations list we pass over spawn | |
| 16:17:28 | mriedem | which RP? | |
| 16:17:30 | efried | mriedem: TL;DR: the virt driver needs to be responsible for maintaining a mapping between RP and "real thing". The provider tree already has that information, so don't bother building something special, just save it off during upt. | |
| 16:17:47 | bauzas | mriedem: the RP which was allocated a VGPU | |
| 16:17:55 | mriedem | so the pgpu? | |
| 16:17:58 | mriedem | the child provider | |
| 16:18:01 | bauzas | yup | |
| 16:18:15 | bauzas | so that libvirt can use or create a mdev from *this* pgpu | |
| 16:18:25 | bauzas | http://paste.openstack.org/show/745109/ | |
| 16:18:44 | mriedem | what's the alternative? the driver holding it's own mapping of rp_uuid -> rp_name? which is something ProviderTree already does | |
| 16:18:49 | efried | yes ^ | |
| 16:18:56 | efried | (to both) | |
| 16:18:56 | bauzas | above is the fact that when we create a new instance, we use a random mdev which is unrelated to the pgpu which was allocated by placement | |
| 16:19:25 | mriedem | i don't have a strong feeling either way | |
| 16:19:46 | mriedem | ProviderTree is convenient albeit heavy weight and hopefully someone doesn't abuse it somehow, but doing our own mapping dict is redundant | |
| 16:19:49 | mriedem | so shrug | |
| 16:20:07 | bauzas | yeah me too, but I'm opinionated enough to make it the smoothiest | |
| 16:20:30 | bauzas | and caching the value in the libvirt driver object seems the smoothiest approach | |
| 16:20:45 | mriedem | so the provider tree would just get reset on every upt call? | |
| 16:20:47 | mriedem | if so, then sure | |
| 16:20:51 | bauzas | yeah | |
| 16:20:54 | mriedem | wfm | |
| 16:21:02 | bauzas | i'm done with complex cache invalidation mechanisms | |
| 16:21:13 | mriedem | are you SO done with them? | |
| 16:25:21 | bauzas | mriedem: /me tries to not think hard of AZs | |
| 16:25:34 | bauzas | or server groups | |
| 16:25:47 | mriedem | hey if you like azs i have some good news for you | |
| 16:25:48 | mriedem | https://review.openstack.org/#/c/509206/ | |
| 16:25:51 | mriedem | finally fixing that shitty old bug | |
| 16:26:35 | bauzas | \o/ | |
| 16:26:46 | bauzas | mriedem: straight adding it into my queue | |
| 16:27:39 | bauzas | gibi: you'll be happy to hear that functests work now, btw. :) | |
| 16:28:07 | gibi | bauzas: awesome | |
| 16:29:24 | gibi | every time I start kicking https://review.openstack.org/#/c/635859/ I got depressed after 15 minutes about I capabilities to understand what happening | |
| 16:29:38 | melwitt | o/ | |
| 16:30:14 | melwitt | gibi: me too. I tried for awhile and gave up | |
| 16:31:31 | gibi | melwitt: but I feel something deep is behind that so I cannot gave up :) | |
| 16:32:09 | melwitt | gibi: that's good, I think you will get to the bottom of it :) | |
| 16:32:20 | mriedem | well if you want to get really depressed http://logs.openstack.org/47/635147/11/check/nova-tox-functional-py35/5c3c2a9/job-output.txt.gz#_2019-02-13_13_33_41_103932 | |
| 16:32:34 | mriedem | that traceback wins the grand prize | |
| 16:32:41 | mriedem | and i don't know what to do about it | |
| 16:32:48 | mriedem | looks like the db dropped in the middle of the test | |
| 16:33:32 | gibi | mriedem: nice :) | |
| 16:33:56 | mriedem | the dumping of os-traits contents to the logs on each functional test run with the PlacementFixture isn't helping either | |
| 16:34:08 | mriedem | http://logs.openstack.org/47/635147/11/check/nova-tox-functional-py35/5c3c2a9/job-output.txt.gz#_2019-02-13_13_33_41_193979 | |
| 16:34:24 | gibi | mriedem: I think we can move that to DEBUG | |
| 16:35:18 | mriedem | yeah probably http://logs.openstack.org/59/636459/2/check/tempest-full/3226eb1/controller/logs/screen-placement-api.txt.gz#_Feb_13_13_54_30_464250 | |
| 16:35:23 | gibi | I'm affraid that the drop of the databease could be another interfering test case. Did you managed to reproduce that locally? | |
| 16:35:31 | mriedem | nope | |
| 16:35:35 | bauzas | gibi: efried: what's the best way to copy a providertree object ? copy.deepcopy() ? | |
| 16:35:54 | openstackgerrit | Elod Illes proposed openstack/nova master: DNM: test pip 19 - pip freeze with git editable https://review.openstack.org/636980 | |
| 16:35:56 | bauzas | or do we have other built-in mechanism ? | |
| 16:36:18 | bauzas | tbc, I want to have a separate reference to be stored on the libvirt object | |
| 16:36:54 | gibi | bauzas: I would be affraid of copying the self.lock = lockutils.internal_lock(_LOCK_NAME) inside the ProviderTree | |
| 16:37:25 | efried | bauzas: upt is already operating on a copy... but I'm actually not positive that ufpt doesn't dork with it. So yeah, copy.deepcopy is probably the best bet. | |
| 16:37:58 | gibi | bauzas: besides the self.lock the rest of the internals seems copiable | |
| 16:38:18 | bauzas | gibi: so, ideally we should provide a copy method | |
| 16:38:19 | efried | gibi: hm, interesting point about the lock. This impl that bauzas is doing should be read-only. But that still wouldn't stop you from bouncing off the lock if the other instance is using it. | |
| 16:38:38 | bauzas | efried: what I want is purely read-only | |
| 16:38:45 | efried | However, I would be surprised if the lockutils lock doesn't override __copy__ to *not* copy itself. | |
| 16:38:49 | bauzas | actually, you know what ? I'm gonna make it explicit | |
| 16:38:55 | bauzas | by using a propertgy | |
| 16:39:11 | bauzas | we're not intended to manipulate this copy | |
| 16:39:12 | gibi | mriedem: I think I will stick to the remove-the-assert-that-blow-the-world issue, that I can reproduce locally at least | |
| 16:39:59 | efried | bauzas: There are methods to get a read-only copy of the data out of ProviderTree, but I'm pretty sure they only get one provider at a time. | |
| 16:40:22 | efried | Yeah, ProviderTree.data() | |
| 16:40:27 | mriedem | gibi: efried: fwiw i'm also holding off on the bw provider series until https://review.openstack.org/#/c/616239/ makes progress | |
| 16:40:40 | mriedem | and we've got 3 weeks from FF today | |
| 16:40:52 | efried | I guess that's my action | |