[PATCH] CTDB protocol and client fixes

[Amitay Isaacs]

On Thu, Nov 19, 2015 at 7:36 PM, Stefan Metzmacher <metze at samba.org> wrote:

> Hi Amitay,
>
> > Here are the changes to the new protocol and client code.
> >
> > Many of them are fixes required for the rewrite of ctdb tool and test
> > binaries using new client API.
> >
> > Major changes to protocol code include:
> >
> > 1. Fix marshaling of data types ctdb_addr_info, ctdb_public_ip_list,
> > ctdb_rec_data
> > 2. Add utility function ctdb_sock_addr_to_string
> >
> > Major changes to client code include:
> >
> > 1. Avoid using sync functions in async computations (tevent_req  based)
> > 2. Add missing sync/async versions of some of the APIs
> > 3. Drop TALLOC_CTX where not required
>
> Can you also remove the 'struct tevent_context *ev' argument from all
> sync functions? They should create a temporary event context,
> see smb2cli_echo() and others as example.
>
> metze
>
>
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.

Creating temporary event context in the sync api essentially creates nested
event loops.  If the code requires the creation of nested event contexts,
then it's just bad code and it should be rewritten using async api to get
rid of the nested event loops.

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.

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

Amitay.