Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2023-02-08
08:58:23 bauzas but I think the leaked thread comes from a RPC call
08:58:38 bauzas not from a libvirt thread
08:58:45 gibi I have some concerns of the pooling in general and I left comments there
08:58:57 sean-k-mooney ack
08:59:04 bauzas honestly, my concern is more about the time here
08:59:13 gibi bauzas, sean-k-mooney: I think the current pooling won't catch the RPC threads as that is created in oslo.messaging
08:59:37 bauzas I see three cores at least looking at this bug while we only have 7 days for merging features
08:59:39 sean-k-mooney gibi: yes it likely wont but those are not created directly by nova
09:00:05 sean-k-mooney gibi: with that said i might be able to make it do that
09:00:12 gibi I will keep rechecking bauzas'
09:00:22 gibi I will keep rechecking bauzas's patch to catch a failure to see where it is coming from
09:00:29 bauzas so, while I think it's important to have a better way to have green threads pooling, I'm just saying that we maybe should try to just find the issue and at least do other stuff
09:00:44 bauzas sean-k-mooney: see,that's a problem then
09:00:55 sean-k-mooney bauzas: no its not
09:01:12 bauzas sean-k-mooney: as I said, I'm pretty sure that the threads that are leaked and create this libvirt exceptioin are RPC calls
09:01:40 bauzas when I say a problem, I mean I'm not sure this change would help then
09:01:46 sean-k-mooney right but ye did not have a repoducer so i tried to create one and found a bunch of issue
09:01:51 bauzas https://4dca9d38a541907e85e1-0253beca39d73a6e7192d5b32ed5edc2.ssl.cf2.rackcdn.com/860282/2/check/nova-tox-functional-py310/466e0d7/testr_results.html
09:02:09 sean-k-mooney it may not fix the current issue but all the other tests if found may be flaky
09:02:17 gibi sean-k-mooney, bauzas: cool, we have two set of issues to solve then :)
09:02:19 bauzas agreed
09:02:26 bauzas and agreed with gibi
09:02:29 sean-k-mooney and if im wirte we are also constantly builiding up greenthreas as the test run
09:02:44 bauzas sean-k-mooneyI'm not saying "NO" to your change and thanks for having worked on it
09:02:44 sean-k-mooney yes
09:02:57 sean-k-mooney two sets of issues
09:03:12 bauzas sean-k-mooney:I'm just saying that I'm afraid this won't help the functest CI failure we have atm
09:04:02 bauzas anyway, I said loudly yesterday that I'll stop looking at the CI failures today and I'll rather review some changes
09:04:15 bauzas os-vif and os-traits first, and then nova features
09:04:34 sean-k-mooney there arnt any we need for os-vif in this release
09:04:35 bauzas and I'll continue to look at https://review.opendev.org/c/openstack/nova/+/872975 and recheck until we get a -1
09:04:42 sean-k-mooney not sure about os-traits
09:04:54 bauzas that's what I'll be doing
09:05:07 bauzas people are free to do anything
09:05:14 bauzas they prefer
09:05:35 bauzas I also need to look at the releases we have for os-vif, os-traits and os-rc
09:06:03 sean-k-mooney i approved the os-vif one yesterday
09:06:09 sean-k-mooney i did not look at the others
09:09:28 sean-k-mooney gibi: so we do stub out the events thread https://github.com/openstack/nova/blob/ea0526d959f7246c7d741ea24c207b52417d224a/nova/tests/fixtures/libvirt.py#L919-L941
09:09:35 sean-k-mooney but not the other one
09:10:31 sean-k-mooney https://github.com/openstack/nova/blob/ea0526d959f7246c7d741ea24c207b52417d224a/nova/virt/libvirt/host.py#L616-L619
09:11:06 sean-k-mooney i wonder if we can just more utils.spawn(self._conn_event_thread) into self._init_events()
09:12:23 gibi sean-k-mooney: I won't mix the events part with the connection thread. the events part uses a native thread
09:12:50 bauzas elodilles: 2023-02-07 14:02:46.446989 | compute1 | neutron-openvswitch-agent: no process found on https://review.opendev.org/c/openstack/nova/+/871702
09:13:04 bauzas elodilles: that's the second recheck having the same problem
09:13:05 gibi but sure we can wrap the connection thread spawning and mock it
09:13:17 bauzas elodilles: so, OK, when you say "broken broken", I understand it :)
09:13:36 sean-k-mooney well these are both related to event handeling
09:14:03 sean-k-mooney so it feell like it should be also in _init_events
09:14:16 gibi bauzas: how do you feel about https://review.opendev.org/c/openstack/nova/+/872975/2/nova/virt/libvirt/driver.py#10064 our current trials haven't hit the true positive case yet, but already hit the half false positives a lot
09:14:56 gibi sean-k-mooney: connection thread is where nova initiate calls to libvirt the event thread is where libvirt initiate call back to nova
09:15:49 sean-k-mooney ok but both of them are related to the libvirt events transmit vs recive
09:15:55 sean-k-mooney https://github.com/openstack/nova/blob/ea0526d959f7246c7d741ea24c207b52417d224a/nova/virt/libvirt/host.py#L480
09:16:16 sean-k-mooney and the doc string makes it sound like a good fit
09:16:30 sean-k-mooney it woudl jsut be adding the spawn to the end fo that function
09:16:42 sean-k-mooney so we spawn the dispatche and reciver threads form the same place
09:16:54 sean-k-mooney and the native thread
09:18:57 sean-k-mooney anyway ill be back in an hour or two i was woken up at around 6 and started looking at this. so im going to see if i ca rest for a bit although at this point i may have missed that window
09:20:36 gibi sean-k-mooney: ack
09:23:19 elodilles bauzas: yepp, it's broken broken this time :]
09:25:19 elodilles bauzas: but this should do the work when merged: https://review.opendev.org/c/openstack/grenade/+/872969
09:26:50 opendevreview Pierre Libeau proposed openstack/nova master: Add mechanism to manage snapshot during nc init https://review.opendev.org/c/openstack/nova/+/873062
09:41:25 zigo sean-k-mooney[m]: Would you know how to easily just forbid *ALL* .vmdk on train and before?
09:43:26 bauzas gibi: sure, let's do it
09:48:46 opendevreview Sylvain Bauza proposed openstack/nova master: DNM: Add logging for leaking out the non-poisoned libvirt testcase https://review.opendev.org/c/openstack/nova/+/872975
09:49:05 bauzas gibi: ^
10:10:47 gibi bauzas: thanks
10:17:43 bauzas folks, on os-traits, given we only merged https://opendev.org/openstack/os-traits/commit/feb3e28a00eeab0d4dbf097dd801aff3adeb10d6 we don't need a release
10:18:21 bauzas that being said, there are 2 open changes that are related to some accepted nova blueprint https://review.opendev.org/q/status:open+project:openstack/os-traits
10:18:57 bauzas elodilles: I just approved https://review.opendev.org/c/openstack/os-traits/+/871226 on os-traits
10:19:06 bauzas elodilles: we'll need a release
10:21:10 bauzas cores, need a second +2/+W on https://review.opendev.org/c/openstack/os-traits/+/872185 to let Uggla not blocked by this os-trait change
10:21:37 bauzas even if there are very little chances to have virtiofs-scaphandre series to be mergeable in Antelope
10:25:19 opendevreview Merged openstack/os-traits master: Add new 'COMPUTE_ADDRESS_SPACE_*' traits https://review.opendev.org/c/openstack/os-traits/+/871226
10:30:06 gibi bauzas: it is in merge conflict now so I will rebase it and approve it
10:31:49 opendevreview Merged openstack/os-resource-classes master: Change minversion of tox to 3.18.0 https://review.opendev.org/c/openstack/os-resource-classes/+/791974
10:33:03 opendevreview Balazs Gibizer proposed openstack/os-traits master: Add 'COMPUTE_SHARE_LOCAL_FS' https://review.opendev.org/c/openstack/os-traits/+/872185
10:34:18 gibi bauzas: done ^^
10:34:41 gibi bauzas: we need to bump the os-traits min version in placement once there is an os-traits release for A
10:42:01 elodilles bauzas: ack, let me know when os-traits can be released
10:43:34 gibi elodilles: I think we only waiting for https://review.opendev.org/c/openstack/os-traits/+/872185 to land then we are good to go
10:43:57 gibi the rest of the open os-traits reviews seems to be not related to the a release
10:44:05 gibi s/a release/A release/
10:46:47 elodilles gibi: ack, thanks
10:48:13 elodilles as I see that patch will land within 2-3 minutes if everything goes well
11:02:50 opendevreview Merged openstack/os-traits master: Add 'COMPUTE_SHARE_LOCAL_FS' https://review.opendev.org/c/openstack/os-traits/+/872185
11:11:53 elodilles and merged ^^^, do you want me to create the release patch?
11:14:30 gibi elodilles: if it means you cannot +2 it then I will create the release patch :)
11:22:30 elodilles gibi: then please do o:)
11:25:03 gibi on it
11:27:33 opendevreview John Garbutt proposed openstack/nova stable/victoria: Revert "Revert resize: wait for events according to hybrid plug" https://review.opendev.org/c/openstack/nova/+/857423
11:30:58 gibi elodilles, bauzas https://review.opendev.org/c/openstack/releases/+/873106
11:34:30 elodilles thanks! will review when the release jobs have finished
11:37:58 gibi elodilles: cool, and bauzas will be back soon I guess to review it too
11:38:28 bauzas just done
11:42:18 elodilles +2'd!
11:42:45 elodilles bauzas: Bon Appétit!
11:42:45 elodilles bauzas: Bon Appétit!
11:45:18 gibi bauzas: enjoy
12:28:54 gibi bauzas: I just realized that we don't have debug log enabled in the functional test by default so I will going to change https://review.opendev.org/c/openstack/nova/+/872975 to log a warning instead

Earlier   Later