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.

...

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. -- Chris Dent ٩◔̯◔۶ https://anticdent.org/ freenode: cdent tw: @anticdent

On Fri, 2019-03-15 at 12:15 -0700, melanie witt wrote:

On Fri, 15 Mar 2019 18:53:35 +0000 (GMT), Chris Dent <cdent+os@anticdent.org> wrote:

...

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.

Is there an alternative we could use for threading in the API that is compatible with python 2.7? I'd be happy to convert the cells scatter-gather code to use it, if so. its a little heavy weight but python multi process or explicit threads would work.

taking the multi processing example https://docs.python.org/2/library/multiprocessing.html from multiprocessing import Pool def f(x): return x*x if __name__ == '__main__': p = Pool(5) print(p.map(f, [1, 2, 3])) you bassically could create a pool X process and map the function acroos the pool. X could either be a fixed parallelisum factor or the total numa of cells. if f is a function that takes the cell and lists the instances then p.map(f,["cell1","cell2",...]) returns the set of results from each of the concurrent executions. but it also blocks until they are completed. eventlets gives you concurency which means we interleave the requests but only one request is executing at anyone time. using mulitprocess will give real parallelism but no concurance as we will block until all the parralel request are completed. you can kind of get the best of both world by submitting the request asycronously as is showen in the later example https://docs.python.org/2/library/multiprocessing.html#using-a-pool-of-worke... # launching multiple evaluations asynchronously *may* use more processes multiple_results = [pool.apply_async(os.getpid, ()) for i in range(4)] print [res.get(timeout=1) for res in multiple_results] this allows you to submit multiple request to the pool in parallel then retrive them with a timeout after all results are submitted allowing use limit the time we wait if there is a slow or down cell. wsgi also provides an additional layer of parallelism as each instance of the api should be severing only one request and that parallelism is managed by uwsgi or apache if using mod_wsgi. im not sure if multiprocess is warented in this case but if we did use it we should proably create the pool once and reuse it rather then inline in the scater gather function. anyway that would how i would personally aproch this if it was identified as a perfromace issue with a config argument to control the number of processes in the pool but it would definitely be a train thing as its a non tivial change in how this currently works.

-melanie

On 3/15/19 4:41 PM, Monty Taylor wrote:

FWIW, we use explicit threads in Zuul and Nodepool and have been very happy with them- since they're explicit and don't require weird monkeypatching.

In sdk, there are a few things that need to be done in the background (when uploading a very large swift object and we're going to split it in to chunks and upload concurrently) In that case we used concurrent.futures.ThreadPoolExecutor which creates a pool of threads and allows you to submit jobs into it ... very simiar to the using-pool-of-workers example above.

This is what we did in the oslo.privsep daemon too when we added parallel execution.

On Tue, Mar 19, 2019 at 6:57 PM Sean Mooney <smooney@redhat.com> wrote:

On Fri, 2019-03-15 at 17:06 -0500, Ben Nemec wrote:

...

On 3/15/19 4:41 PM, Monty Taylor wrote:

[snip]

...

...

In sdk, there are a few things that need to be done in the background (when uploading a very large swift object and we're going to split it in to chunks and upload concurrently) In that case we used concurrent.futures.ThreadPoolExecutor which creates a pool of threads and allows you to submit jobs into it ... very simiar to the using-pool-of-workers example above.

This is what we did in the oslo.privsep daemon too when we added parallel execution.

yes so on that point i wanted to talk to the oslo team about possibly breaking out the core of privsep into oslo concurancy.

for a lot of the concurancy we need in nova we could create a privsep deamon context with no permisions and simply dispatch the execution of api queries to it but that feels a bit strainge. i was wondering if there was merrit in either packaging up the core of the deamon without the actuall privlage seperation logic or provideing a very thin shim over the concurrent futures. though by the looks of it there is a python2.7 backport already https://pypi.org/project/futures/ i had discounted using concurrent futrues with the ThreadPoolExecutor as i had tought it was python 3 only but yes it provides a nice api and workflow form this type of concurrent or parallel dispatch.

Is there a lot of gain to splitting the concurrency out of privsep? The lack of escalated permissions for a privsep'd function is a very small (one line IIRC) change. I haven't looked at the API for parallel privsep to be honest. How does it handle returning results to callers?

its a topic i would like to discuss with people at the ptg but i would personally be interested in seeing if we can 1 remove where resoanable our explict use of eventlets, and second explore transitioning to a more explitct concurancy model if other were open to it either by delegating execution to an un privalaged privesep deamon or something like concurent futures with a thread pool or asyncio event loop using https://docs.python.org/3/library/asyncio-future.html. my distaste for eventlets has mellowed over the last year or two as debugers have actully began to understand gevent and greenlets but if i had an architetural wand to change one thin in the U cycle it would be breaking our eventlet depency and moving to a functionality form the standard libary instead when we are python 3 only.

One thing I wanted recently was a way to start a thread which ran as a "sidecar" to nova-compute but didn't share anything with nova-compute apart from wanting to always be running when nova-compute was. Think the metadata server or a prometheus metrics exporter. I nearly wrote an implementation with privsep, but didn't get around to it. I feel like an unprivileged privsep parallel executor which never returns might be very close to what I wanted. I wont be at the PTG, but would like to be kept informed of where you get to on this. Michael

On 3/19/19 11:39 PM, Sean Mooney wrote:

...

On Tue, Mar 19, 2019 at 6:57 PM Sean Mooney <smooney@redhat.com> wrote:

...

On Fri, 2019-03-15 at 17:06 -0500, Ben Nemec wrote:

...

On 3/15/19 4:41 PM, Monty Taylor wrote:

[snip]

...

...

...

In sdk, there are a few things that need to be done in the background (when uploading a very large swift object and we're going to split it in to chunks and upload concurrently) In that case we used concurrent.futures.ThreadPoolExecutor which creates a pool of threads and allows you to submit jobs into it ... very simiar to the using-pool-of-workers example above.

This is what we did in the oslo.privsep daemon too when we added parallel execution.

...

yes so on that point i wanted to talk to the oslo team about possibly breaking out the core of privsep into oslo concurancy.

for a lot of the concurancy we need in nova we could create a privsep deamon context with no permisions and simply dispatch the execution of api queries to it but that feels a bit strainge. i was wondering if there was merrit in either packaging up the core of the deamon without the actuall privlage seperation logic or provideing a very thin shim over the concurrent futures. though by the looks of it there is a python2.7 backport already https://pypi.org/project/futures/ i had discounted using concurrent futrues with the ThreadPoolExecutor as i had tought it was python 3 only but yes it provides a nice api and workflow form this type of concurrent or parallel dispatch.

Is there a lot of gain to splitting the concurrency out of privsep? The lack of escalated permissions for a privsep'd function is a very small (one line IIRC) change.

I haven't looked at the API for parallel privsep to be honest. How does it handle returning results to callers?

On Wed, 2019-03-20 at 08:53 +1100, Michael Still wrote: privsep api is entrily blocking form a callers perspective. internally privsep has a small asycronos event loop that does paralel dispatch to a threadpoolexecutor. so while the privsep deamon itelf will execute request in parallel the caller of privsep rely on the eventlet context switch that happens when the priv sep decorator writes to the unix socket which causes the callers green tread to yeild until the result callback is executed by the privsep deamon which wites the result back to the unix socket in either a reply or error message.

while privsep has many of the feature you would want in a parallel sidecar that you could dispatch asyc task to it does not have the api you would want and looking at the implementation a little more closly it proably is not ideal for performace sensivive code. the way we decorate module level function to annotate them as privsep entrypoints means we cannot have class/instanstance methods that can be executed via privsep and while we do have the concept of channels and futures we could do better by creating a dedicated reusable event loop that can be seeded with an executor at creation time. the python comunity has already done this infact in the python 3 standard lib.

we could proably add the concurrent.futures executor interface to privsep if we wanted its only 3 function after all https://docs.python.org/3/library/concurrent.futures.html#concurrent.futures... but since there is a python 2.7 backport we may be better served just using it.

It's also worth pointing out the futurist library, which is part of oslo: https://opendev.org/openstack/futurist It's similar to (and uses) concurrent.futures - except it also has a bunch of different backend impls - so you can have tasks run in threads, or it can use eventlet, or processes - or work synchronously.

privsep is effectivly just a ThreadPoolExecutor that layers on the privlage escalation/descalation logic via either fork and drop privalages or sudo + rootwrap/privsep helper to gain privileges.

...

...

its a topic i would like to discuss with people at the ptg but i would personally be interested in seeing if we can 1 remove where resoanable our explict use of eventlets, and second explore transitioning to a more explitct concurancy model if other were open to it either by delegating execution to an un privalaged privesep deamon or something like concurent futures with a thread pool or asyncio event loop using https://docs.python.org/3/library/asyncio-future.html. my distaste for eventlets has mellowed over the last year or two as debugers have actully began to understand gevent and greenlets but if i had an architetural wand to change one thin in the U cycle it would be breaking our eventlet depency and moving to a functionality form the standard libary instead when we are python 3 only.

One thing I wanted recently was a way to start a thread which ran as a "sidecar" to nova-compute but didn't share anything with nova-compute apart from wanting to always be running when nova-compute was. Think the metadata server or a prometheus metrics exporter. I nearly wrote an implementation with privsep, but didn't get around to it. I feel like an unprivileged privsep parallel executor which never returns might be very close to what I wanted.

For this case, I don't think concurrent.futures or privsep pr futurist is what you want. You just actually want to start a thread directly. http://codesearch.openstack.org/?q=threading.Thread&i=nope&files=&repos=zuul,nodepool has a bunch of examples that may be worth looking at. We've just been using the direct threading.Thread interface and have been very pleased with it at very high scale. https://opendev.org/openstack-infra/nodepool/src/branch/master/nodepool/laun... Is where the main nodepool launcher thread starts the cleanup, deleter and stats worker threads.

...

I wont be at the PTG, but would like to be kept informed of where you get to on this.

I'm sad that you won't be at the PTG. Your face will be missed.

On Wed, 20 Mar 2019 07:39:20 +0000, Sean Mooney <smooney@redhat.com> wrote:

On Wed, 2019-03-20 at 02:22 +0000, Monty Taylor wrote:

...

...

we could proably add the concurrent.futures executor interface to privsep if we wanted its only 3 function after all https://docs.python.org/3/library/concurrent.futures.html#concurrent.futures... but since there is a python 2.7 backport we may be better served just using it.

It's also worth pointing out the futurist library, which is part of oslo:

https://opendev.org/openstack/futurist

It's similar to (and uses) concurrent.futures - except it also has a bunch of different backend impls - so you can have tasks run in threads, or it can use eventlet, or processes - or work synchronously.

thanks i was looking for this. i was aware that oslo.messaging to name buy one lib supported a configurable executor.

i had expected to find those executos in oslo.concurancy but looking a futurist it makes sense.

i see the executors defiend in https://opendev.org/openstack/futurist/src/branch/master/futurist/_futures.p... do indeed derive form the concurent future executor abstract base class.

the map function is in the executor base class is internally in terms of the submit function so the futuist executors are compatiable with the standard lib varients from python3.

i wonder if it would make sense to crete an optional dependency on privsep from futurist and add a privsep executor?

pivoting back to melenie's original question regarding what alternative we have for nova we could change our explict use of eventlets to the GreenThreadPoolExecutor https://opendev.org/openstack/futurist/src/branch/master/futurist/_futures.p... this would still require and use eventlets under the hood so the perfromace would be the same as our current explict use of the eventlet event loop but would allow us to change the eventloop via config the same way oslo.messaging allows changing for the eventlet executor to the threading executor.

if we removed all explcit use of eventlet this way we could then start looking at our implit use of eventlets which is largely for db acess, rest api calls to other services and some virt dirver specific calls.

anyway i had assumed that revisiting dropping eventlets form services would be a U conversation at the earliest but if we swap to using the futurist executors for the multi cell scater gather list operation we could stop monkeypatching under wsgi again and instead use the ThreadPoolExecutor or ProcessPoolExecutor in that case. when running the api without wsgi we can revert to the GreenThreadPoolExecutor.

Thanks for all this info. I've taken a hack at replacing direct eventlet use with futurist.GreenThreadPoolExecutor in scatter_gather_cells as a first step: https://review.openstack.org/650172 -melanie

...

...

privsep is effectivly just a ThreadPoolExecutor that layers on the privlage escalation/descalation logic via either fork and drop privalages or sudo + rootwrap/privsep helper to gain privileges

On 3/15/19 8:17 PM, Sean Mooney wrote:

i know its unrealistic to transition away from eventlet entirly while we still support python 2

Oh! One more good reason to get rid of Python 2. Thanks. :)

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.

Please!

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.

When are we planning to get rid of Python 2? My understanding is that pretty much all downstream distros have flipped the switch, no? Cheers, Thomas Goirand (zigo)

On 17/03/19 6:50 PM, Tony Breeds wrote:

On Sun, Mar 17, 2019 at 11:05:05PM +0100, Thomas Goirand wrote:

...

When are we planning to get rid of Python 2? My understanding is that pretty much all downstream distros have flipped the switch, no?

The 'U' Release is the first release where existing projects can switch to py3 only (ie remove py2 support). New projects have been able to be py3 only since we opened for stein development.

And for the record, this is documented here: https://governance.openstack.org/tc/resolutions/20180529-python2-deprecation...

The latest patch is here, and finally passes CI: https://review.openstack.org/#/c/626952/ . Note that it's been through a bunch of iterations as I've played whack-a-bug, but I finally feel pretty good about it. A point of clarification: I believe this bug *is* a regression from Rocky. I wasn't previously clear on that myself, but looking at the history I have convinced myself, at least, that this is the case. In Rocky, wsgi apps were not monkey patched, and they did not require it. https://review.openstack.org/#/c/592698/ (Batch results per cell when doing cross-cell listing) landed on 25th August 2018, which regressed wsgi apps by introducing an eventlet dependency in nova-api without adding monkey patching to wsgi apps. https://review.openstack.org/#/c/592285/ (Make monkey patch work in uWSGI mode) added monkey patching to wsgi apps, but added it late in the import order such that it still fails for some library version combinations. My patch moves the monkey patching of wsgi apps much earlier in the import order, which finally fixes the original regression introduced by cross-cell listing. The whole eventlet monkey patching situation is still a hack, and this change is really just rearranging the sticky tape. However, a fix is necessary Stein which was not required for Rocky. For Train/U it would be great to start ripping it out. Matt On Mon, 18 Mar 2019 at 12:04, Matthew Booth <mbooth@redhat.com> wrote:

On Fri, 15 Mar 2019 at 19:00, Chris Dent <cdent+os@anticdent.org> 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.

...

...

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.

Thanks for the feedback, folks.

I appreciate the desire to drop eventlet entirely (elsewhere in this thread) but as has also been noted that's not a short term option. Probably feasible for U, though, and lets keep it in mind. On that note, my preference for multi-threaded in Nova has always been to use Python's own apis except when that's not possible. Eventlet monkey patches these to do its own thing when it's in use, and when not using eventlet this obviously works fine.

I think I'm hearing that moving monkey patching to the top level is the right solution for now. I'm going to go ahead and knock that up.

Matt -- Matthew Booth Red Hat OpenStack Engineer, Compute DFG

Phone: +442070094448 (UK)

-- Matthew Booth Red Hat OpenStack Engineer, Compute DFG Phone: +442070094448 (UK)