| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-09-05 | |||
| 10:07:51 | sean-k-mooney[m] | anyway something to look into i guess | |
| 10:07:55 | gibi | yepp | |
| 10:08:36 | sean-k-mooney[m] | im going to grab a coffee and take my blood pressre meds and then check on freya be back in about 10 mins | |
| 10:09:02 | gibi | ack | |
| 10:09:22 | gibi | for me coffee is the blood pressure med | |
| 10:32:32 | sean-k-mooney | gibi: should fastener reintoudce the workaround they had | |
| 10:35:20 | gibi | that is a way too but I have less authority over that project | |
| 10:35:51 | sean-k-mooney | isnit it an oslo deliverable too | |
| 10:36:09 | sean-k-mooney | or has it move out of openstack | |
| 10:36:56 | sean-k-mooney | oh its not an openstack project | |
| 10:37:02 | sean-k-mooney | i tough it used to be at one point | |
| 10:38:21 | sean-k-mooney | i guess we can fix it in oslo but we should leth the know that under eventlet the rentrant guarentee is broken | |
| 10:38:25 | sean-k-mooney | https://github.com/harlowja/fasteners#-overview | |
| 10:38:35 | sean-k-mooney | then note that it should be reentrant | |
| 10:44:17 | sean-k-mooney | gibi: https://github.com/harlowja/fasteners/issues/86 | |
| 10:44:17 | sean-k-mooney | that also intereisng | |
| 10:44:17 | sean-k-mooney | https://github.com/harlowja/fasteners/pull/87/files | |
| 10:44:17 | sean-k-mooney | elif not self.has_pending_writers: | |
| 10:44:17 | sean-k-mooney | elif (self._writer == me) or not self.has_pending_writers: | |
| 10:49:45 | gibi | I'm not sure I follow how this connects to your current problem. | |
| 10:49:52 | gibi | our | |
| 10:50:16 | sean-k-mooney | its relying on threading.current_thread | |
| 10:50:35 | sean-k-mooney | an now it allows you to reaquire the lock if tha tis the same | |
| 10:50:47 | sean-k-mooney | but with spwan_n | |
| 10:51:00 | sean-k-mooney | that means two greenthread coudl get the same lock if they run on the same os thread | |
| 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 | |