Open Stack

On 17/12/15 14:17, SURO wrote:
> Josh,
> You pointed out correct! magnum-conductor has monkey-patched code, so
> the underlying thread module is actually using greenthread.
> - I would use eventlet.greenthread explicitly, as that would enhance the
> readability

I'd suggest that you should use the oslo_service.threadgroup module 
instead. (It uses greenthreads under the hood.)

> - greenthread has a potential of not yielding by itself, if no i/o,
> blocking call is made. But in the present scenario, it is not much of a
> concern, as the container-operation execution is lighter on the client
> side, and mostly block for the response from the server, after issuing
> the request.

Yep, that's correct.

Note that using pre-emptive OS threads - i.e. the stdlib 
(non-monkeypatched) threading.Thread - comes with severe performance 
penalties in Python, which is why Oslo and most OpenStack projects use 
eventlet.

cheers,
Zane.

> I will update the proposal with this change.
>
> Regards,
> SURO
> irc//freenode: suro-patz
>
> On 12/16/15 11:57 PM, Joshua Harlow wrote:
>> SURO wrote:
>>> Josh,
>>> Please find my reply inline.
>>>
>>> Regards,
>>> SURO
>>> irc//freenode: suro-patz
>>>
>>> On 12/16/15 6:37 PM, Joshua Harlow wrote:
>>>>
>>>>
>>>> SURO wrote:
>>>>> Hi all,
>>>>> Please review and provide feedback on the following design proposal
>>>>> for
>>>>> implementing the blueprint[1] on async-container-operations -
>>>>>
>>>>> 1. Magnum-conductor would have a pool of threads for executing the
>>>>> container operations, viz. executor_threadpool. The size of the
>>>>> executor_threadpool will be configurable. [Phase0]
>>>>> 2. Every time, Magnum-conductor(Mcon) receives a
>>>>> container-operation-request from Magnum-API(Mapi), it will do the
>>>>> initial validation, housekeeping and then pick a thread from the
>>>>> executor_threadpool to execute the rest of the operations. Thus Mcon
>>>>> will return from the RPC request context much faster without blocking
>>>>> the Mapi. If the executor_threadpool is empty, Mcon will execute in a
>>>>> manner it does today, i.e. synchronously - this will be the
>>>>> rate-limiting mechanism - thus relaying the feedback of exhaustion.
>>>>> [Phase0]
>>>>> How often we are hitting this scenario, may be indicative to the
>>>>> operator to create more workers for Mcon.
>>>>> 3. Blocking class of operations - There will be a class of operations,
>>>>> which can not be made async, as they are supposed to return
>>>>> result/content inline, e.g. 'container-logs'. [Phase0]
>>>>> 4. Out-of-order considerations for NonBlocking class of operations -
>>>>> there is a possible race around condition for create followed by
>>>>> start/delete of a container, as things would happen in parallel. To
>>>>> solve this, we will maintain a map of a container and executing
>>>>> thread,
>>>>> for current execution. If we find a request for an operation for a
>>>>> container-in-execution, we will block till the thread completes the
>>>>> execution. [Phase0]
>>>>> This mechanism can be further refined to achieve more asynchronous
>>>>> behavior. [Phase2]
>>>>> The approach above puts a prerequisite that operations for a given
>>>>> container on a given Bay would go to the same Magnum-conductor
>>>>> instance.
>>>>> [Phase0]
>>>>> 5. The hand-off between Mcon and a thread from executor_threadpool can
>>>>> be reflected through new states on the 'container' object. These
>>>>> states
>>>>> can be helpful to recover/audit, in case of Mcon restart. [Phase1]
>>>>>
>>>>> Other considerations -
>>>>> 1. Using eventlet.greenthread instead of real threads => This approach
>>>>> would require further refactoring the execution code and embed yield
>>>>> logic, otherwise a single greenthread would block others to progress.
>>>>> Given, we will extend the mechanism for multiple COEs, and to keep the
>>>>> approach straight forward to begin with, we will use
>>>>> 'threading.Thread'
>>>>> instead of 'eventlet.greenthread'.
>>>>>
>>>>
>>>> Also unsure about the above, not quite sure I connect how greenthread
>>>> usage requires more yield logic (I'm assuming you mean the yield
>>>> statement here)? Btw if magnum is running with all things monkey
>>>> patched (which it seems like
>>>> https://github.com/openstack/magnum/blob/master/magnum/common/rpc_service.py#L33
>>>>
>>>> does) then magnum usage of 'threading.Thread' is a
>>>> 'eventlet.greenthread' underneath the covers, just fyi.
>>>>
>>> SURO> Let's consider this -
>>> function A () {
>>> block B; // validation
>>> block C; // Blocking op
>>> }
>>> Now, if we make C a greenthread, as it is, would it not block the entire
>>> thread that runs through all the greenthreads? I assumed, it would and
>>> that's why we have to incorporate finer grain yield into C to leverage
>>> greenthread. If the answer is no, then we can use greenthread.
>>> I will validate which version of threading.Thread was getting used.
>>
>> Unsure how to answer this one.
>>
>> If all things are monkey patched then any time a blocking operation
>> (i/o, lock acquisition...) is triggered the internals of eventlet go
>> through a bunch of jumping around to then switch to another green
>> thread (http://eventlet.net/doc/hubs.html). Once u start partially
>> using greenthreads and mixing real threads then you have to start
>> trying to reason about yielding in certain places (and at that point
>> you might as well go to py3.4+ since it has syntax made just for this
>> kind of thinking).
>>
>> Pointer for the thread monkey patching btw:
>>
>> https://github.com/eventlet/eventlet/blob/master/eventlet/patcher.py#L346
>>
>> https://github.com/eventlet/eventlet/blob/master/eventlet/patcher.py#L212
>>
>> Easy way to see this:
>>
>> >>> import eventlet
>> >>> eventlet.monkey_patch()
>> >>> import thread
>> >>> thread.start_new_thread.__module__
>> 'eventlet.green.thread'
>> >>> thread.allocate_lock.__module__
>> 'eventlet.green.thread'
>>
>>>
>>> In that case, keeping the code for thread.Threading is portable, as it
>>> would work as desired, even if we remove monkey_patching, right?
>>
>> Yes, use `thread.Threading` (if u can) so that maybe magnum could
>> switch off monkey patching someday, although typically unless u are
>> already testing that turning it off in unit tests/functional tests it
>> wouldn't be an easy flip that will typically 'just work' (especially
>> since afaik magnum is using some oslo libraries which only work under
>> greenthreads/eventlet).
>>
>>>>>
>>>>> Refs:-
>>>>> [1] -
>>>>> https://blueprints.launchpad.net/magnum/+spec/async-container-operations
>>>>>
>>>>>
>>>>
>>>> __________________________________________________________________________
>>>>
>>>>
>>>> OpenStack Development Mailing List (not for usage questions)
>>>> Unsubscribe:
>>>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>>
>>> __________________________________________________________________________
>>>
>>> OpenStack Development Mailing List (not for usage questions)
>>> Unsubscribe:
>>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>> __________________________________________________________________________
>>
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe:
>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev