| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-01-14 | |||
| 17:11:16 | sean-k-mooney | dansmith: sure i can | |
| 17:11:26 | dansmith | thanks | |
| 17:11:36 | dansmith | I'll stand by to fast approve :) | |
| 17:12:02 | opendevreview | sean mooney proposed openstack/nova master: Revert "libvirt: Wrap un-proxied listDevices() and listAllDevices()" https://review.opendev.org/c/openstack/nova/+/939262 | |
| 17:12:42 | opendevreview | sean mooney proposed openstack/nova master: Revert "libvirt: Wrap un-proxied listDevices() and listAllDevices()" https://review.opendev.org/c/openstack/nova/+/939262 | |
| 17:12:50 | sean-k-mooney | just fixed a typo in the commit | |
| 17:13:08 | opendevreview | ribaudr proposed openstack/os-traits master: Add 'HW_PCI_LIVE_MIGRATABLE' https://review.opendev.org/c/openstack/os-traits/+/939264 | |
| 17:17:53 | sean-k-mooney | dansmith: thinking about it, i guess the tpool.proxy is a "contains" or "has a" relationship rahter then an inheritance so self.assertIsInstance() works in that case. so that makes sense | |
| 17:18:38 | sean-k-mooney | we are not just replacing the metacass or simairl on teh orginl object, its fully wrapping it | |
| 19:52:37 | melwitt | sean-k-mooney: sorry about that. I'm not sure why I thought tpool proxy stuff in general wasn't covered in unit tests. I see now that there is, so I'll add to that area with the reproposal | |
| 19:56:16 | sean-k-mooney | melwitt: it was dansmith that noticed, i didnt think we had testign that the returned obejcts were proxies either | |
| 19:57:06 | dansmith | maybe it's just me, but any patch that touches production code and doesn't touch anything in the test tree at all should probably be a red flag, and at least have a note in the commit message about why it's not tested | |
| 19:58:50 | sean-k-mooney | ocationally its a pure refactor with no observable change, that how i was looking at this but that was not actully ture in this case | |
| 19:59:02 | sean-k-mooney | we can observe that the objects are wrapped | |
| 20:00:08 | melwitt | it was originally a test patch that I uploaded for someone to run experiments while I was debugging a downstream incident and then time passed and then for some reason I didn't find the existing test coverage. I thought it was something we don't test in general, but obviously I had missed it | |
| 20:01:50 | dansmith | sean-k-mooney: sure a refactor is a good reason to have no test coverage showing the refactor has no impact.. but this was not that :) | |
| 20:03:37 | sean-k-mooney | does anyone maintian storyboard or freezer anymore? | |
| 20:03:44 | sean-k-mooney | i assume no | |
| 20:07:44 | melwitt | dunno anything about freezer but afaik storyboard is no longer being developed | |
| 20:08:25 | sean-k-mooney | so there is an eventlet isseu with watcher im trying to debug | |
| 20:08:47 | sean-k-mooney | but very few projects use the apschduler lib | |
| 20:08:48 | JayF | sean-k-mooney: obligatory: have you ensured the new eventlet+oslo.log releases don't fix it? | |
| 20:08:57 | sean-k-mooney | and sofar i dont think they use eventlet | |
| 20:09:07 | sean-k-mooney | JayF: yep | |
| 20:09:18 | JayF | too bad :( | |
| 20:09:42 | sean-k-mooney | im tryign to confirm if wathcer is the only project that uses that lib and eventlet | |
| 20:10:09 | sean-k-mooney | zuul ueses apscheduler for example but i dotn thinik it uses eventlet | |
| 20:10:49 | sean-k-mooney | oh also the issue only happen on python 3.12 and the actual issue is happenign isn sqlachemy... | |
| 20:11:25 | sean-k-mooney | so you know tottally trivial to figure out what the hell is happening | |
| 20:12:13 | melwitt | good times | |
| 20:19:40 | sean-k-mooney | the fun part is they are working on a 4.0 release like sqlalchme 2.0 and have been in the process of that migration for a while | |
| 20:20:56 | sean-k-mooney | so even if i get this workign with 3.11 we weill have to eventually port to the new 4.0 interface, 3.x is still supproted at least | |
| 20:23:12 | sean-k-mooney | the 4.x brnach supprot sqlalchemy 2.0 im not sure that is true of the 3.x branch but that one of the paths im looking into | |
| 20:35:52 | opendevreview | Douglas Viroel proposed openstack/nova master: WIP - Add support for showing scheduler_hints in server details https://review.opendev.org/c/openstack/nova/+/938604 | |
| 20:36:10 | opendevreview | Douglas Viroel proposed openstack/nova master: Add support for showing scheduler_hints in server details https://review.opendev.org/c/openstack/nova/+/938604 | |
| #openstack-nova - 2025-01-15 | |||
| 02:53:59 | hemanth | Hi team, unfortunately I could not attend weekly meeting yesterday to ask about one of the item which I added in https://etherpad.opendev.org/p/nova-2025.1-status, appreciate if someone can review https://review.opendev.org/c/openstack/nova/+/939044 and provide feedback, thanks! | |
| 03:28:15 | opendevreview | Mengyang Zhang proposed openstack/nova master: Add Burst Length Support to Cinder QoS https://review.opendev.org/c/openstack/nova/+/939310 | |
| 06:46:17 | opendevreview | melanie witt proposed openstack/nova master: libvirt: Wrap un-proxied listDevices() and listAllDevices() https://review.opendev.org/c/openstack/nova/+/939317 | |
| 06:48:14 | melwitt | sean-k-mooney: stacked this on top of your revert fyi ^ | |
| 09:39:42 | opendevreview | Dmitriy Chubinidze proposed openstack/nova master: Using "dnf" instead of "yum" with uncommenting compute_driver option. https://review.opendev.org/c/openstack/nova/+/939325 | |
| 09:55:49 | opendevreview | Ivan Anfimov proposed openstack/nova master: doc: Adding link for RabbitMQ installation during nova deployment on controller node. https://review.opendev.org/c/openstack/nova/+/938702 | |
| 10:02:52 | opendevreview | Dmitriy Chubinidze proposed openstack/nova master: Enabling of using LibvirtDriver for compute node. https://review.opendev.org/c/openstack/nova/+/939325 | |
| 10:14:20 | opendevreview | Dmitriy Chubinidze proposed openstack/nova master: doc: Enabling of using LibvirtDriver for compute node. https://review.opendev.org/c/openstack/nova/+/939325 | |
| 10:21:32 | opendevreview | Dmitriy Chubinidze proposed openstack/nova master: doc: Enabling of using LibvirtDriver for compute node. https://review.opendev.org/c/openstack/nova/+/939325 | |
| 14:47:16 | opendevreview | Mengyang Zhang proposed openstack/nova-specs master: Add Burst Length Support to Cinder QoS https://review.opendev.org/c/openstack/nova-specs/+/932653 | |
| 15:00:24 | MengyangZhang[m] | ||
| 15:08:19 | opendevreview | Takashi Kajinami proposed openstack/nova master: Add Burst Length Support to Cinder QoS https://review.opendev.org/c/openstack/nova/+/939310 | |
| 15:43:55 | sean-k-mooney | JayF: im actully questioning if the oslo.log patch did actully fix the event let issue or not. | |
| 15:45:05 | sean-k-mooney | i swaped form my debuggin patch to the tip of master because i wanted to do a rebase and if i restart the service instead of getting the blockign call in the first 3-5 second like i was reliablly before it s been running for 5 mins witn no issue | |
| 15:46:22 | JayF | Given the nature of the eventlet change, I wonder more if it's "existing bug that's coming to the surface more easily" vs it being a direct eventlet issue | |
| 15:46:35 | JayF | I wonder if you have any pipemutex-shaped constructs in nova that might need some patching | |
| 15:47:07 | JayF | if you can isolate the case, you can see if you can get itamarst in #openstack-eventlet-removal to look if you get stuck :) | |
| 15:47:08 | sean-k-mooney | oh the thing im debuging is in watcher | |
| 15:47:10 | sean-k-mooney | not nova | |
| 15:47:12 | JayF | aha | |
| 15:47:39 | JayF | either way, same comment applies, even though TBH our shared downstream doesn't use watcher :D | |
| 15:48:29 | sean-k-mooney | well the thing im trying to debug is actuly related to oslo.db and sqlachemy, eventlet complains that you shoudl not make blocking calls on the main tread | |
| 15:48:55 | sean-k-mooney | the blockign call is time.sleep(0) form oslo.db _thread_yeild function | |
| 15:49:49 | sean-k-mooney | so this https://github.com/openstack/oslo.db/blob/21c484ffb26ed03cbb1ca9dd7fd27fb19a1d4b36/oslo_db/sqlalchemy/engines.py#L43-L52 | |
| 15:51:10 | JayF | I wonder if that should be something more like "if eventlet is imported; sleep(0)" | |
| 15:51:24 | JayF | time.sleep(0) unpatched technically is a blocking call, yeah? | |
| 15:51:25 | sean-k-mooney | https://paste.opendev.org/show/bGPgfURx1cZYOsgmtDyw/ | |
| 15:52:00 | JayF | I'm really, really curious if changing that to an eventlet.sleep(0) in your test fixes that | |
| 15:52:04 | sean-k-mooney | that is the error but it iss patched as far as i could tell | |
| 15:53:11 | JayF | sean-k-mooney: where does the patch happen? | |
| 15:53:14 | sean-k-mooney | JayF: this is where the runtim error is comign form https://github.com/eventlet/eventlet/blob/3450e6adb017f4cd47d1c4e06331c9fa26a986d1/eventlet/greenthread.py#L35 | |
| 15:53:23 | JayF | sean-k-mooney: I'm looking for it, I see no calls to https://github.com/openstack/watcher/blob/master/watcher/eventlet.py | |
| 15:53:58 | JayF | hmm, yeah, this is patching late | |
| 15:54:08 | JayF | I suspect this is like, half eventlet patched | |
| 15:54:09 | sean-k-mooney | https://github.com/openstack/watcher/blob/master/watcher/cmd/__init__.py#L29 | |
| 15:54:23 | sean-k-mooney | JayF: its not patching late | |
| 15:54:26 | JayF | is watcher wsgi? | |
| 15:54:37 | sean-k-mooney | only the api | |
| 15:54:41 | sean-k-mooney | the rest is not | |
| 15:54:50 | JayF | hmm | |
| 15:55:23 | sean-k-mooney | anyway checkign out master and restarting everything with the new eventlest release im not seeign the problem anymore | |
| 15:55:40 | sean-k-mooney | im going to check the latest ci run and see if we see the error in the logs | |
| 15:55:56 | JayF | I am always happy to be a sounding board for eventlet stuff, but at some of this level I can't keep it all straight | |
| 15:56:24 | sean-k-mooney | ah dambit | |
| 15:56:39 | sean-k-mooney | it just triggred in my debug console | |
| 15:57:26 | JayF | is it possible this is whatever the opposite of a hesenbug is? the debug console itself causing the headache? | |
| 15:57:29 | sean-k-mooney | this is why its so annoying to debug. its been fine without the issue for the 5-10 mins we have been chatting | |
| 15:57:43 | sean-k-mooney | we are seeing it in the gate too | |
| 15:58:04 | sean-k-mooney | i did a partial fix patch that was enough to make the tempest jobs pass | |
| 15:58:19 | sean-k-mooney | then then i was inpsectign the logs and notice that we still had those runtime errors | |
| 15:58:52 | sean-k-mooney | so i think my orgianl "fix" just chagned the timing and made it less problematic | |
| 16:00:50 | JayF | I'm really having trouble wrapping my head around eventlet code | |
| 16:00:54 | JayF | that change you linked | |
| 16:01:02 | JayF | eventlet.sleep() is everywhere | |
| 16:01:16 | JayF | and making it so we only call that from coroutines (if I read the code right) seems absolutely brutal | |
| 16:01:28 | JayF | which is why I assumed at first that it must be somehow unpatched | |
| 16:01:44 | sean-k-mooney | right we document if you ever do something long running add eventlet.sleep(0) | |
| 16:01:48 | sean-k-mooney | to yeild | |
| 16:02:19 | sean-k-mooney | or time.sleep if you want to rely on the monkey patching | |
| 16:02:51 | JayF | maybe it's worth asking about that pattern in light of recent code changes in an eventlet issue? | |
| 16:03:26 | sean-k-mooney | oh i never looked at the history of this | |
| 16:03:28 | sean-k-mooney | https://github.com/eventlet/eventlet/commit/48c259c98edb1eb671afeda9800ca4db17b3ccbf | |
| 16:03:31 | JayF | if this is new in this eventlet release, it implies maybe there's a misalignment between how we use it and how they think we use it | |
| 16:03:36 | sean-k-mooney | it was maide a runtime error not an assert | |
| 16:04:00 | JayF | so it still would've failed, yes? | |