Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-09-05
10:51:17 gibi yes, ReaderWriterLock rely on current_thread for reentrancy
10:52:01 sean-k-mooney this was changed in januaary
10:52:16 sean-k-mooney could you try downgrading fastener to 0.17.2
10:52:30 gibi but the fact that ReaderWriterLock depends on current_thread was not introduced there, it was there before https://github.com/harlowja/fasteners/pull/87/files#diff-bdd827bd84626190e8a93d1a50782b998b426261511e653de5bb775e9082e1f3L169
10:52:32 sean-k-mooney gibi: it did not used to in the reader writer case
10:53:04 sean-k-mooney right but it used to prevent geting the reader lock if there were any writers
10:53:46 gibi olso depends on the writer lock it seems https://github.com/openstack/oslo.concurrency/blob/master/oslo_concurrency/lockutils.py#L288
10:53:56 gibi and the writer part had reentrancy before 0.17.2
10:54:34 sean-k-mooney im oging to try your oslo repoducer and downgrade it just to see
10:54:49 gibi and writer lock is affected independently from the 0.17.2 https://github.com/harlowja/fasteners/pull/87/files#diff-bdd827bd84626190e8a93d1a50782b998b426261511e653de5bb775e9082e1f3L208
10:56:36 sean-k-mooney synconise is takign a write_lock ya?
10:56:44 sean-k-mooney if so then its not related to that change
10:57:00 sean-k-mooney but the is_writer code is not eventlet safe
10:57:09 sean-k-mooney likely becuase of the workaround you mentioned they remvoed
10:57:51 sean-k-mooney https://github.com/harlowja/fasteners/commit/467ed75ee1e9465ebff8b5edf452770befb93913
10:58:31 sean-k-mooney so 0.15 dropped that
11:00:12 gibi yes it is broken since 0.15
11:01:10 gibi it is effecting master and yoga
11:01:15 gibi in xena we have < 0.15
11:12:53 sean-k-mooney https://github.com/harlowja/fasteners/issues/96
11:13:20 sean-k-mooney at leasst they can triage ^ and decied if its somethign they want to fix
11:20:23 gibi thanks
11:33:01 opendevreview Balazs Gibizer proposed openstack/nova master: Fix fair internal lock used from eventlet.spawn_n https://review.opendev.org/c/openstack/nova/+/855717
11:33:24 gibi added the fasteners issue link and hopefully stabilized the unit test ^^
11:34:00 opendevreview Balazs Gibizer proposed openstack/nova stable/yoga: Fix fair internal lock used from eventlet.spawn_n https://review.opendev.org/c/openstack/nova/+/855718
11:34:20 sean-k-mooney im going to propose two reverts to fasteneres and assocaite it with the issue link too incase they deciced that that is approicate
11:35:30 gibi ack
11:40:33 sean-k-mooney https://github.com/harlowja/fasteners/pull/97
11:47:10 gibi thanks
13:00:34 opendevreview Balazs Gibizer proposed openstack/nova master: Show candidate combinatorial explosion by dev number https://review.opendev.org/c/openstack/nova/+/855885
13:00:46 gibi sean-k-mooney: here are the numbers of the combinatorial explosion
13:00:48 gibi ^^
13:01:16 gibi in case of single device per RP (ie PCI or PF) we have worst case factorial amount of candidates
13:01:48 gibi in case a single RP provides more than one devices (n VFs for a PF RP) then worst case we have exponential candidates
13:04:09 gibi and placement first generate all of them then limit the result based on the limit queryparam https://github.com/openstack/placement/blob/723da65faf66cc9b8d02f3756387dc58437e62af/placement/objects/research_context.py#L289-L292
13:13:19 gibi so this probably needs a bit of (probably massive) refactoring if we want to avoid placement to blow up on 8 devices
13:13:34 gibi we need to inline the limit somehow
13:15:45 gibi but by that we would potentially loose viable candidates
13:16:12 gibi so placement alone cannot decide where to limiot
13:23:49 gibi So modeling similar PFs of PCI devs does not help as it would lead to the VF scenario.
13:24:39 gibi Also we cannot model count=n as a single group as placement never splits a suffixed group to fit it into multiple RP
13:26:31 gibi while for nova it would be enough to have a small number of candidates per compute host, while we have nova side PCI filtering we need all candidates from placement as we don't know which will fulfill the nova side filtering
13:44:29 sean-k-mooney gibi: ya so this is exactly why we did not want each VF to ba an RP
13:44:40 sean-k-mooney we were very concerned it would explode like this
13:46:04 sean-k-mooney gibi: this feels a bit like the numa node combintorial issue
13:46:20 sean-k-mooney is this happening in sql or in python
13:46:28 gibi this is in python
13:46:47 gibi in case of numa we re-tried host - guest numa mapping multiple times
13:46:50 sean-k-mooney ack i wond if we can use itertools.combinations there in stead of permuations in that case
13:47:07 gibi we use itertools.product to generate all mapping between RPs and groups
13:47:29 sean-k-mooney hum ya i wonder if we really need all
13:47:43 gibi from placement perspective we need all
13:47:53 sean-k-mooney do you have a pointer to the code
13:48:04 gibi nova might be able to limit it by providing al the PCI filtering information to placement
13:48:26 sean-k-mooney gibi: we likely can take a perhost limit and then use a genortator to limit the amount we return
13:49:00 sean-k-mooney gibi: to porvide all the info we would also need to pass the numa toplogy info which would change the tree structure
13:49:11 sean-k-mooney doable but a lot of work
13:49:16 gibi the perhost limit has the problem that if nova still filters out PCI devices after placement then we need to make sure that placement returns enough candidate to fulfill that extra filtering
13:49:40 sean-k-mooney https://github.com/openstack/placement/blob/c68d472dca6619055579831ad5464042f745557a/placement/objects/allocation_candidate.py#L364-L387
13:50:01 gibi yeh I linked it above :)
13:50:05 sean-k-mooney gibi: ya its the same issue with the current request limit
13:50:38 sean-k-mooney you linked to the resarch context
13:50:49 sean-k-mooney unless i missed it
13:51:03 gibi ahh sorry yes
13:51:12 gibi in the commit message I linked to the product call
13:51:26 gibi https://review.opendev.org/c/openstack/nova/+/855885/1//COMMIT_MSG#26
13:51:34 sean-k-mooney ack
13:54:16 gibi at the moment I don't think this is easy to fix and given my time allocation for the next months I won't start on it.
13:54:35 gibi songwenping_ might have time and ideas to hack on it
13:55:12 gibi I left the above functional test top of the PCI series so we will not forget that this needs to be fixed
13:55:35 gibi but this makes me question if we want to merge the scheduling support in AA
13:55:48 gibi it might be useful for small deployments (<8 devs per host)
13:56:00 gibi but it is dangerous for big deployments
13:56:42 gibi bauzas: do we have a PTG etherpad?
13:56:49 sean-k-mooney im wondering if we need to have a way to limit this form the api query
13:56:59 bauzas gibi: not yet, but I can create one
13:57:08 gibi bauzas: I could use one :)
13:57:19 bauzas as you want
13:58:55 gibi sean-k-mooney: if we limit the a_c query then we need to give hints to placement about which order to iterate the candidates to fill the limited response
13:59:20 gibi sean-k-mooney: but I'm not sure I can express what we need
13:59:59 gibi sean-k-mooney: it is skip those candidates that are "too similar" to an already found candidate
14:01:04 sean-k-mooney gibi: im wonderign if we can avoid it by generatting a suffictly diverse set of combinations
14:01:19 gibi i.e. in case RP1(2), RP2(2), G1(1), G2(1) -> (RP1-G1, RP2-G2) and (RP1-G2, RP2-G1) might be too similar if G1 an G2 asks for the same RC and traits
14:01:52 gibi yeah divers set of candidates == skip the too similar ones :)
14:02:03 gibi but what is divers might be not universal
14:02:55 gibi like ir RP2 is remote_managed=True in nova then placement still sees the same symmetry but the two RP is not equivalent from nova perspecgive
14:02:59 gibi perspective
14:04:12 sean-k-mooney right so we want to ignore order when lookign at equvialce provided the request group is the same
14:04:40 sean-k-mooney i feel likel there is definetly a way to optimise so that we dont generate a product
14:04:51 sean-k-mooney that is goign to over produce results
14:05:19 sean-k-mooney but off the top of my head im not sure the correct way to proceed
14:05:41 gibi bauzas: I've created https://etherpad.opendev.org/p/nova-antelope-ptg
14:06:34 sean-k-mooney lookingat https://docs.python.org/3/library/itertools.html#itertools-recipes
14:06:34 gibi sean-k-mooney: I agree on the second part "but off the top of my head im not sure the correct way to proceed" but I'm not sure we can have better than actually iterating the product
14:07:44 sean-k-mooney maybe take(per_host_limit, random_product(...))
14:09:30 gibi yeah random is a way out even if a dirty one
14:09:38 sean-k-mooney gibi: if we cant avoid the need to generate the product im wonderign if we can break the implict lexegfacical ordering
14:10:14 opendevreview Amit Uniyal proposed openstack/nova master: add regression test case for bug 1552777 https://review.opendev.org/c/openstack/nova/+/855900
14:10:14 opendevreview Amit Uniyal proposed openstack/nova master: Adds check for instance resizing https://review.opendev.org/c/openstack/nova/+/855901
14:10:50 gibi if nova could define a requested order based on information that nova has but placement doesnt, then yes, ordering can be a solution. that is basically nova asking placement to generate diverse candiates with a definition of divers provided by nova in the a_c query

Earlier   Later