opendevreview | Xiang Wang proposed openstack/oslo.messaging master: Implement ConsulOperator as a service broker to support HTTP Driver https://review.opendev.org/c/openstack/oslo.messaging/+/912499 | 01:08 |
---|---|---|
opendevreview | Vasyl Saienko proposed openstack/oslo.log master: Fix eventlet detection https://review.opendev.org/c/openstack/oslo.log/+/914190 | 10:38 |
opendevreview | Vasyl Saienko proposed openstack/oslo.log master: Fix eventlet detection https://review.opendev.org/c/openstack/oslo.log/+/914190 | 10:49 |
opendevreview | Vasyl Saienko proposed openstack/oslo.log master: Fix eventlet detection https://review.opendev.org/c/openstack/oslo.log/+/914190 | 12:02 |
hberaud | hey oslo folks, for your information, we recently observed failures related to a manual monkey patch of the stdlib threading module, made by oslo.log https://github.com/eventlet/eventlet/issues/948 surely related to https://opendev.org/openstack/oslo.log/commit/94b9dc32ec1f52a582adbd97fe2847f7c87d6c17. Context is available in the github link | 12:45 |
hberaud | Vasyl is patch ^ seems related to the observed failure too | 13:23 |
frickler | ah, that looks similar to what sean-k-mooney saw in devstack, nice | 13:30 |
sean-k-mooney | so what was concerning about the devstack run | 13:31 |
sean-k-mooney | was that keystone does not use eventlet | 13:32 |
sean-k-mooney | and seting the env var for teh ascyio hub caused issues with eventlet | 13:32 |
sean-k-mooney | so defining EVENTLET_HUB seam to both enabled eventlet to some degree and set the default hub to use | 13:33 |
sean-k-mooney | im not sure if that i intentional but it seams like an eventlet bug to me | 13:33 |
sean-k-mooney | oh | 13:34 |
sean-k-mooney | actully reading that patch | 13:34 |
sean-k-mooney | ya oslo is broken | 13:34 |
sean-k-mooney | eventlet = sys.modules.get('eventlet') | 13:34 |
sean-k-mooney | if eventlet: | 13:34 |
sean-k-mooney | that just tells you that eventlet is installed but it does not check if its in use in the current process | 13:35 |
sean-k-mooney | let me see if i can hack something quickly to chekc if eventlet monkey patching is actully in use | 13:36 |
hberaud | sean-k-mooney: see my comments on https://github.com/eventlet/eventlet/issues/948 | 13:41 |
hberaud | I think the condition based on the result of `sys.module.get('eventlet')` is a bad thing and trigger the patching of the threading module. Patched threading module who broke the asyncio hub. | 13:43 |
opendevreview | sean mooney proposed openstack/oslo.log master: only patch threading if monkey patched https://review.opendev.org/c/openstack/oslo.log/+/914246 | 13:44 |
sean-k-mooney | i think ^ would fix it right | 13:44 |
hberaud | this condition should be based on if we already patched the env, and not if the module is available in sys moduke (else importing eventlet will make appear eventlet in sys.module) | 13:45 |
sean-k-mooney | right so thats what i did | 13:45 |
sean-k-mooney | import eventlet.patcher | 13:45 |
sean-k-mooney | if not eventlet.patcher.is_monkey_patched('thread'): | 13:45 |
sean-k-mooney | return | 13:45 |
sean-k-mooney | i kept that code in the orgianal if as eventlet may not be isntalled if this is deployed in a container | 13:46 |
hberaud | and eventlet 0.36.0 already contains patch to ensure that the asyncio hub do not use a patched threading module https://github.com/eventlet/eventlet/commit/520c6e243774aeb9a80c712e1eacfb5957d472d0 | 13:46 |
hberaud | unfortunately eventlet in 0.35.1 do not contains that fix | 13:47 |
opendevreview | Vasyl Saienko proposed openstack/oslo.log master: Fix eventlet detection https://review.opendev.org/c/openstack/oslo.log/+/914190 | 13:47 |
hberaud | stars were not aligned | 13:48 |
sean-k-mooney | so https://review.opendev.org/c/openstack/devstack/+/914108 should be running with eventlet 0.36.0 by the way | 13:49 |
hberaud | sean-k-mooney: I think your patch is duplicated with Vasyl his one. Also I'd suggest to use eventletutils as I suggested early today to Vasyl https://review.opendev.org/c/openstack/oslo.log/+/914190 | 13:50 |
sean-k-mooney | yep https://zuul.opendev.org/t/openstack/build/29ba9328076440f997f6096cdabc0045/log/job-output.txt#7018 | 13:51 |
sean-k-mooney | yep so ill abandon my oslo patch | 13:51 |
sean-k-mooney | and then ill make my devstack patch depend on Vasyl's patch | 13:51 |
hberaud | cool, thanks | 13:51 |
sean-k-mooney | ill also disbale some of the zuul jobs to waste less resoucs untill this work | 13:51 |
hberaud | yeah sure | 13:52 |
sean-k-mooney | ok we can check back on https://review.opendev.org/c/openstack/devstack/+/914108 in an hour if its still running it likely got past the inital failure in oslo.log | 13:57 |
sean-k-mooney | otherwise we would expect it to fail in 25 mins or so | 13:58 |
hberaud | ack | 13:59 |
sean-k-mooney | hum this time the bootstrap commadn seams to run but we hit another atribute error in keystone | 14:18 |
sean-k-mooney | https://zuul.opendev.org/t/openstack/build/b56da78c8f974d8c9d683a191a6c2265/log/job-output.txt#13020 | 14:19 |
sean-k-mooney | that however does not look like its related to olo or evnetlet | 14:19 |
hberaud | indeed, seems related to an other problem | 14:20 |
sean-k-mooney | there is a second devstack job in the buildset | 14:21 |
sean-k-mooney | https://zuul.opendev.org/t/openstack/stream/148ef12138f24e7086475fe55e9d8ce6?logfile=console.log | 14:21 |
hberaud | https://github.com/pyca/bcrypt/issues/684 | 14:21 |
sean-k-mooney | and that is currently installing neutron | 14:21 |
sean-k-mooney | so lets wait and see | 14:21 |
sean-k-mooney | we might need to pin that buggy version in uc and one of the jobs just got unlucky | 14:22 |
hberaud | passlib is mostly abandoned and should be removed at somepoint | 14:22 |
hberaud | but that's an other topic | 14:22 |
sean-k-mooney | well it has not had an update since october 2020 | 14:23 |
sean-k-mooney | so i assume they are not going to do a quick release for the bcrypt change | 14:23 |
sean-k-mooney | so unless bcrypt fix the issue we might be force too do something about htat | 14:23 |
hberaud | yep | 14:23 |
sean-k-mooney | https://github.com/pyca/bcrypt/issues/684#issuecomment-1902590553 we proably need to copy that | 14:25 |
hberaud | yes | 14:27 |
sean-k-mooney | so it looks like keystone is the only thing that imports it and it does not use it for much | 14:27 |
sean-k-mooney | actully trove improts it to | 14:28 |
hberaud | AFAIK the maintenance activity on keystone seems a quite low, so we may find other similar issues related to outdated things | 14:29 |
sean-k-mooney | ya so im current lookign at this rabbithole. i dont really have time to fix all the thigns but ill see what i can do in 30 mins | 14:30 |
hberaud | tkajinam, damani: please can you review Vasyl's patch? https://review.opendev.org/c/openstack/oslo.log/+/914190 | 14:32 |
hberaud | (see the discussion above) | 14:33 |
sean-k-mooney | im wokring on a patch to remove passlib form trove now | 14:40 |
sean-k-mooney | that looks trivial | 14:40 |
sean-k-mooney | keystone will likely take longer then i have right now | 14:40 |
sean-k-mooney | but proably not that much longer | 14:41 |
hberaud | ack | 14:41 |
hberaud | thank you sean-k-mooney for all your great works! | 14:41 |
sean-k-mooney | im on pto for the next 10 days starting thurday so i dont really have time to start anythign new | 14:41 |
sean-k-mooney | so hacking on small things like this is proably as useful as anything else | 14:42 |
hberaud | np | 14:42 |
hberaud | yes | 14:42 |
hberaud | I proposed a patch to bump U.C to the latest version of eventlet https://review.opendev.org/c/openstack/requirements/+/914256 | 14:46 |
tkajinam | hberaud, +2+A-ed that patch. probably we should close https://bugs.launchpad.net/oslo.messaging/+bug/2043661 and see if that oslo.log fix works | 14:46 |
hberaud | indeed | 14:47 |
sean-k-mooney | https://review.opendev.org/c/openstack/trove/+/914257 i think is all that is needed | 14:47 |
sean-k-mooney | unit tests pass locally at least | 14:47 |
hberaud | we should also backport this patch to stable branches that contains the original code | 14:47 |
hberaud | tkajinam: ^ | 14:47 |
tkajinam | yeah | 14:48 |
hberaud | damani recently started to backport related parent patches | 14:48 |
hberaud | damani: do you want to handle these backports? | 14:48 |
tkajinam | I was looking into oslo.messaging after that bug was reported but I didn't find why we haven't seen this problem until recent versions but not it makes very clear sense | 14:48 |
hberaud | (the new ones) | 14:48 |
tkajinam | probably we can use closes-bug instead | 14:49 |
sean-k-mooney | im pretty sure we have that backported downstream for redhat's osp product too so if we can do the backport upstream that helps everyone out | 14:49 |
opendevreview | Takashi Kajinami proposed openstack/oslo.log master: Fix eventlet detection https://review.opendev.org/c/openstack/oslo.log/+/914190 | 14:50 |
tkajinam | I can take care of it. | 14:50 |
tkajinam | hberaud, I've replaced related-bug tag by closes-bug tag because we consider this is the actual fix. I've already added my +2+A but it'd be nice if you can re-add your +2. | 14:52 |
hberaud | sure | 14:54 |
hberaud | tkajinam: don't you want to also link the oslo.messaging but in the commit message? | 14:55 |
hberaud | we could kill two birds with one stone | 14:56 |
tkajinam | hberaud, I don't need it now, because I've closed that oslo.messaging bug as duplicate of bug 2039346 | 14:57 |
hberaud | indeed it also works :) | 14:57 |
damani | hberaud, i can do it | 14:59 |
hberaud | damani: cool, I let you manage that point with Takashi | 14:59 |
hberaud | thx | 14:59 |
opendevreview | Daniel Bengtsson proposed openstack/oslo.log stable/2024.1: Fix eventlet detection https://review.opendev.org/c/openstack/oslo.log/+/914262 | 15:14 |
tkajinam | hberaud, do you think we have to create a release with this fix before 2024.1 GA or can we create one after GA ? I'm unsure if this is causing problems frequently enough to be considered as a critical bug | 15:22 |
tkajinam | I know that this is problematic, though. | 15:23 |
opendevreview | Daniel Bengtsson proposed openstack/oslo.log stable/2023.2: Fix eventlet detection https://review.opendev.org/c/openstack/oslo.log/+/914266 | 15:33 |
opendevreview | Daniel Bengtsson proposed openstack/oslo.log stable/2023.1: Fix eventlet detection https://review.opendev.org/c/openstack/oslo.log/+/914267 | 15:35 |
hberaud | creating a release ASAP would be better, though, I don't know the criticity of this bug too. IMO, this bug is more or less isolated in a special kind of env for now... but.. I wouldn't bet too much on it either. | 15:35 |
hberaud | tkajinam: ^ | 15:36 |
JayF | Seems to me like it causes zero harm, and eliminates an unknown-unknown. I'd likely want the fix in a stable release as long as it doesn't create a hardship. | 15:36 |
JayF | IMO I doubt it makes a significant difference if that is pre-or-post 2024.1 release actually happening :) | 15:37 |
hberaud | sean-k-mooney: the same failure appear again in the latest grenade job run https://zuul.opendev.org/t/openstack/build/b93dcee5237e4fc2aaeade08b41d2def/log/job-output.txt#11862. Even with your latest PS changes. Does I missed something? | 15:40 |
hberaud | (just to be sure) | 15:40 |
hberaud | and same thing on the devstack job | 15:41 |
tkajinam | let's see how quickly we can merge the fix... we have no concern about backport. my main concern is that we are quite close to GA and I'm a bit hesitant to rush to cover a new release and also req bump at this timing. | 16:05 |
sean-k-mooney | the devstack job hit the bcrypt issue | 16:05 |
sean-k-mooney | https://zuul.opendev.org/t/openstack/build/148ef12138f24e7086475fe55e9d8ce6/log/job-output.txt#12649 | 16:05 |
sean-k-mooney | so thats not the oslo log issue | 16:06 |
tkajinam | hberaud, I'm unsure what's your expectation about 2024.2 ? we don't have stable/2024.2 branch yet, apparently | 16:06 |
opendevreview | Takashi Kajinami proposed openstack/oslo.log stable/2023.1: Fix eventlet detection https://review.opendev.org/c/openstack/oslo.log/+/914267 | 16:10 |
hberaud | tkajinam: do you speak about => https://review.opendev.org/c/openstack/oslo.log/+/914266 ? | 16:53 |
hberaud | the stable branch seems to have been created https://review.opendev.org/q/project:openstack/oslo.log+branch:stable/2023.2 | 16:54 |
hberaud | and my comment on Daniel's patch is about the fact that the commit message refer to 2 "cherry-pick" when this patch is the first one of the list, so I wonder why we have 2 cherry-pick listed there | 16:56 |
hberaud | sean-k-mooney: yes, but, if I'm right we apparently still obseve the same issue with oslo.log, but those seems ignored (Exception ignored in: <function _removeHandlerRef at 0x7f07a17a4040>) https://zuul.opendev.org/t/openstack/build/148ef12138f24e7086475fe55e9d8ce6/log/job-output.txt#12345 | 16:59 |
hberaud | sean-k-mooney: sorry, that's not exactly the same error, but it looks like it. Eventlet seems not involved | 17:02 |
sean-k-mooney | ah ok its what i tought it was | 17:08 |
sean-k-mooney | LIBS_FROM_GIT=cinder,devstack,glance,keystone,neutron,nova,placement,requirements,swift | 17:08 |
sean-k-mooney | hberaud: os oslo.log is not in requried projects for that job so while zuul is cloneing it devstack is not using it | 17:09 |
hberaud | ok | 17:20 |
opendevreview | Merged openstack/oslo.log master: Fix eventlet detection https://review.opendev.org/c/openstack/oslo.log/+/914190 | 18:27 |
sean-k-mooney | hberaud: this time https://review.opendev.org/c/openstack/devstack/+/914108 to running swift | 19:59 |
sean-k-mooney | so improvement but still proably a long way off | 19:59 |
sean-k-mooney | hberaud: https://zuul.opendev.org/t/openstack/build/f29ddb7b451b425db9f60cc1d5940c96/log/controller/logs/screen-s-account.txt#32 | 19:59 |
sean-k-mooney | raise RuntimeError("Multiple readers are not yet supported by asyncio hub") | 20:00 |
sean-k-mooney | hberaud: i could turn off swift for now. it is one of the hevier users of eventlet details | 20:01 |
sean-k-mooney | its still in logging code but i belive swift does not use oslo log | 20:02 |
*** zbitter is now known as zaneb | 23:22 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!