Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-01-15
16:04:00 JayF so it still would've failed, yes?
16:04:14 sean-k-mooney no before it was just an assert
16:04:19 sean-k-mooney so it woudl have done nothing at runtime
16:04:40 sean-k-mooney assert are off unless you run your interperter in debug mode
16:05:17 sean-k-mooney this was added in 0.36.0
16:07:13 sean-k-mooney JayF: so oslo.db injects a time.sleep(0) on every "checkin" event emmited by the sqlalchemy engine
16:07:15 sean-k-mooney https://github.com/openstack/oslo.db/blob/21c484ffb26ed03cbb1ca9dd7fd27fb19a1d4b36/oslo_db/sqlalchemy/engines.py#L360-L361
16:07:46 JayF aha
16:07:53 JayF sean-k-mooney: I'd file a bug against that change then in eventlet
16:08:14 JayF sean-k-mooney: it's going to break so much stuff in so many sneaky ways
16:09:26 sean-k-mooney JayF: nova's docs have this by the way https://docs.openstack.org/nova/latest/reference/threading.html#yielding-the-thread-in-long-running-tasks
16:10:05 JayF so my comment of "replace the time.sleep() with an if eventlet: eventlet.sleep()" was pretty close to a solution, seemingly?
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] Thanks for the help! So i did a local rebase agains master and tried git review again but still same error
16:38:53 MengyangZhang[m] ```
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:39:10 MengyangZhang[m] git rebase master
16:39:10 MengyangZhang[m] Current branch mzhang-branch is up to date.
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

Earlier   Later