Re: [PATCH v4 2/2] git-svn: allow git-svn fetching to work using serf

On Jul 18, 2013, at 12:29, Jonathan Nieder wrote:

(cc-ing Eric Wong, who maintains git-svn and knows both it and
the libsvn perl bindings much better than I do)
Kyle J. McKay wrote:

quoted

David Rothenberger [off-list ref] has determined the cause to
be that ra_serf does not drive the delta editor in a depth-first
manner [...]. Instead, the calls come in this order:

Thanks.

Sorry to nitpick, but the problem is not depth-first versus
breadth-first versus random.  Blaming the traversal order makes this
completely confusing.  The actual problem is that the driver asks us
to keep multiple files open at a time.

On Sun, 07 Jul 2013 18:00:40 GMT, Branko Čibej wrote:

On 07.07.2013 19:40, Jonathan Nieder wrote:

quoted

(cc-ing subversion's users@ list for advice)
Kyle McKay wrote:

quoted

On Jul 6, 2013, at 18:37, Jonathan Nieder wrote:

quoted

quoted

Kyle McKay wrote:

quoted

Begin forwarded message:

quoted

[2] http://subversion.tigris.org/issues/show_bug.cgi?id=2932

Ah, thanks for the context.

It's still not clear to me how we know that ra_serf driving the  
editor
in a non depth-first manner is the problem here.  Has that  
explanation
been confirmed somehow?

[...]
Since ra_serf makes multiple connections to the server (hard-coded
to 4 prior to svn 1.8, defaults to 4 in svn 1.8 but can be set to
between 1 and 8) it makes sense there would be multiple active calls
to apply_textdelta if processing is done as results are received on
the multiple connections.

Ah, that's worrisome.  Do I understand you correctly that to work  
with
ra_serf in skelta mode, callers need to make their apply_textdelta
callback thread-safe?

No; the editor drive is single-threaded, but the order of the  
operations
isn't strictly depth-first.

Brane also describes this as a non-depth-first traversal order which  
is the root of the problem.  If the order of operations were strictly  
depth-first, the previous file would end up being closed before the  
next one's opened.

The approach taken in this patch would be racy if the driver calls us
multiple times concurrently (since temp_acquire can fail).  I believe
it doesn't but haven't checked.

Brane says the editor drive is single-threaded so that doesn't seem  
like a problem.