Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-04-25
16:40:38 imacdonn somewhat of a tangent, I suppose ... but I think we need to figure out what to do about the eventlet monkey_patching vs. WSGI issue ... it's blocking me from upgrading to Stein
16:42:54 melwitt afaik there's not a clear thing we should do to address the problem, or did I miss it? eventlet monkey patching is not something I understand well
16:44:35 imacdonn same here
16:45:19 imacdonn the nova docs rather strongly suggest moving to WSGI, but maybe my only option is to go back to running nova-api the old way :/
16:46:22 imacdonn alternative interim may be to hack the monkey-patching out, but I don't understand the implications of that well enough ...
16:46:42 dansmith yeah I don't think you can just remove it
16:47:00 imacdonn I don't have it in my Rocky deployments, though
16:47:20 dansmith don't have what? the problem or the monkeypatching?
16:47:25 imacdonn both ;)
16:48:11 dansmith hrm, I'm trying to think about what the api service does that might continue to work without being patched
16:48:36 dansmith if we use all the regular primitives, then it might just work with regular threading, but I expect there are some subtle behavior assumptions
16:48:38 imacdonn it was added here: https://github.com/openstack/nova/commit/23ba1c690652832c655d57476630f02c268c87ae
16:48:40 dansmith locks we don't hold
16:48:45 melwitt I have a patch series that is going on the route to removing eventlet dependency for nova-api https://review.opendev.org/650172 but it needs a follow up patch that makes the executor type configurable (or outright changes it away from green threads)
16:49:32 dansmith imacdonn: right, but it was added because we didn't realize that we weren't patching when the wsgi stuff was split out from service originally, IIRC
16:49:55 melwitt I'm not sure if there are other reasons aside from scatter-gather that eventlet is a dependency in nova-api
16:50:20 imacdonn dansmith: right, and presumably someone decided that it was, infact, needed, with WSGI
16:50:48 imacdonn dansmith: there's mention of multiple cells (which I don't have (other than cell0)) ... but dunno if there are other cases where it's "needed"
16:50:56 dansmith melwitt: that'd be the big one, but any other spawn we do in some layer that can be called from api and other services
16:51:01 dansmith imacdonn: right but even with cell0 you're affected by that
16:51:21 coreycb melwitt: mriedem: added my +1
16:52:04 dansmith I've been a bit disconnected.. is the problem that seems to be introduced the amq heartbeat thing?
16:53:04 mriedem imacdonn: i thought https://review.opendev.org/#/c/647310/ was in response to https://github.com/openstack/nova/commit/23ba1c690652832c655d57476630f02c268c87ae in a way, but you said that fix doesn't help you right?
16:54:32 imacdonn mriedem: that just changes where monkey-patching gets inserted ... I have my problem either way
16:57:01 mriedem i guess you could try exporting OS_NOVA_DISABLE_EVENTLET_PATCHING=1 but that's likely not something you want to run with
16:59:35 imacdonn yeah
16:59:39 mriedem dansmith: there are only 4 places we use the long_rpc_timeout heartbeat stuff and they'd all be calls from either conductor or compute neither of which are wsgi so i'm not sure how that would be involved, unless it was the moving of the monkey patch location in https://github.com/openstack/nova/commit/23ba1c690652832c655d57476630f02c268c87ae
17:00:25 dansmith mriedem: I think the long rpc stuff all uses regular threading primitives so it should work regardless of if you're patched or not right?
17:00:45 mriedem idk, i've never looked at the oslo.messaging implementation
17:00:53 dansmith mriedem: do you have reason to believe the the long rpc stuff is related, or are you just looking for related things?
17:01:06 mriedem imacdonn: have you tried putting melwitt's series https://review.opendev.org/#/q/topic:cell-scatter-gather-futurist+(status:open+OR+status:merged) into rocky and see if that fixes your issue?
17:01:31 dansmith mriedem: that patch doesn't actually change anything yet, IIUC
17:01:34 mriedem dansmith: just in response to "(11:52:08 AM) dansmith: I've been a bit disconnected.. is the problem that seems to be introduced the amq heartbeat thing?"
17:01:40 dansmith it just wraps use of the eventlet stuff so we can do it differentyly
17:01:47 melwitt that's still using eventlet, you'd have to change the futurist executor to the native one
17:01:58 dansmith yeah ^
17:02:29 dansmith mriedem: I think you're confusing heartbeats no?
17:02:58 dansmith mriedem: the heartbeat that long_rpc added is layer 7, unrelated to AMQ heartbeats, which I thought was the subject of that thread
17:03:18 mriedem oh i guess i am then
17:03:28 mriedem i'll step out of this mess (this is why i'm not involved)
17:04:07 dansmith mriedem: yeah, re-reading, they're getting a connection timeout because AMQP heartbeats aren't being sent (a different thing)
17:04:20 dansmith the theory is that eventlet patching prevents some thread from running that sends those
17:04:25 dansmith totes unrelated to long_rpc
17:04:40 imacdonn yes, that's how it appears
17:13:26 openstackgerrit melanie witt proposed openstack/nova master: nova-manage db archive_deleted_rows is not multi-cell aware https://review.opendev.org/507486
17:21:04 openstackgerrit Adam Spiers proposed openstack/os-traits master: Update SEV trait docs to avoid misleading people https://review.opendev.org/655671
17:38:57 dansmith melwitt: the Future object makes it sound like cancel() might not always be supported.. have you confirmed?
17:39:53 melwitt dansmith: yeah, was just writing that on the review. I think it only does something for eventlet GreenThreadPoolExecutor. been looking around in the source to confirm because it doesn't seem like the docs are explicit about which types support cancel()
17:39:55 dansmith melwitt: "If the call is currently being executed and cannot be cancelled then the method will return"
17:40:31 dansmith I think since it's really just a wrapper around a queue, it's just going to cancel if the thing hasn't been dequeued yet
17:40:37 melwitt yeah, cancel() returns True/False and for native threads I expect it just always returns False
17:40:37 dansmith might not even kill a thread in process, I dunno
17:40:40 melwitt right
17:42:04 melwitt yeah, I don't see futurist doing anything different for cancel() other than passing it through to the primitives
17:43:02 dansmith yeah
17:43:09 melwitt so it would be a matter of what does the Future primitive do for cancel() for the different types of executors
17:43:20 melwitt not really seeing it documented on https://docs.python.org/3/library/concurrent.futures.html#future-objects
17:43:25 dansmith melwitt: just to be clear, when I say the primitives, I'm talking about the threading library not the concurrent stuff
17:43:32 melwitt oh, ok. my bad
17:43:37 dansmith the latter is the high-level implementation, which may very well not be in py2
17:44:09 dansmith concurrent (and c.futures) just use the low-level primitives in threading, I imagine
17:44:17 melwitt yeah, it's not but it's all included in a separate futures package containing backports https://pypi.org/project/futures/
17:44:19 dansmith which is why I said what I did
17:44:25 dansmith aye
17:44:43 melwitt yeah, I imagine the same, using same low-level primitives underneath
17:51:18 openstackgerrit Artom Lifshitz proposed openstack/nova master: Revert "Wait for network-vif-plugged on resize revert" https://review.opendev.org/639396
17:51:18 openstackgerrit Artom Lifshitz proposed openstack/nova master: Run revert resize tests in nova-live-migration https://review.opendev.org/653498
17:51:19 openstackgerrit Artom Lifshitz proposed openstack/nova master: Revert resize: wait for external events in compute manager https://review.opendev.org/644881
17:51:59 openstackgerrit Merged openstack/os-vif master: Remove IP proxy methods https://review.opendev.org/655695
17:57:27 mriedem the cancel() thing came up in kevin's changes for canceling queued live migrations,
17:57:33 mriedem there is probably something in the compute manager code around that
17:58:54 mriedem oh right cancel() just returns false if it's already executing and you can't stop it
17:59:04 melwitt yeah, so far I'm not seeing in the futurist source that it event does a green thread kill() when cancel() is called with the GreenThreadPoolExecutor
17:59:09 melwitt *even
17:59:20 melwitt I can't tell that cancel() ever does anything
18:03:36 dansmith ...because it's just a wrapper around a work queue :)
18:17:03 melwitt here's what cancel() does in concurrent.futures https://github.com/python/cpython/blob/d7befad328ad1a6d1f812be2bf154c1cd1e01fbc/Lib/concurrent/futures/_base.py#L352
18:18:22 dansmith "I'm guessing that this cancel is actually a condition variable"
18:18:39 dansmith some dude said ^
18:31:42 melwitt dansmith: you.... win!
18:36:46 melwitt I like futures, this is fun
18:40:24 openstackgerrit melanie witt proposed openstack/nova master: Use futurist.ThreadPoolExecutor in scatter_gather_cells https://review.opendev.org/650172
18:44:30 cooper6581 mriedem: Quick follow up for the { in the password for the transport_url yesterday during our upgrade. I don't know enough if this is a bug or a documentation issue, but I wrote a quick test that reproduces the issue - https://gist.github.com/cooper6581/467f982a0a44494ca32d0c2f755112bc
18:45:16 efried gibi: FYI I copied the berlin onboarding deck and updated it for denver/train https://docs.google.com/presentation/d/1W81DsZGG_bqns2JEdSKwqPCLdAQlR5gTtss-kiGT7Pk/edit?usp=sharing
18:45:43 cooper6581 Specifically, I don't know if we should have ever gotten into a state where transport_url was set, and the config had a non templated url. Maybe it could have been an error on our side during the upgrade v0v
18:46:56 melwitt lyarwood, mriedem: it's alive... http://grafana.openstack.org/d/-iKINcImz/ceph-failure-rate?orgId=1 (and not tracking tempest-full as nicely as we would hope)
18:47:38 mriedem cooper6581: hmm, i'm not sure, that would require some brain power for sorting out the templated stuff which i don't have loaded up right now, dansmith might know off hand though
18:47:52 mriedem i have to re-learn how the templating code each time i look at it b/c i'm slow
18:47:59 mriedem *code works
18:48:15 mriedem melwitt: that's just 6 hours though
18:48:17 melwitt I'm gonna propose a new change to make the graphs cover more time (make look more like http://grafana.openstack.org/d/Hj5IHcSmz/neutron-failure-rate?orgId=1) and add more projects and stable branches
18:48:18 mriedem http://grafana.openstack.org/d/-iKINcImz/ceph-failure-rate?orgId=1&fullscreen&panelId=2&from=now-30d&to=now
18:48:30 mriedem melwitt: you can toggle that in the dashboard
18:48:35 melwitt oh cool, I didn't know that
18:48:41 mriedem top right
18:48:45 melwitt oh I see it now
18:48:48 melwitt niiiiiice
18:48:53 mriedem looks like the neutron one is default 7 days
18:49:17 mriedem i'm more interested in why the nova graph doesn't go beyond .... oh
18:49:19 mriedem opendev rename

Earlier   Later