On Fri, 2019-03-15 at 18:53 +0000, Chris Dent wrote:
On Fri, 15 Mar 2019, Ben Nemec wrote:
According to [0], the monkey-patching wasn't done in nova/__init__.py because it was supposed to be able to run without eventlet under WSGI. If that's not the case, then it's probably fine to do it there.
0: http://specs.openstack.org/openstack/openstack-specs/specs/eventlet-best-pra...
"Monkey patching should also be done in a way that allows services to run without it, such as when an API service runs under Apache. This is the reason for Nova not simply monkey patching in nova/__init__.py."
Yeah, I think several people have run through the eventlet initialization at various times, including me, with various results, none of them quite good enough.
The mod_wsgi/uwsgi side of things strived to be eventlet free as it makes for weirdness, and at some point I did some work to be sure it never showed up in placement [1] while it was still in nova. A side effect of that work was that it also didn't need to show up in the nova-api unless you used the cmd line script version of it. At the same time I also explored not importing the world, and was able to get some improvements (mostly by moving things out of __init__.py in packages that had deeply nested members) but not as much as would have liked.
However, we later (as mentioned elsewhere in the thread) made getting cell mappings parallelize, bring back the need for eventlet, it seems. for what its worth i think reintoducing events was unitentional. we did inentionally make listing instances in cells use eventlets but i dont explictly talking about the fact that that would mean we could no longer run nova api without it.
its rather late in the cycle to change the cell parallelization code but i think we should for revisit this in train.
TL;DR I want to move monkey patching to nova/__init__.py because, for now at least, we always require it anyway. When we eventually remove the requirement for wsgi services to monkey patch we can move monkey patching back to nova/cmd/__init__.py.
This sounds fine to me, but it's been a loooong time since I looked at this and I'm not familiar with some of the more recent changes you talked about, so take my opinion with a grain of salt.
This does seem like the expedient solution (assuming it works), but if we continue to use eventlet anywhere in the api service it will continue to introduce difficulties and inflexibility with deployment choices as it will interact poorly with at least some of the web-servers that could be chosen to run the nova API.
[1] Recently we managed to get placement to a state where not only does it not use eventlet, it doesn't even use anything that has it as an unused dependency. This made me very happy.
yep i would like to vote for restoring the nova apis ability to run without eventlet too. i know its unrealistic to transition away from eventlet entirly while we still support python 2 but i would like to avoid useing eventlets where it is not needed. we currently have a test only depency on eventlet that i hope to remove form os-vif in train but one of my personal goals in U or if we proceed with a unified agent as a comunity goal would be to remove the use of eventlets entirely. this has come up in the past so im not going to rathole on it but when we move to python3 only i how we can reassess as a comunity if we can move to python3 native asyncio and remove the need for monkey patching entirly. the current design of our agent code makes that challanging but the api is far cleaner and should not require eventlet at least in principal.