[PATCH] CTDB protocol and client fixes
amitay at gmail.com
Tue Nov 24 08:35:32 UTC 2015
On Tue, Nov 24, 2015 at 6:45 PM, Volker Lendecke <Volker.Lendecke at sernet.de>
> On Tue, Nov 24, 2015 at 06:04:40PM +1100, Amitay Isaacs wrote:
> > On Tue, Nov 24, 2015 at 5:48 PM, Volker Lendecke <
> Volker.Lendecke at sernet.de>
> > wrote:
> > > On Tue, Nov 24, 2015 at 05:33:23PM +1100, Amitay Isaacs wrote:
> > > > Since no one has replied to this email, let me summarize.
> > > >
> > > > 1. For some reason I cannot fathom, async processing is treated
> > > *different*
> > > > from sync processing. A global event context for all async
> processing is
> > > > ok, but for sync processing need lots of temporary contexts.
> > >
> > > When you use tevent_loop_once deep inside the call stack where some
> > > other tevent_loop_once is also active, anything can happen. As long as
> > > callers work without tevent_loop_allow_nesting then it might be
> > >
> > > > 2. I can drop the tevent_context from sync API, but it will still
> use the
> > > > cached event context rather than creating a new one for every sync
> > > > call. This is because of the current design of the client API. It
> > > > designed to not use tevent_add_fd() for reading each individual
> > > from
> > > > an fd.
> > >
> > > "cached event context" is something that I am worried about using in
> > > smbd and winbind. We have gone through a lot of work to remove this
> > > the core smb libraries. You might prove me wrong by finding spots where
> > > we are not there yet, but in the SMB libraries I would see that as
> > > a bug that needs fixing.
> > >
> > The cached event context arises only in sync API which will be primarily
> > used in the ctdb tool and some of the test code. Samba would never use
> > CTDB's sync API, but (I am hoping) will be converted to use CTDB's async
> > client API.
> > Questions are:
> > 1. Should I keep the sync API completely separate, so it's not used by
> > code other than CTDB. As I mentioned before, the new client API is not a
> > public library yet. When we make CTDB client API public, then it can
> > publish the async version and not the sync version.
> > 2. More fundamental question: Should I get rid of the optimization of not
> > using tevent_add_fd() for reading each individual packet from an fd?
> > avoids the extra system call, but requires the use of same event context.
> > If no one else cares about it, I would be willing to get rid of it.
> If you have a message-listener always active, you might live
> with one tevent_fd always active. You then of course have
> the problem of dispatching to the right listener
> (message/correct control req), but with multiple active
> tevent_req's for controls you have that problem anyway.
> Sure, this lifts the problem up one level, but we keep it
> away from the base libraries.
In the client API (which is not very low level API), each control request
is tracked with unique reqid. So there can be multiple tevent_reqs in
flight at the same time. In fact, the parallel database recovery helper
relies on this to work correctly. As you rightly identified, there is
always a message-listener active which uses the single tevent_fd to get all
CTDB client API uses other abstractions (comm, read_pkt, write_pkt, reqid)
to implement the single tevent_fd based packet read and reqid based
> If the sync APIs are only ever called inside ctdb tool or
> tests, why is the additional overhead a problem?
The problem is not of the overhead. Since the API is designed around
single tevent_fd, you always need the same event context around. Otherwise
you end up having to do tevent_add_fd() in each sync API. This is even
trickier since the tevent_add_fd() is burried in a lower level abstraction
(comm). That's why it's easier to implement sync API with single event
context. And if I drop tevent_context from the API arguments, then I will
need to use the cached event context.
More information about the samba-technical