| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-05-24 | |||
| 17:10:20 | sean-k-mooney | osp 10 is what newton i think we just used the nova-api command directly at that point | |
| 17:10:27 | melwitt | but even if we stop monkey patching in nova-api, we will still see this in nova-scheduler and nova-conductor at least | |
| 17:11:00 | sean-k-mooney | is there a cler writeup of how the error happens | |
| 17:11:02 | melwitt | yeah. and in the sosreports I've looked at for 13 it's also the nova-api command in these cases, afaict from the ps output | |
| 17:11:49 | sean-k-mooney | i think ooo avoid using apache initally due to concens of memory overhead | |
| 17:11:49 | melwitt | sean-k-mooney: yeah but not in the context of openstack. this is a private bug but here https://bugzilla.redhat.com/show_bug.cgi?id=1927994#c45 and the links are https://github.com/PyMySQL/PyMySQL/issues/234 https://github.com/sqlalchemy/sqlalchemy/issues/3258 https://github.com/PyMySQL/PyMySQL/issues/260 | |
| 17:11:50 | openstack | melwitt: Error: Error getting bugzilla.redhat.com bug #1927994: NotPermitted | |
| 17:12:28 | sean-k-mooney | ah ok i was looking at the nova bug report and trying to find the repoducer | |
| 17:13:16 | zzzeek | hey just snooping a little bit, I think the main thing nova is doing that nobody else is, is using eventlet monkeypatching *with* mod_wsgi at the same time | |
| 17:13:18 | melwitt | we haven't been able to reproduce it in openstack | |
| 17:13:39 | zzzeek | so that's two frameworks with heavy and opposing opinions on concurrency getting together | |
| 17:13:57 | melwitt | zzzeek: the traces I've been looking at have all been not running under mod_wsgi and also occurred in services (scheduler and conductor) that are not using wsgi in any form | |
| 17:14:08 | zzzeek | ah | |
| 17:14:50 | zzzeek | melwitt: that's odd. pymysql doesnt like if you use eventlet but as long as the scope of a connection is maintained in only one greenlet at a time, this kind of error shouldnt happen. what can happen is if requsts are interrupted and not cleaned up correctly | |
| 17:15:27 | zzzeek | or if cleanup code itself is not able to run correctly due to the monkeypatrching | |
| 17:15:41 | sean-k-mooney | dont we initalise the connection globally and share it between all greentherads | |
| 17:15:49 | melwitt | no we don't | |
| 17:16:24 | melwitt | that's not a "connection" it's a "transaction context manager" which is a factory if I'm remembering terminology zzzeek explained to me last time | |
| 17:16:41 | sean-k-mooney | ah yes that is what i was thinking of | |
| 17:17:00 | sean-k-mooney | the object we recently wraped in teh run once decorator | |
| 17:17:10 | melwitt | and he has confirmed that is the correct way to use it, the threads can share that factory and get new connections from it | |
| 17:17:55 | sean-k-mooney | so when ever we context switch eventlet never guarentes we will resume on the same tread. | |
| 17:18:01 | sean-k-mooney | but normally we have only one | |
| 17:18:02 | melwitt | yeah the configure() of those objects | |
| 17:18:15 | sean-k-mooney | could this be related to the use of pthread for the heartbeat | |
| 17:18:52 | melwitt | which heartbeat? the service heartbeats are eventlet, that I saw | |
| 17:19:15 | sean-k-mooney | the only real pthread i know of in nova are teh oslo.messaging heartbeat and the libvirt one | |
| 17:19:28 | sean-k-mooney | although no that would not make sense fo 10/13 | |
| 17:20:03 | dansmith | at one point we changed the ordering of our imports relative to the monkeypatching to "fix" something | |
| 17:20:14 | melwitt | zzzeek: yeah... dansmith pointed out that glance and cinder use eventlet and monkey patch, but yet we don't see this error from them | |
| 17:20:15 | dansmith | and I think that got backported.. I wonder if that's relevant? | |
| 17:21:00 | sean-k-mooney | melwitt: i was refering to https://github.com/openstack/oslo.messaging/blob/5aa645b38b4c1cf08b00e687eb6c7c4b8a0211fc/oslo_messaging/_drivers/impl_rabbit.py#L90-L100 | |
| 17:21:14 | melwitt | * melwitt looks for link | |
| 17:21:31 | sean-k-mooney | dansmith: mdboots change | |
| 17:21:39 | dansmith | sean-k-mooney: right | |
| 17:21:52 | melwitt | I was just looking at that earlier | |
| 17:21:59 | dansmith | sean-k-mooney: I wonder if that ended up with us getting a combination of real and green threads in a way that is problematic.. | |
| 17:22:13 | sean-k-mooney | https://github.com/openstack/nova/commit/3c5e2b0e9fac985294a949852bb8c83d4ed77e04#diff-c2e5ad6353633e738ba126e0f11ea14ed3f6ea94554deec967586fd2dfcf060d | |
| 17:22:16 | sean-k-mooney | that one | |
| 17:22:35 | melwitt | yeah that's it | |
| 17:22:58 | melwitt | sean-k-mooney: ack thanks (pthread) | |
| 17:23:16 | sean-k-mooney | dansmith: well in principal that should have moved the patching eairler so less likely to get a mix | |
| 17:23:28 | sean-k-mooney | but you are suggestign without it we still could be | |
| 17:23:31 | sean-k-mooney | if we have not backported it | |
| 17:23:34 | dansmith | well, | |
| 17:23:45 | melwitt | yeah, we did not backport it | |
| 17:23:49 | dansmith | I think in wsgi mode that will come in at the point at which we hit it due to importing that api module | |
| 17:24:15 | dansmith | melwitt: oh I thought we did.. maybe that's related to the sudden cessation of reports? :) | |
| 17:24:29 | melwitt | could be, yeah | |
| 17:24:45 | dansmith | melwitt: did you say you didn't see it at all in later releases, or just ... less? | |
| 17:25:07 | sean-k-mooney | this was merged in train | |
| 17:25:18 | melwitt | dansmith: I could not find any mention of it past queens/13 when I bugzilla searched everything under nova and pymysql | |
| 17:25:22 | sean-k-mooney | so if we did not abckprot we shoudl see it up to 15 | |
| 17:26:00 | dansmith | melwitt: that seems like it could be a strong contender for being related then | |
| 17:26:14 | sean-k-mooney | didnt we also change the mysql clint at one point | |
| 17:26:17 | melwitt | dansmith: yeah, agree | |
| 17:26:21 | dansmith | sean-k-mooney: long ago | |
| 17:26:27 | sean-k-mooney | i think pymsql is the new one right | |
| 17:26:38 | sean-k-mooney | it used to be mysql_python or something | |
| 17:26:43 | dansmith | it is, but that change was like icehouse or something I think | |
| 17:26:57 | sean-k-mooney | ya ok | |
| 17:27:53 | melwitt | I had been looking at it from the context of it also providing a way to disable monkey patching, but due to my lack of understanding of eventlet and mixing with native threads, it did not click for me to think it could have fixed things to monkey patch earlier | |
| 17:28:27 | melwitt | it makes sense when you say it now though.. | |
| 17:28:31 | dansmith | a combination of references to the un-patched library and the patched one could very much be relevant | |
| 17:28:43 | dansmith | and that's what that change was aabout | |
| 17:28:54 | dansmith | and it's also the argument against monkeypatching altogether of course :P | |
| 17:29:42 | sean-k-mooney | its ok stephenfin will reventyly get around to deleteing all the eventlet code like all the ohter stuff he has deleted :) | |
| 17:29:53 | melwitt | yeah. that change was mostly non understandable by my brain | |
| 17:29:56 | sean-k-mooney | but ya we did this for urllib3 eventully | |
| 17:30:00 | sean-k-mooney | *orginally | |
| 17:30:13 | sean-k-mooney | and som eohter service i guess but it makes sesne | |
| 17:30:40 | dansmith | urllib3 being socket-oriented, along with pymysql ... :) | |
| 17:30:58 | sean-k-mooney | we did actully backport this https://review.opendev.org/c/openstack/nova/+/647310 | |
| 17:31:02 | sean-k-mooney | but only to stien | |
| 17:31:19 | melwitt | ah, ok, my bad | |
| 17:31:27 | melwitt | I had thought it landed in stein | |
| 17:31:30 | dansmith | okay I was sure we did backport it some, but .. fair enough | |
| 17:31:32 | melwitt | (originally) | |
| 17:31:59 | sean-k-mooney | i guess that stien is where the orginal bug was reported | |
| 17:32:08 | sean-k-mooney | and we just did not bring it back before that | |
| 17:33:19 | sean-k-mooney | hum https://bugs.launchpad.net/nova/+bug/1808951 | |
| 17:33:21 | openstack | Launchpad bug 1808951 in tripleo "python3 + Fedora + SSL + wsgi nova deployment, nova api returns RecursionError: maximum recursion depth exceeded while calling a Python object" [High,Incomplete] | |
| 17:33:34 | sean-k-mooney | oh i miss read SSL as SQL | |
| 17:33:44 | sean-k-mooney | i was going to say it refrence SQL too | |
| 17:34:16 | melwitt | ok, I think it would be interesting if I build them a test package with that change and see if they can try it out | |
| 17:34:30 | melwitt | that would be good proof that it is/was the fix | |
| 17:34:36 | dansmith | melwitt: yeah if they're willing I think that'd be a good test | |
| 17:35:20 | melwitt | I'll get that done and give them the option | |
| 17:55:04 | sean-k-mooney | melwitt: lyarwood so it look like the new resovled is breaking lowerconstraits on stable os-vif branches | |
| 17:55:16 | sean-k-mooney | how is that adressed for stabel brances | |
| 17:55:29 | sean-k-mooney | do we update to the oldest lib that works? | |
| 17:55:51 | sean-k-mooney | hacking seams to be what is breakign things | |
| 17:56:01 | sean-k-mooney | although there coudl be other issues | |
| 17:57:16 | sean-k-mooney | ok on master stephenfin removed any non direct deps | |
| 17:57:18 | sean-k-mooney | https://github.com/openstack/os-vif/commit/44d8937148aac1a61f40e59d3271c45f9fe6aa03 | |
| 17:57:48 | sean-k-mooney | ill see if i can do somethign similar | |
| 17:58:45 | lyarwood | sean-k-mooney: ack yeah or backport a modified version of that? | |
| 17:59:13 | sean-k-mooney | well that is waht i was going to do but with lower min version to match the branch its going too | |
| 17:59:44 | sean-k-mooney | im not sure if starting from that patch will be faster or not | |
| 18:30:24 | openstackgerrit | sean mooney proposed openstack/os-vif stable/victoria: Resolve dependency issues https://review.opendev.org/c/openstack/os-vif/+/792840 | |