Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-09-02
13:26:03 sean-k-mooney many a littl latere
13:26:09 sean-k-mooney then FF woudl be febuary 14
13:26:25 sean-k-mooney so about 6 week of time after the winder break
13:26:56 sean-k-mooney gibi: ideally i would hope we could merge the rest of the pci seriese by the end of septemeber or early october
13:27:18 sean-k-mooney but we will have to see how RC1 goes
13:28:02 gibi it is feature complete today :) (but to be honest how I store a list of RP UUIDs in InstancePCIRequest.extra_info might need a rework)
13:28:49 sean-k-mooney i have not looked at that bit yet
13:29:08 gibi https://review.opendev.org/c/openstack/nova/+/854121/13/nova/compute/utils.py#1537
13:29:17 sean-k-mooney i was expecting the uuid in the device extra info column
13:29:19 sean-k-mooney but only one
13:29:21 gibi extra_infor is a dict of strings
13:29:31 gibi and I needed to store a list of UUIDs
13:29:37 gibi so I serialized /o\
13:29:57 gibi one InstancePCIRequest might be fulfilled from a list of RPs
13:30:03 gibi due to count > 1
13:30:39 gibi we need the mapping _before_ the PciDevice object is allocated to drive the selection
13:30:42 sean-k-mooney hum ill have to think about htat
13:30:46 gibi sure
13:31:03 sean-k-mooney i was epecting to be able to do a 1 :1 mappign of each request object to one rp uuid from the allocation
13:31:24 sean-k-mooney since we are going to split the flavor based allcoation into multipel request objects
13:31:30 gibi request group - RP is in 1:1 but I did not split InstancePCIRequest objects jut the RequestGroups
13:31:45 sean-k-mooney we shoudl split the object too i think
13:32:08 sean-k-mooney that way you can directly map each rest object to the group and allocation
13:32:46 sean-k-mooney is there a downside to doing that? at least for new code path
13:33:29 sean-k-mooney you dont need to answer that now just woth thinking about
13:33:48 gibi the stats module does many things per InstancePCIRequest today
13:34:04 gibi but it already handles that a single instance has multiple requests
13:34:35 sean-k-mooney yep it need to because of neutron and the fact we can have multiple differnt alaises
13:34:49 sean-k-mooney i think we can simplfy the code and basicaly alway have a count of 1
13:35:10 gibi yeah , if we can split, then the count filed become unused
13:35:37 sean-k-mooney i think thats ok
13:35:39 gibi and some logic where we loop on request and the loop on count can be refactored to a single loop
13:35:54 sean-k-mooney we can drop it in a future version if we rev the object major version
13:36:52 gibi the only thing where we have to be careful is code that might did some affinity or block based decision based on a single request with count > 1
13:37:11 gibi instead of doing it device by device
13:37:21 gibi the filter_pools code need to be checked
13:37:51 sean-k-mooney gibi: well it was ment to be per device today
13:38:00 sean-k-mooney so if it was per alias that would be a bug
13:38:16 sean-k-mooney i think you noted that the request could be fullfiled form differnt pools already
13:38:30 sean-k-mooney so hopefuly that all works
13:38:42 sean-k-mooney but soudn like more functional test can prove that one way or another
13:40:11 gibi yeah, I'm not afraid of the actual consumption part as that already needs to work per device as we can have multiple RP uuid per PCIreq in the current series
13:40:26 gibi this was my last fix btw ^^
13:41:02 sean-k-mooney ah ok
13:41:10 sean-k-mooney that was the bug
13:41:12 frickler gibi: sean-k-mooney: prettytable 3.4.1 was just releases which reverts the broken change. nova tests work fine for me with that locally. does one of you want to propose the exclude in reqs?
13:41:42 gibi frickler: if you already at it then feel free to propose the exclude
13:41:43 sean-k-mooney frickler: should we proceed with hardeing the nova code anyway
13:42:31 sean-k-mooney my patch works with 3.4.1 too it seams
13:43:05 frickler can't hurt to be on the safe side, then, I'd say
13:43:56 sean-k-mooney ok ill keep it open and backport it to yoga so
13:44:26 sean-k-mooney at least that way if a disto hits this issue or they make the chagne in 4.0 we will be fine
13:48:18 frickler now I only need to find out the path it get's pulled in, since it doesn't seem to be a direct dependency
13:52:58 sean-k-mooney frickler: PrettyTable ?
13:53:07 sean-k-mooney nova depend on it directly but i guess we might not list it
13:53:34 sean-k-mooney https://github.com/openstack/nova/blob/master/requirements.txt#L18
13:53:38 sean-k-mooney its there
13:53:57 bauzas sean-k-mooney: I checked all the libs and we don't need a new release
13:54:01 bauzas did I miss one ?
13:54:35 sean-k-mooney we may want anotther release of python-novaclint
13:54:37 sean-k-mooney brb
13:54:45 sean-k-mooney if we merge some pending patches
14:02:20 opendevreview Slawek Kaplonski proposed openstack/nova master: WIP Don't provide MTU value in metadata service if DHCP is enabled https://review.opendev.org/c/openstack/nova/+/855664
14:04:39 frickler sean-k-mooney: ah, CaseMismatch in my search, thx
14:08:38 sean-k-mooney gibi: i may hae missed something but https://review.opendev.org/c/openstack/nova-specs/+/855218 looks good over all a cople of nits inline
14:08:49 sean-k-mooney im gong to read it again quickly
14:09:32 gibi thanks I will fix the nits a bit later todayt
14:10:49 bauzas fwiw, I'm not using the -2 hammer yet
14:10:56 bauzas for all the open changes
14:11:13 bauzas I'll do it only on Tuesday
14:13:50 sean-k-mooney unless there is anything urgnet im going to take a break form lookign at upstream reviews and work on some automation
14:15:41 bauzas => goes getting his kid from school
16:34:08 gibi I think lockutils.synchronized(...fair=True) + eventlet.spawn_n() + fastener > 0.15 actually breaks nova proper. Not just the test code that we fixed in https://review.opendev.org/c/openstack/nova/+/813114
16:34:25 gibi here is a minimal reproductionhttps://gist.github.com/gibizer/9051369e67fd46a20d52963dac534852
16:34:28 gibi https://gist.github.com/gibizer/9051369e67fd46a20d52963dac534852
16:35:32 gibi the realization came when I looked at the logs in https://bugs.launchpad.net/nova/+bug/1988311//
16:35:56 gibi those logs shows that two rebuild_claim can take the same lock twice
16:36:37 sean-k-mooney isnt it an instantace lock
16:36:42 sean-k-mooney or is it the rt lock
16:37:05 sean-k-mooney it must be the rt lock actully
16:37:18 gibi it is the rt lock
16:37:25 gibi https://github.com/openstack/nova/blob/8b55b44cc605533f2a12189a2b5899c0f58c91a7/nova/compute/resource_tracker.py#L201-L202
16:38:14 sean-k-mooney i havent looked in deail but the lock name would be the same
16:38:29 sean-k-mooney i assuem you are loking at someting in the outpu specificaly
16:38:31 gibi yes it is compute_resources
16:38:47 sean-k-mooney that shows they are taking the same lock wtich
16:38:47 gibi https://bugs.launchpad.net/nova/+bug/1988311/comments/3
16:38:53 gibi yepp
16:39:36 sean-k-mooney are we context switch between the two coroutines inside the critical section under the lock?
16:39:58 sean-k-mooney and there for data racing?
16:40:36 gibi the bug is written as two pinned VM evacuated and ended up selecting overlapping cpus
16:41:13 sean-k-mooney yep so unlike pci devices we dont enforce that in the db
16:41:28 sean-k-mooney the only protection we have is the rt lock
16:41:41 sean-k-mooney to ensure we claim the cpus and update the host numa toplogy blob
16:42:05 sean-k-mooney then we regrenrate that over time based on the instance numa toplogy blob in the perodic
16:42:28 sean-k-mooney so if this lock is broken its very posible for that to break and not be able to fix itslef
16:43:00 sean-k-mooney it need to not only prorect against the concurrent evacuate btu also the preiodic running
16:43:44 gibi yes
16:43:56 gibi and we have the rt lock around many actions
16:47:30 sean-k-mooney have you treid https://review.opendev.org/c/openstack/nova/+/842359/5/nova/monkey_patch.py

Earlier   Later