[PATCH] CTDB protocol and client fixes

[Volker Lendecke]

On Fri, Nov 20, 2015 at 11:25:58AM +1100, Amitay Isaacs wrote:
> The premise here is the "nested event loops are bad".  With single event
> context nesting is definitely evil.  However, using multiple event contexts
> also leads to nesting, which is not identified as nesting in the tevent
> framework.

The idea is that with what we call nested event contexts is
that in tevent_loop_once only the one event source can fire.

> Creating temporary event context in the sync api essentially creates nested
> event loops.  If the code requires the creation of nested event contexts,

Just to get a common understanding: When I talk about nested
event loops I always mean multiple tevent_loop_once calls on
the stack for the same event context.

> then it's just bad code and it should be rewritten using async api to get
> rid of the nested event loops.

100% ack. If we were there, we would not need the sync
wrappers anymore.

> Also, the abstractions in the new ctdb client code (pkt_read, comm) are
> designed with single event context in mind and they will not work with
> temporary contexts.  To support nested event loops,  I will have to
> redesign those abstractions.

Just picking a random sync wrapper sample from source3/libsmb:

        if (smbXcli_conn_has_async_calls(cli->conn)) {
                /*                                                              
                 * Can't use sync call while an async call
                 * is in flight         
                 */
                status = NT_STATUS_INVALID_PARAMETER;
                goto fail;
        }

It's right that it is *very* hard to support both sync and
async calls simultaneously. In source3/libsmb we took the
path to just bail if this happens.

The real problem in the ctdb protocol is async messages. You
do a sync call on a nested event *context*, and an async
message comes in. What do you do? We need some provision for
that. But apart from this, I do not see why having nested
event contexts is really a problem.

> I don't see any benefit of using temporary event contexts.  Can you
> elaborate why using nested event loops is a good thing?

Nested event *loops* on the same event *context* are evil,
essentially that's pthreads without mutexes. Nested event
*contexts* are okay, they are a good way to limit the events
that can fire to just the ones we expect and silence
everything else.

Hope that helps,

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de