| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2026-08-25 | |||
| 08:44:58 | gibi | I ran a lot of functional-threading tests locally and reasonable amount of it in CI before we made the functional-py313-threading voting and it was all shiny 3 weeks ago. Now I have multiple bugs filed on new race conditions | |
| 08:55:18 | gibi | gmaan: https://review.opendev.org/c/openstack/nova/+/1002223 needs more tweaking according to CI | |
| 09:04:34 | gibi | I assigned two bugs to myself and started looking for a root cause / fix https://bugs.launchpad.net/nova/+bug/2164606 and https://bugs.launchpad.net/nova/+bug/2164893 they are more frequent than the rest of non-graceful hits | |
| 09:51:30 | gibi | btw I can trigger some new race conditions locally if I artificially restric the amount of cpu and io I give to tox via | |
| 09:51:41 | gibi | systemd-run --user --scope -p CPUQuota=400% -p IOReadBandwidthMax="/ 2M" tox -e functional-py313-threading | |
| 09:55:51 | opendevreview | Balazs Gibizer proposed openstack/nova master: Stabilize TestInstanceNotificationSample.test_instance_action https://review.opendev.org/c/openstack/nova/+/1002246 | |
| 09:56:10 | gibi | OK that is one fix for the many ^^ | |
| 10:07:40 | opendevreview | Takashi Kajinami proposed openstack/nova master: Remove unused default argument https://review.opendev.org/c/openstack/nova/+/922073 | |
| 11:16:17 | sean-k-mooney | 'compute.exception' is a notification? | |
| 11:16:34 | sean-k-mooney | is that like any uncuaght excpetion on the compaute agent | |
| 11:16:56 | sean-k-mooney | or something we send at a specific time | |
| 11:19:18 | sean-k-mooney | i mean that definetly a think that exists and we are waitign for it in rebuild filaers https://github.com/openstack/nova/blob/9aa9a54e04fdfec9a8775bf90488558783179d4b/nova/tests/functional/test_servers_resource_request.py#L2230-L2232 | |
| 11:19:23 | sean-k-mooney | so the pattern looks right | |
| 12:03:35 | gibi | sean-k-mooney: yepp compute.exception is a catch all notification | |
| 12:04:10 | sean-k-mooney | i +2'd with a note about doing it for our only other *_with_error helper as well | |
| 12:05:00 | sean-k-mooney | i assume we may have a similar race for share detach as share attach | |
| 12:06:11 | gibi | yepp you are right and I'm updating the patch | |
| 12:06:12 | sean-k-mooney | i dont know if it has a simialr assertion however hence +2, if you think its needed ill happilry re review | |
| 12:06:39 | sean-k-mooney | cool i didnt really want ot dig into that to confirm | |
| 12:15:06 | opendevreview | Balazs Gibizer proposed openstack/nova master: Stabilize TestInstanceNotificationSample.test_instance_action https://review.opendev.org/c/openstack/nova/+/1002246 | |
| 12:15:15 | gibi | sean-k-mooney: fixed it up ^^ | |
| 12:32:37 | sean-k-mooney | oh it was listing them but only asserting 2 | |
| 12:32:40 | sean-k-mooney | fun | |
| 12:32:48 | gibi | :) | |
| 13:06:05 | opendevreview | Balazs Gibizer proposed openstack/nova master: Stabilize TestInstanceNotificationSampleWithMultipleCompute https://review.opendev.org/c/openstack/nova/+/1002281 | |
| 13:06:15 | gibi | another fix ^^ | |
| 13:10:00 | gibi | I'm taking https://bugs.launchpad.net/nova/+bug/2164888 as well now | |
| 13:25:22 | clif | anyone familiar with a mypy failure around "ExceptionGroup" it complains the name is not defined despite it being a builtin... | |
| 13:31:39 | clif | also are nova tox functional tests able to be run locally? | |
| 13:31:53 | sean-k-mooney | an excpetion group is not a type | |
| 13:32:00 | sean-k-mooney | its a collection of types | |
| 13:32:19 | sean-k-mooney | as far as im aware at least | |
| 13:32:23 | sean-k-mooney | but no i ahve not seen that | |
| 13:32:34 | clif | wdym, it is a type isn't it? https://docs.python.org/3/library/exceptions.html#ExceptionGroup | |
| 13:33:02 | sean-k-mooney | hum ok added in 3.11 | |
| 13:33:19 | sean-k-mooney | it wasnt a type before this is the first release we have been abot to use 3.11 features | |
| 13:33:38 | sean-k-mooney | before expction groups were jsut a tuple of possible expctions | |
| 13:34:18 | clif | it seems to work fine based on local testing but mypy blows up on it in tox pep8 | |
| 13:34:48 | sean-k-mooney | why are you trying to define it in a type signigure? | |
| 13:35:10 | sean-k-mooney | 3.11 was for derive not the full feature | |
| 13:40:16 | clif | I'm not sure what you mean, I'm not using it as a type signature but as a raised exception due to stephenfin's review comment | |
| 13:40:23 | sean-k-mooney | i would not use it here for example https://review.opendev.org/c/openstack/nova/+/980679/12/nova/compute/manager.py#11572 | |
| 13:40:37 | sean-k-mooney | which comment | |
| 13:41:03 | clif | https://review.opendev.org/c/openstack/nova/+/980679/comment/a6f02680_f3646440/ | |
| 13:41:05 | stephenfin | clif: Got the review link to hand? I can take a look | |
| 13:41:09 | stephenfin | nvm :) | |
| 13:41:23 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/980679/comment/a6f02680_f3646440/ | |
| 13:41:37 | sean-k-mooney | so i dont think we shoudl be raiseing an execption groupu there | |
| 13:41:50 | sean-k-mooney | i intentlaly was raisign only the first excption | |
| 13:42:18 | sean-k-mooney | we coudl do that but we cant assuem the srounding code will handle the group properly | |
| 13:42:55 | stephenfin | clif: look at tool.mypy.python_version in pyproject.toml :( | |
| 13:43:04 | clif | stephenfin: aha :( | |
| 13:43:31 | clif | is it fine to bump that to 3.11? | |
| 13:43:33 | stephenfin | not the first itme I've been bitten by that https://github.com/python/mypy/issues/19349 | |
| 13:43:51 | sean-k-mooney | ah 3.10 so it was a 3.11 related chagne | |
| 13:44:08 | stephenfin | clif: yeah, it should have been done when we bumped project.requires-python but obviously got missed | |
| 13:45:03 | stephenfin | just remember to run `tox -e mypy` locally before submitting since mypy _might_ raise new warnings about e.g. things that were newly deprecated in Python 3.11 | |
| 13:45:14 | stephenfin | sean-k-mooney: ExceptionGroup is new in 3.11, yup | |
| 13:45:16 | sean-k-mooney | will an expction group match an excpt block that only matches on the encloded exeptions? | |
| 13:45:28 | sean-k-mooney | and will it properly handel all of them or just one | |
| 13:45:41 | stephenfin | I believe so, but there's a very easy way to test it ;) | |
| 13:45:44 | opendevreview | Clif Houck proposed openstack/nova master: Bump python_version for mypy to 3.11 https://review.opendev.org/c/openstack/nova/+/1002290 | |
| 13:46:04 | sean-k-mooney | stephenfin: right so the issue i have is your chanign the behvior of the algorthim not just changing the typing | |
| 13:46:17 | sean-k-mooney | again it was intentlly only rasing the first excption not all of them | |
| 13:46:36 | sean-k-mooney | it was loging all of them | |
| 13:49:34 | stephenfin | Sorry, what algorithm was changed? | |
| 13:50:56 | sean-k-mooney | it now raising all the expctions as a group instead of just the first one | |
| 13:51:08 | sean-k-mooney | in the non paralle mode it would have failed after the first expction | |
| 13:51:21 | sean-k-mooney | so the intent was to keep the external behvior the same | |
| 13:51:36 | sean-k-mooney | we can rasie all of them as a group but that a slightly diffenernt semantic | |
| 13:52:19 | stephenfin | (a) that desire isn't captured anywhere so I'm not sure if that was the intent | |
| 13:52:34 | sean-k-mooney | that was part of the code i wote | |
| 13:52:42 | sean-k-mooney | before clif took it over so that was the intent | |
| 13:52:46 | stephenfin | and (b) the behavior has already changed since in the non-parallel mode, only N/M operations would have run (where the Nth operation raised the exception) whereas now M operations will run | |
| 13:54:35 | stephenfin | if operation N raises FooException and N+X raises BarException, raising only FooException is lossy and makes things harder to debug. My suggesion was to try to avoid that | |
| 13:54:44 | sean-k-mooney | both can work but im not sure what we get by raisign all of them | |
| 13:55:14 | sean-k-mooney | stephenfin: that why all of them were logged | |
| 13:55:16 | sean-k-mooney | for debugging | |
| 13:55:29 | sean-k-mooney | stephenfin: we intend to backport this | |
| 13:55:40 | sean-k-mooney | so that why im qustioning using a 3.11 feature | |
| 13:55:52 | stephenfin | okay, backporting does change the equation. I wasn't aware of that | |
| 13:55:58 | sean-k-mooney | well we intned to backprot he prevsiou patch this wone was deabateable | |
| 13:57:58 | sean-k-mooney | we can modify it on backport if requried but ya the hope ofr the O(n2) patch at least was to backport to all stable branches | |
| 13:58:23 | sean-k-mooney | the parallisze i know operator will want it backported but im not sure it fully fits stabel policy | |
| 13:58:29 | stephenfin | okay, if that's the plan then yeah, we'd want to go back to just raising the first exception | |
| 13:58:31 | sean-k-mooney | so that patch may be propsoedn abut not mergd | |
| 13:59:23 | sean-k-mooney | *may be proposed but not merged | |
| 13:59:48 | sean-k-mooney | we can do both we could add a patch to swap to excption groups as a followup | |
| 14:00:02 | stephenfin | clif: ☝️ apologies for the slight waste of your time there 🙇 | |
| 14:00:04 | sean-k-mooney | but if we can keep this 3.10 compatiable it woudl be nice | |
| 14:03:41 | sean-k-mooney | stephenfin: since your about https://review.opendev.org/c/openstack/nova/+/1002246 gibi puhsed htat to stablise a functional test mind taking a look | |
| 14:11:14 | clif | ok, yea I can revert the ExceptionGroup change | |
| 14:12:28 | clif | relatedly, some functional tests are failing on the parallel per-node resource update change, for instance: nova.tests.functional.libvirt.test_pci_in_placement.PlacementPCIInventoryReportingTests.test_device_reconfiguration_with_allocations_config_change_stop fails because the exception raised has changed | |
| 14:14:58 | clif | any advice on that? | |
| 14:19:22 | clif | well now that I think about it some the failures may be related to the exceptiongroup change | |
| 14:34:48 | clif | ah yea it was the exceptiongroup changing how exceptions got raised, makes sense | |
| 14:39:23 | opendevreview | Clif Houck proposed openstack/nova master: Parallelize per-node resource updates https://review.opendev.org/c/openstack/nova/+/980679 | |
| 15:05:10 | gmaan | gibi: ack, will check | |
| 15:19:58 | gibi | gmaan: thanks | |
| 15:23:11 | gmaan | I saw this again, I think this hit twice, I will work on this today https://bugs.launchpad.net/nova/+bug/2164880 | |