[PATCH] Make server-side copy (copy-chunk) async

[Ralph Böhme]

On Wed, Mar 22, 2017 at 02:44:48PM -0700, Jeremy Allison wrote:
> On Wed, Mar 22, 2017 at 10:24:39PM +0100, Ralph Böhme wrote:
> > On Wed, Mar 22, 2017 at 01:22:23PM -0700, Jeremy Allison wrote:
> > > On Wed, Mar 22, 2017 at 07:14:05PM +0100, Ralph Böhme via samba-technical wrote:
> > > > Hi!
> > > > 
> > > > Attached is a patchset I've been working on that makes our server-side copy
> > > > asynchronous.
> > > > 
> > > > At the SMB layer the chunks are processed sequentially in order, but left to
> > > > right merged if copy ranges are adjecent. In the backend (vfs_default) I'm
> > > > essentially just using pread|pwrite_send|recv instead of the sync versions.
> > > > 
> > > > I'm also adding a small new feature to tevent: cancel forwarding. It allows the
> > > > implementation of a tevent request using subrequest to request forwarding of
> > > > cancels to be done automagically by the tevent library. This is useful for
> > > > forwarding cancels through a chain of subrequests.
> > > 
> > > OK. I have to ask the nasty question :-).
> > > 
> > > Do you *really* need to extend tevent to add this ?
> > > 
> > > You're only forwarding cancel to one event at a time.
> > > 
> > > Couldn't you just add a normal cancel function
> > > on the parent req, which calls tevent_req_cancel()
> > > on the subreq stored in the parent req private data ?
> > 
> > In the end I need the cancel to get all the way down to vfs_default, as that's
> > where the actual copying takes place, potentially in a loop (fruit:copyfile
> > case) copying some big file in *completely* in one subreq. *That*s the one I
> > want to be able to cancle, though admittedly the client doesn't support
> > cancelling a pending fruit:copyfile style copy.
> 
> I still don't see why you need to change the
> tevent library to add this. Each cancel function
> forwards to the next one down the stack.
> 
> Yeah it's more work, but is it a generic
> feature needed to add to tevent ? It's
> replacing a linking tevent_cancel function
> that forwards manually to a stored subreq, right ?

Sure, it's just syntactic sugar. I just found it this piece was missing in
the tevent request libraray, request cancel forwarding just with a single
function call instead of adding a full fledged cancel function that the just
pushes the cancel button of the subrequest.

> What about a tevent useage where a containing
> tevent_req issues a set of multiple async
> sub requests backed by underlying pthreads,
> or a it's using a tevent_queue ? It doesn't
> help cancelling them as there's more than
> one subreq we're waiting on.

Sure, for the more complicated cases you need a full fledged cancel
functions. tevent_req_set_cancel_forwarding() is supposed to be used from
function that would otherwise don't need a cancel function at all.

> Convince me ! :-) :-).

With tevent_req_set_cancel_forwarding() it's one line of code instead of, well,
10? I'm using it in four places, makes 40 lines less boilerplate code. Smaller
diff wins! :)

I personally just *like* this API, but I guess I'm biased. :)

Cheerio!
-slow