| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-01-15 | |||
| 16:10:14 | sean-k-mooney | we have documented the use of time.sleep(0) or greenthread.sleep(0) since before we supproted python3 | |
| 16:10:45 | JayF | yeah, so let the eventlet folks know so they can revert the problematic change :) | |
| 16:10:55 | JayF | or at least maybe except sleep(0) | |
| 16:11:08 | sean-k-mooney | JayF: i dont think it will actully change the behivor | |
| 16:11:33 | sean-k-mooney | what i can do is try locally reverting it in my venv | |
| 16:11:38 | sean-k-mooney | and see what happens | |
| 16:12:00 | sean-k-mooney | well hum i guess that wont tell me much | |
| 16:12:43 | sean-k-mooney | it will likely fix the issue but that because asssert are ignored | |
| 16:13:31 | sean-k-mooney | JayF: ill file an eventlet bug and push a revert patch, this is the watcher bug i had previously filed for this https://bugs.launchpad.net/watcher/+bug/2086710 | |
| 16:13:50 | JayF | I suspect the nice middle ground might be | |
| 16:13:59 | JayF | RuntimeError() if the sleep is nonzero | |
| 16:14:02 | JayF | but IMBW | |
| 16:14:11 | JayF | I also don't fully understand the case they're guarding against in eventlet | |
| 16:14:21 | JayF | or maybe I should say; why it's bad to do that | |
| 16:14:29 | sean-k-mooney | ya i think that could work. | |
| 16:14:48 | sean-k-mooney | what we are really using sleep(0) for is to yeild form teh current corotie back to the hub | |
| 16:15:07 | sean-k-mooney | so you could say well we shoudl have done hub.swtich() or the equivlent | |
| 16:15:57 | sean-k-mooney | but im pretty sure the eventlet docs used ot docuemtn time.sleep(0) too | |
| 16:16:59 | sean-k-mooney | JayF: ah the docsting says it all https://github.com/eventlet/eventlet/blob/3450e6adb017f4cd47d1c4e06331c9fa26a986d1/eventlet/greenthread.py#L25-L27 | |
| 16:17:33 | JayF | yeah | |
| 16:17:35 | sean-k-mooney | JayF: so ya i think they just forgot to make this condtional | |
| 16:17:38 | JayF | ++ | |
| 16:17:50 | JayF | If you can file that issue and/or put up a PR; I'll point itamar at it | |
| 16:18:00 | JayF | I can't tell him to work on it, but he's generally cooperative :D | |
| 16:18:34 | sean-k-mooney | thanks for the fresh persepctive i didnt think to see if this was a eventlet regression just because i expected this to kill nova and other things too | |
| 16:23:52 | JayF | I've moved beyond "who regressed" and into "the whole thing is so damn fragile any change can have massive ripple effects" | |
| 16:23:55 | JayF | lol | |
| 16:24:10 | JayF | is it eventlet's fault? is it openstack's fault? yes. all of us. lol | |
| 16:24:46 | sean-k-mooney | well i tought it was an issue with apschduelr because it does nto actully supprot uage with eventlet and is spwaning backgorund threads | |
| 16:25:06 | sean-k-mooney | os i tried to fix it by ensuring the backgound thread was monkey patched initally | |
| 16:25:57 | sean-k-mooney | JayF: watcher is in one optional part of the code also mixing asyncio with eventlet without usign the asyncio hub | |
| 16:26:38 | sean-k-mooney | so i was going down the line "ok watcher is mixing may concurance models so it must be a watcher issue becuase nova is ok" | |
| 16:29:27 | MengyangZhang[m] | Hello, I have a question about git review. I was following the Gerrit workflow tutorial https://docs.opendev.org/opendev/infra-manual/latest/sandbox.html#sandbox and ran into this error when running git review. I... (full message at <https://matrix.org/oftc/media/v1/media/download/AYfP68eKdfAp4GdhSOGMJNoxIcgRvFjHe-ES5V35PeyBtFAsEz-1GYjZH3yveUoKSXQ1CvR5Y8ZkyioBMNdqdLtCeUsuVqzQAG1hdHJpeC5vcmcvUHNabUJzV29SVVdnS2FwYUtYYm11d3RR>) | |
| 16:30:12 | JayF | MengyangZhang[m]: that's a new one on me, perhaps #opendev would have more insight? | |
| 16:30:52 | MengyangZhang[m] | okk thanks for quick response. I will raise it there. | |
| 16:31:08 | JayF | good luck! | |
| 16:31:21 | sean-k-mooney | MengyangZhang[m] so i think this is related to the verion of git you have installed | |
| 16:31:57 | MengyangZhang[m] | mine is 2.39.2 | |
| 16:32:00 | sean-k-mooney | i have not hit that but you coudl work around it by doing a local rebase agaisnt master before doing "git revew" | |
| 16:32:13 | sean-k-mooney | for what its worth we do nto allow merge commits to be part of a review | |
| 16:33:13 | sean-k-mooney | i have git version 2.46.0 | |
| 16:33:20 | sean-k-mooney | so your version is not that old | |
| 16:33:24 | JayF | I've absolutely used git review w/git 2.39.2 at some point, I think it's unlikely to be that | |
| 16:33:33 | JayF | sean-k-mooney may have id'd the issue -- if there are any merge commits in your push | |
| 16:34:15 | sean-k-mooney | you probaly dont want pull.rebase=merges in your git cofnig either | |
| 16:35:33 | sean-k-mooney | i woudl set that to pull.rebase=true | |
| 16:35:42 | sean-k-mooney | or just unset it to get the default behvior | |
| 16:38:53 | MengyangZhang[m] | ``` | |
| 16:38:53 | MengyangZhang[m] | Thanks for the help! So i did a local rebase agains master and tried git review again but still same error | |
| 16:39:10 | MengyangZhang[m] | Current branch mzhang-branch is up to date. | |
| 16:39:10 | MengyangZhang[m] | git rebase master | |
| 16:39:10 | MengyangZhang[m] | * Thanks for the help! So i did a local rebase agains master and tried git review again but still same error | |
| 16:42:05 | JayF | Look at the git config changes that Sean suggested, perhaps? | |
| 16:43:41 | sean-k-mooney | so you shoudl do it like this | |
| 16:43:47 | sean-k-mooney | git fetch --all | |
| 16:43:56 | sean-k-mooney | git rebase -i gerrit/master | |
| 16:44:00 | sean-k-mooney | git review | |
| 16:44:29 | sean-k-mooney | if your local master is behind origin or gerrit master there may be a remote merge conflict that you are not seeing | |
| 16:47:02 | sean-k-mooney | JayF: ill see if i can find time to push a ptch to add teh if check later but i need to step away for a bit, this is the bug https://github.com/eventlet/eventlet/issues/1014 | |
| 16:54:38 | JayF | sean-k-mooney: sounds like Itamar agrees with our suggested fix, will work on it soon :) | |
| 16:55:35 | sean-k-mooney | oh awsome | |
| 16:56:29 | sean-k-mooney | im going to try hackign it in locally to confirm that it does nto fail later but that woudl be good if they had time | |
| 16:59:38 | JayF | it's a good fix regardless of if it resolves the watcher issue | |
| 16:59:46 | JayF | but hopefully it cleans up both | |
| 18:16:12 | opendevreview | ribaudr proposed openstack/nova-specs master: Fix a line that was not removed before merging the specification https://review.opendev.org/c/openstack/nova-specs/+/939380 | |
| 21:19:03 | opendevreview | Dmitriy Chubinidze proposed openstack/nova master: Correct description for AggregateMultiTenancyIsolation. https://review.opendev.org/c/openstack/nova/+/939398 | |
| 21:36:22 | opendevreview | Mengyang Zhang proposed openstack/nova master: Add Burst Length Support to Cinder QoS https://review.opendev.org/c/openstack/nova/+/939310 | |
| 23:37:29 | opendevreview | melanie witt proposed openstack/nova master: libvirt pwr mgmt: reproduce bug 2085122 https://review.opendev.org/c/openstack/nova/+/932922 | |
| 23:37:30 | opendevreview | melanie witt proposed openstack/nova master: pwr mgmt: power down free PCPUS when updating compute node https://review.opendev.org/c/openstack/nova/+/932926 | |
| #openstack-nova - 2025-01-16 | |||
| 00:42:53 | opendevreview | Merged openstack/nova master: Improve heal_instance_info_cache periodic https://review.opendev.org/c/openstack/nova/+/935858 | |
| 02:18:16 | opendevreview | melanie witt proposed openstack/nova master: pwr mgmt: power down free PCPUS when updating compute node https://review.opendev.org/c/openstack/nova/+/932926 | |
| 03:16:11 | opendevreview | suiong ng proposed openstack/nova master: Fix parameter order in add_instance_info_to_node https://review.opendev.org/c/openstack/nova/+/939411 | |
| 03:32:02 | opendevreview | Mengyang Zhang proposed openstack/nova master: Add Burst Length Support to Cinder QoS https://review.opendev.org/c/openstack/nova/+/939310 | |
| 04:01:10 | opendevreview | suiong ng proposed openstack/nova master: Fix parameter order in add_instance_info_to_node https://review.opendev.org/c/openstack/nova/+/939411 | |
| 09:48:51 | opendevreview | Merged openstack/nova master: Revert "libvirt: Wrap un-proxied listDevices() and listAllDevices()" https://review.opendev.org/c/openstack/nova/+/939262 | |
| 14:12:25 | gibi | bauzas: when you are aroun I'm +2 on https://review.opendev.org/c/openstack/os-traits/+/939264 . If you can approve it then Uggla can quickly propose a new os-traits release and we can start bumping the min version in nova | |
| 14:13:31 | bauzas | gibi: cool, I'll look | |
| 14:19:49 | opendevreview | Bogdan Dobrelya proposed openstack/nova master: Run nova-next without periodic cache healing https://review.opendev.org/c/openstack/nova/+/939453 | |
| 14:23:08 | opendevreview | Merged openstack/nova-specs master: Fix a line that was not removed before merging the specification https://review.opendev.org/c/openstack/nova-specs/+/939380 | |
| 14:24:53 | opendevreview | Merged openstack/os-traits master: Add 'HW_PCI_LIVE_MIGRATABLE' https://review.opendev.org/c/openstack/os-traits/+/939264 | |
| 15:04:08 | gibi | Uggla: ^^ you can propose a os-traits release now :) | |
| 17:08:33 | Uggla | gibi, yep I will do that. | |
| 17:46:14 | melwitt | sean-k-mooney, stephenfin: fyi the revert of the libvirt tpool proxy patch merged and I have reproposed the patch with unit tests added here https://review.opendev.org/c/openstack/nova/+/939317 if you might have a chance to re-review at your convenience | |
| 17:46:28 | stephenfin | already done | |
| 17:46:44 | melwitt | thanks stephenfin | |
| 17:47:04 | sean-k-mooney | cool ill take a look shortly | |
| 17:47:23 | sean-k-mooney | im runnging the funtional test locally and my laptop is a littel laggy right now | |
| 17:48:17 | melwitt | thanks stephenfin | |
| 17:48:21 | melwitt | argh | |
| 17:48:23 | melwitt | thanks sean-k-mooney | |
| 17:49:04 | sean-k-mooney | load average of 20 oin 8cores.16 thread... its fine.... | |
| 17:49:46 | sean-k-mooney | 22, the fact its going up is definally not an issue, nope nope nope :) | |
| 18:06:36 | melwitt | 😳 | |
| 18:08:25 | opendevreview | sean mooney proposed openstack/nova master: disable the heal instance info cache periodic https://review.opendev.org/c/openstack/nova/+/939476 | |
| 18:10:48 | sean-k-mooney | melwitt: two questions, since we are redoing https://review.opendev.org/c/openstack/nova/+/939317 do we want a release note too? second question is i have readded +2 but do we want to wait for dan to review before +w | |
| 18:11:40 | sean-k-mooney | melwitt: there isnt anything required of the operator so we dont technially need a release not i was just wondering if you think its important enough to call out as a fixes note or not | |
| 18:15:42 | sean-k-mooney | the upstream bug does not have an improants set althoguh the downstream ones you linked seam to be high/medium | |
| 18:15:55 | sean-k-mooney | so it might be worth callign out its up to you however | |
| 18:16:36 | melwitt | sean-k-mooney: hm, I hadn't considered that. I'm not sure how commonly people hit the issue but if they do hit it, I would consider it a pretty significant issue and worth a reno. and yeah probably a good idea to have dansmith look at it before +Wing | |
| 18:17:19 | dansmith | yeah it's probably worth one because if people don't know why their stuff is locking up, they're not going to be looking for something specific | |